Skip to content

Restrict registry names to same style as package names. #6469

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 20, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions src/cargo/core/package_id_spec.rs
Original file line number Diff line number Diff line change
@@ -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,
17 changes: 2 additions & 15 deletions src/cargo/ops/cargo_new.rs
Original file line number Diff line number Diff line change
@@ -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(())
}

17 changes: 10 additions & 7 deletions src/cargo/ops/registry.rs
Original file line number Diff line number Diff line change
@@ -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<String>,
) -> CargoResult<RegistryConfig> {
let (index, token) = match registry {
Some(registry) => (
Some(config.get_registry_index(&registry)?.to_string()),
config
.get_string(&format!("registries.{}.token", registry))?
.map(|p| p.val),
),
Some(registry) => {
validate_package_name(&registry, "registry name", "")?;
(
Some(config.get_registry_index(&registry)?.to_string()),
config
.get_string(&format!("registries.{}.token", registry))?
.map(|p| p.val),
)
}
None => {
// Checking out for default index and token
(
5 changes: 3 additions & 2 deletions src/cargo/util/command_prelude.rs
Original file line number Diff line number Diff line change
@@ -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)
3 changes: 2 additions & 1 deletion src/cargo/util/config.rs
Original file line number Diff line number Diff line change
@@ -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<Url> {
validate_package_name(registry, "registry name", "")?;
Ok(
match self.get_string(&format!("registries.{}.index", registry))? {
Some(index) => {
16 changes: 16 additions & 0 deletions src/cargo/util/mod.rs
Original file line number Diff line number Diff line change
@@ -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(())
}
16 changes: 2 additions & 14 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
@@ -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)?;

47 changes: 47 additions & 0 deletions tests/testsuite/alt_registry.rs
Original file line number Diff line number Diff line change
@@ -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();
}
}