From 080f0b34c468f9d9065fec7cc35dcebdae38e4d7 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Wed, 19 Dec 2018 20:33:57 -0800 Subject: [PATCH] Restrict registry names to same style as package names. --- src/cargo/core/package_id_spec.rs | 8 ++---- src/cargo/ops/cargo_new.rs | 17 ++--------- src/cargo/ops/registry.rs | 17 ++++++----- src/cargo/util/command_prelude.rs | 5 ++-- src/cargo/util/config.rs | 3 +- src/cargo/util/mod.rs | 16 +++++++++++ src/cargo/util/toml/mod.rs | 16 ++--------- tests/testsuite/alt_registry.rs | 47 +++++++++++++++++++++++++++++++ 8 files changed, 84 insertions(+), 45 deletions(-) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index dcb216f12fe..a2cb6171b80 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -7,7 +7,7 @@ use url::Url; use crate::core::PackageId; use crate::util::errors::{CargoResult, CargoResultExt}; -use crate::util::{ToSemver, ToUrl}; +use crate::util::{validate_package_name, ToSemver, ToUrl}; /// Some or all of the data required to identify a package: /// @@ -64,11 +64,7 @@ impl PackageIdSpec { Some(version) => Some(Version::parse(version)?), None => None, }; - for ch in name.chars() { - if !ch.is_alphanumeric() && ch != '_' && ch != '-' { - failure::bail!("invalid character in pkgid `{}`: `{}`", spec, ch) - } - } + validate_package_name(name, "pkgid", "")?; Ok(PackageIdSpec { name: name.to_string(), version, diff --git a/src/cargo/ops/cargo_new.rs b/src/cargo/ops/cargo_new.rs index 3fdd88141af..7bef7e8a665 100644 --- a/src/cargo/ops/cargo_new.rs +++ b/src/cargo/ops/cargo_new.rs @@ -10,7 +10,7 @@ use git2::Repository as GitRepository; use crate::core::{compiler, Workspace}; use crate::util::errors::{CargoResult, CargoResultExt}; use crate::util::{existing_vcs_repo, internal, FossilRepo, GitRepo, HgRepo, PijulRepo}; -use crate::util::{paths, Config}; +use crate::util::{paths, validate_package_name, Config}; use toml; @@ -161,20 +161,7 @@ fn check_name(name: &str, opts: &NewOptions) -> CargoResult<()> { } } - for c in name.chars() { - if c.is_alphanumeric() { - continue; - } - if c == '_' || c == '-' { - continue; - } - failure::bail!( - "Invalid character `{}` in crate name: `{}`{}", - c, - name, - name_help - ) - } + validate_package_name(name, "crate name", name_help)?; Ok(()) } diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 9d159459b1e..8a1c4d6badd 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -19,8 +19,8 @@ use crate::sources::{RegistrySource, SourceConfigMap}; use crate::util::config::{self, Config}; use crate::util::errors::{CargoResult, CargoResultExt}; use crate::util::important_paths::find_root_manifest_for_wd; -use crate::util::paths; use crate::util::ToUrl; +use crate::util::{paths, validate_package_name}; use crate::version; pub struct RegistryConfig { @@ -294,12 +294,15 @@ pub fn registry_configuration( registry: Option, ) -> CargoResult { let (index, token) = match registry { - Some(registry) => ( - Some(config.get_registry_index(®istry)?.to_string()), - config - .get_string(&format!("registries.{}.token", registry))? - .map(|p| p.val), - ), + Some(registry) => { + validate_package_name(®istry, "registry name", "")?; + ( + Some(config.get_registry_index(®istry)?.to_string()), + config + .get_string(&format!("registries.{}.token", registry))? + .map(|p| p.val), + ) + } None => { // Checking out for default index and token ( diff --git a/src/cargo/util/command_prelude.rs b/src/cargo/util/command_prelude.rs index f57fa805821..4ae067585ea 100644 --- a/src/cargo/util/command_prelude.rs +++ b/src/cargo/util/command_prelude.rs @@ -6,7 +6,7 @@ use crate::core::Workspace; use crate::ops::{CompileFilter, CompileOptions, NewOptions, Packages, VersionControl}; use crate::sources::CRATES_IO_REGISTRY; use crate::util::important_paths::find_root_manifest_for_wd; -use crate::util::paths; +use crate::util::{paths, validate_package_name}; use crate::CargoResult; use clap::{self, SubCommand}; @@ -370,11 +370,12 @@ pub trait ArgMatchesExt { requires -Zunstable-options to use." )); } + validate_package_name(registry, "registry name", "")?; if registry == CRATES_IO_REGISTRY { // If "crates.io" is specified then we just need to return None // as that will cause cargo to use crates.io. This is required - // for the case where a default alterative registry is used + // for the case where a default alternative registry is used // but the user wants to switch back to crates.io for a single // command. Ok(None) diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index 8a3d92028ee..cb01edaa05b 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -24,11 +24,11 @@ use crate::core::shell::Verbosity; use crate::core::{CliUnstable, Shell, SourceId, Workspace}; use crate::ops; use crate::util::errors::{internal, CargoResult, CargoResultExt}; -use crate::util::paths; use crate::util::toml as cargo_toml; use crate::util::Filesystem; use crate::util::Rustc; use crate::util::ToUrl; +use crate::util::{paths, validate_package_name}; use url::Url; use self::ConfigValue as CV; @@ -656,6 +656,7 @@ impl Config { /// Gets the index for a registry. pub fn get_registry_index(&self, registry: &str) -> CargoResult { + validate_package_name(registry, "registry name", "")?; Ok( match self.get_string(&format!("registries.{}.index", registry))? { Some(index) => { diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index 598a94b2f77..2860133445e 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -59,3 +59,19 @@ pub fn elapsed(duration: Duration) -> String { format!("{}.{:02}s", secs, duration.subsec_nanos() / 10_000_000) } } + +/// Check the base requirements for a package name. +/// +/// This can be used for other things than package names, to enforce some +/// level of sanity. Note that package names have other restrictions +/// elsewhere. `cargo new` has a few restrictions, such as checking for +/// reserved names. crates.io has even more restrictions. +pub fn validate_package_name(name: &str, what: &str, help: &str) -> CargoResult<()> { + if let Some(ch) = name + .chars() + .find(|ch| !ch.is_alphanumeric() && *ch != '_' && *ch != '-') + { + failure::bail!("Invalid character `{}` in {}: `{}`{}", ch, what, name, help); + } + Ok(()) +} diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 3c691d5a465..53e0d7ad8b3 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -21,7 +21,7 @@ use crate::core::{GitReference, PackageIdSpec, SourceId, WorkspaceConfig, Worksp use crate::sources::{CRATES_IO_INDEX, CRATES_IO_REGISTRY}; use crate::util::errors::{CargoResult, CargoResultExt, ManifestError}; use crate::util::paths; -use crate::util::{self, Config, ToUrl}; +use crate::util::{self, validate_package_name, Config, ToUrl}; mod targets; use self::targets::targets; @@ -809,19 +809,7 @@ impl TomlManifest { failure::bail!("package name cannot be an empty string") } - for c in package_name.chars() { - if c.is_alphanumeric() { - continue; - } - if c == '_' || c == '-' { - continue; - } - failure::bail!( - "Invalid character `{}` in package name: `{}`", - c, - package_name - ) - } + validate_package_name(package_name, "package name", "")?; let pkgid = project.to_package_id(source_id)?; diff --git a/tests/testsuite/alt_registry.rs b/tests/testsuite/alt_registry.rs index 4c734234ebc..ce0728497bd 100644 --- a/tests/testsuite/alt_registry.rs +++ b/tests/testsuite/alt_registry.rs @@ -558,3 +558,50 @@ fn patch_alt_reg() { ) .run(); } + +#[test] +fn bad_registry_name() { + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["alternative-registries"] + + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies.bar] + version = "0.0.1" + registry = "bad name" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + p.cargo("build") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr( + "\ +[ERROR] failed to parse manifest at `[CWD]/Cargo.toml` + +Caused by: + Invalid character ` ` in registry name: `bad name`", + ) + .run(); + + for cmd in &[ + "init", "install", "login", "owner", "publish", "search", "yank", + ] { + p.cargo(cmd) + .arg("-Zunstable-options") + .arg("--registry") + .arg("bad name") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr("[ERROR] Invalid character ` ` in registry name: `bad name`") + .run(); + } +}