Skip to content
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

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

Merged
merged 1 commit into from
Dec 20, 2018
Merged
Show file tree
Hide file tree
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
Expand Up @@ -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:
///
Expand Down Expand Up @@ -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,
Expand Down
17 changes: 2 additions & 15 deletions src/cargo/ops/cargo_new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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(())
}

Expand Down
17 changes: 10 additions & 7 deletions src/cargo/ops/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
(
Expand Down
5 changes: 3 additions & 2 deletions src/cargo/util/command_prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion src/cargo/util/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) => {
Expand Down
16 changes: 16 additions & 0 deletions src/cargo/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Up @@ -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;
Expand Down Expand Up @@ -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)?;

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