From 7956f03dbcddb44c3b8aecf92922c67645f970bb Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Fri, 6 Mar 2020 08:53:11 -0800 Subject: [PATCH 1/5] Add some doc-strings to registry functions. --- src/cargo/ops/registry.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 3b287816a83..0db43ebee80 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -25,8 +25,13 @@ use crate::util::IntoUrl; use crate::util::{paths, validate_package_name}; use crate::version; +/// Registry settings loaded from config files. +/// +/// This is loaded based on the `--registry` flag and the config settings. pub struct RegistryConfig { + /// The index URL. If `None`, use crates.io. pub index: Option, + /// The authentication token. pub token: Option, } @@ -316,10 +321,15 @@ fn transmit( } } +/// Returns the index and token from the config file for the given registry. +/// +/// `registry` is typically the registry specified on the command-line. If +/// `None`, returns the default index. pub fn registry_configuration( config: &Config, registry: Option, ) -> CargoResult { + // `registry.default` is handled in command-line parsing. let (index, token) = match registry { Some(registry) => { validate_package_name(®istry, "registry name", "")?; @@ -344,6 +354,17 @@ pub fn registry_configuration( Ok(RegistryConfig { index, token }) } +/// Returns the `Registry` and `Source` based on command-line and config settings. +/// +/// * `token`: The token from the command-line. If not set, uses the token +/// from the config. +/// * `index`: The index URL from the command-line. This is ignored if +/// `registry` is set. +/// * `registry`: The registry name from the command-line. If neither +/// `registry`, or `index` are set, then uses `crates-io`, honoring +/// `[source]` replacement if defined. +/// * `force_update`: If `true`, forces the index to be updated. +/// * `validate_token`: If `true`, the token must be set. fn registry( config: &Config, token: Option, @@ -739,6 +760,10 @@ pub fn yank( Ok(()) } +/// Gets the SourceId for an index or registry setting. +/// +/// The `index` and `reg` values are from the command-line or config settings. +/// If both are None, returns the source for crates.io. fn get_source_id( config: &Config, index: Option, From 9d00f9f07fa636650d4f9eb088f70633c194bb76 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Fri, 6 Mar 2020 09:56:58 -0800 Subject: [PATCH 2/5] Make setting `registry.index` a hard error. This has been deprecated for 4 years. This helps simplify this code. --- src/cargo/core/source/source_id.rs | 21 +-------- src/cargo/ops/registry.rs | 12 ++--- src/cargo/util/config/mod.rs | 15 +++--- src/doc/src/reference/config.md | 2 +- tests/testsuite/alt_registry.rs | 73 ------------------------------ tests/testsuite/registry.rs | 50 ++++++++++++++++++++ 6 files changed, 66 insertions(+), 107 deletions(-) diff --git a/src/cargo/core/source/source_id.rs b/src/cargo/core/source/source_id.rs index f93c34161b5..d54b1f4fb0a 100644 --- a/src/cargo/core/source/source_id.rs +++ b/src/cargo/core/source/source_id.rs @@ -4,8 +4,6 @@ use std::fmt::{self, Formatter}; use std::hash::{self, Hash}; use std::path::Path; use std::ptr; -use std::sync::atomic::AtomicBool; -use std::sync::atomic::Ordering::SeqCst; use std::sync::Mutex; use log::trace; @@ -14,7 +12,6 @@ use serde::ser; use url::Url; use crate::core::PackageId; -use crate::ops; use crate::sources::DirectorySource; use crate::sources::{GitSource, PathSource, RegistrySource, CRATES_IO_INDEX}; use crate::util::{CanonicalUrl, CargoResult, Config, IntoUrl}; @@ -189,22 +186,8 @@ impl SourceId { /// a `.cargo/config`. pub fn crates_io(config: &Config) -> CargoResult { config.crates_io_source_id(|| { - let cfg = ops::registry_configuration(config, None)?; - let url = if let Some(ref index) = cfg.index { - static WARNED: AtomicBool = AtomicBool::new(false); - if !WARNED.swap(true, SeqCst) { - config.shell().warn( - "custom registry support via \ - the `registry.index` configuration is \ - being removed, this functionality \ - will not work in the future", - )?; - } - &index[..] - } else { - CRATES_IO_INDEX - }; - let url = url.into_url()?; + config.check_registry_index_not_set()?; + let url = CRATES_IO_INDEX.into_url().unwrap(); SourceId::for_registry(&url) }) } diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 0db43ebee80..fe9b52577cd 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -324,7 +324,7 @@ fn transmit( /// Returns the index and token from the config file for the given registry. /// /// `registry` is typically the registry specified on the command-line. If -/// `None`, returns the default index. +/// `None`, `index` is set to `None` to indicate it should use crates.io. pub fn registry_configuration( config: &Config, registry: Option, @@ -341,13 +341,9 @@ pub fn registry_configuration( ) } None => { - // Checking for default index and token - ( - config - .get_default_registry_index()? - .map(|url| url.to_string()), - config.get_string("registry.token")?.map(|p| p.val), - ) + // Use crates.io default. + config.check_registry_index_not_set()?; + (None, config.get_string("registry.token")?.map(|p| p.val)) } }; diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index 576ca50969a..260d011c851 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -1016,12 +1016,15 @@ impl Config { ) } - /// Gets the index for the default registry. - pub fn get_default_registry_index(&self) -> CargoResult> { - Ok(match self.get_string("registry.index")? { - Some(index) => Some(self.resolve_registry_index(index)?), - None => None, - }) + /// Returns an error if `registry.index` is set. + pub fn check_registry_index_not_set(&self) -> CargoResult<()> { + if self.get_string("registry.index")?.is_some() { + bail!( + "the `registry.index` config value is no longer supported\n\ + Use `[source]` replacement to alter the default index for crates.io." + ); + } + Ok(()) } fn resolve_registry_index(&self, index: Value) -> CargoResult { diff --git a/src/doc/src/reference/config.md b/src/doc/src/reference/config.md index 0a917024bdc..6fafaecc962 100644 --- a/src/doc/src/reference/config.md +++ b/src/doc/src/reference/config.md @@ -686,7 +686,7 @@ specified. ##### `registry.index` -This value is deprecated and should not be used. +This value is no longer accepted and should not be used. ##### `registry.default` * Type: string diff --git a/tests/testsuite/alt_registry.rs b/tests/testsuite/alt_registry.rs index 6561bdfbd06..cf03ab45559 100644 --- a/tests/testsuite/alt_registry.rs +++ b/tests/testsuite/alt_registry.rs @@ -528,28 +528,6 @@ fn publish_with_crates_io_dep() { ); } -#[cargo_test] -fn passwords_in_registry_index_url_forbidden() { - registry::init(); - - let config = paths::home().join(".cargo/config"); - fs::write( - config, - r#" - [registry] - index = "ssh://git:secret@foobar.com" - "#, - ) - .unwrap(); - - let p = project().file("src/main.rs", "fn main() {}").build(); - - p.cargo("publish") - .with_status(101) - .with_stderr_contains("error: Registry URLs may not contain passwords") - .run(); -} - #[cargo_test] fn passwords_in_registries_index_url_forbidden() { registry::init(); @@ -1222,57 +1200,6 @@ fn registries_index_relative_url() { .run(); } -#[cargo_test] -fn registry_index_relative_url() { - let config = paths::root().join(".cargo/config"); - fs::create_dir_all(config.parent().unwrap()).unwrap(); - fs::write( - &config, - r#" - [registry] - index = "file:alternative-registry" - "#, - ) - .unwrap(); - - registry::init(); - - let p = project() - .file( - "Cargo.toml", - r#" - [project] - name = "foo" - version = "0.0.1" - authors = [] - - [dependencies.bar] - version = "0.0.1" - "#, - ) - .file("src/main.rs", "fn main() {}") - .build(); - - Package::new("bar", "0.0.1").alternative(true).publish(); - - fs::remove_file(paths::home().join(".cargo/config")).unwrap(); - - p.cargo("build") - .with_stderr(&format!( - "\ -warning: custom registry support via the `registry.index` configuration is being removed, this functionality will not work in the future -[UPDATING] `{reg}` index -[DOWNLOADING] crates ... -[DOWNLOADED] bar v0.0.1 (registry `[ROOT][..]`) -[COMPILING] bar v0.0.1 (registry `[ROOT][..]`) -[COMPILING] foo v0.0.1 ([CWD]) -[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]s -", - reg = registry::alt_registry_path().to_str().unwrap() - )) - .run(); -} - #[cargo_test] fn registries_index_relative_path_not_allowed() { let config = paths::root().join(".cargo/config"); diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index 031064fba65..84f08786026 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -2079,3 +2079,53 @@ fn readonly_registry_still_works() { t!(fs::set_permissions(path, perms)); } } + +#[cargo_test] +fn registry_index_rejected() { + Package::new("dep", "0.1.0").publish(); + + let p = project() + .file( + ".cargo/config", + r#" + [registry] + index = "https://example.com/" + "#, + ) + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + + [dependencies] + dep = "0.1" + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("check") + .with_status(101) + .with_stderr( + "\ +[ERROR] failed to parse manifest at `[..]/foo/Cargo.toml` + +Caused by: + the `registry.index` config value is no longer supported +Use `[source]` replacement to alter the default index for crates.io. +", + ) + .run(); + + p.cargo("login") + .with_status(101) + .with_stderr( + "\ +[ERROR] the `registry.index` config value is no longer supported +Use `[source]` replacement to alter the default index for crates.io. +", + ) + .run(); +} From 25990711103817a71d7c570d015ddc40a0a12bd7 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Fri, 6 Mar 2020 10:15:14 -0800 Subject: [PATCH 3/5] Don't allow both --index and --registry to be specified at the same time. Otherwise --index was being silently ignored. --- src/cargo/ops/registry.rs | 4 ++++ tests/testsuite/alt_registry.rs | 16 ++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index fe9b52577cd..68d605a436d 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -369,6 +369,10 @@ fn registry( force_update: bool, validate_token: bool, ) -> CargoResult<(Registry, SourceId)> { + if index.is_some() && registry.is_some() { + // Otherwise we would silently ignore one or the other. + bail!("both `--index` and `--registry` should not be set at the same time"); + } // Parse all configuration options let RegistryConfig { token: token_config, diff --git a/tests/testsuite/alt_registry.rs b/tests/testsuite/alt_registry.rs index cf03ab45559..3c76d7d6ad1 100644 --- a/tests/testsuite/alt_registry.rs +++ b/tests/testsuite/alt_registry.rs @@ -1247,3 +1247,19 @@ Caused by: .with_status(101) .run(); } + +#[cargo_test] +fn both_index_and_registry() { + let p = project().file("src/lib.rs", "").build(); + for cmd in &["publish", "owner", "search", "yank --vers 1.0.0"] { + p.cargo(cmd) + .arg("--registry=foo") + .arg("--index=foo") + .with_status(101) + .with_stderr( + "[ERROR] both `--index` and `--registry` \ + should not be set at the same time", + ) + .run(); + } +} From b4c374039f2afcb0d2e2c13137ee1824ff487f56 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Fri, 6 Mar 2020 15:05:12 -0800 Subject: [PATCH 4/5] Do not implicitly load registry.token with --index. The intent is to avoid leaking the crates.io token to other servers. --- src/cargo/ops/registry.rs | 24 +++++-- tests/testsuite/alt_registry.rs | 13 ++-- tests/testsuite/cargo_features.rs | 3 +- tests/testsuite/cross_publish.rs | 3 +- tests/testsuite/owner.rs | 12 ++-- tests/testsuite/publish.rs | 113 ++++++++++++++++-------------- tests/testsuite/registry.rs | 5 +- tests/testsuite/yank.rs | 11 ++- 8 files changed, 98 insertions(+), 86 deletions(-) diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 68d605a436d..d5c635cc0f3 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -378,7 +378,26 @@ fn registry( token: token_config, index: index_config, } = registry_configuration(config, registry.clone())?; - let token = token.or(token_config); + let token = match (&index, &token, &token_config) { + // No token. + (None, None, None) => { + if validate_token { + bail!("no upload token found, please run `cargo login` or pass `--token`"); + } + None + } + // Token on command-line. + (_, Some(_), _) => token, + // Token in config, no --index, loading from config is OK for crates.io. + (None, None, Some(_)) => token_config, + // --index, no --token + (Some(_), None, _) => { + if validate_token { + bail!("command-line argument --index requires --token to be specified") + } + None + } + }; let sid = get_source_id(config, index_config.or(index), registry)?; if !sid.is_remote_registry() { bail!( @@ -408,9 +427,6 @@ fn registry( .ok_or_else(|| format_err!("{} does not support API commands", sid))? }; let handle = http_handle(config)?; - if validate_token && token.is_none() { - bail!("no upload token found, please run `cargo login`"); - }; Ok((Registry::new_handle(api_host, token, handle), sid)) } diff --git a/tests/testsuite/alt_registry.rs b/tests/testsuite/alt_registry.rs index 3c76d7d6ad1..f9f5b04fc35 100644 --- a/tests/testsuite/alt_registry.rs +++ b/tests/testsuite/alt_registry.rs @@ -298,7 +298,7 @@ fn cannot_publish_to_crates_io_with_registry_dependency() { .with_stderr_contains("[ERROR] crates cannot be published to crates.io[..]") .run(); - p.cargo("publish --index") + p.cargo("publish --token sekrit --index") .arg(fakeio_url.to_string()) .with_status(101) .with_stderr_contains("[ERROR] crates cannot be published to crates.io[..]") @@ -413,17 +413,18 @@ fn alt_registry_and_crates_io_deps() { #[cargo_test] fn block_publish_due_to_no_token() { - let p = project().file("src/main.rs", "fn main() {}").build(); - - // Setup the registry by publishing a package - Package::new("bar", "0.0.1").alternative(true).publish(); + registry::init(); + let p = project().file("src/lib.rs", "").build(); fs::remove_file(paths::home().join(".cargo/credentials")).unwrap(); // Now perform the actual publish p.cargo("publish --registry alternative") .with_status(101) - .with_stderr_contains("error: no upload token found, please run `cargo login`") + .with_stderr_contains( + "error: no upload token found, \ + please run `cargo login` or pass `--token`", + ) .run(); } diff --git a/tests/testsuite/cargo_features.rs b/tests/testsuite/cargo_features.rs index 7395a716e96..eca92515804 100644 --- a/tests/testsuite/cargo_features.rs +++ b/tests/testsuite/cargo_features.rs @@ -323,8 +323,7 @@ fn publish_allowed() { ) .file("src/lib.rs", "") .build(); - p.cargo("publish --index") - .arg(registry::registry_url().to_string()) + p.cargo("publish --token sekrit") .masquerade_as_nightly_cargo() .run(); } diff --git a/tests/testsuite/cross_publish.rs b/tests/testsuite/cross_publish.rs index bd6d2a9a356..e46437f9d69 100644 --- a/tests/testsuite/cross_publish.rs +++ b/tests/testsuite/cross_publish.rs @@ -97,8 +97,7 @@ fn publish_with_target() { let target = cross_compile::alternate(); - p.cargo("publish --index") - .arg(registry::registry_url().to_string()) + p.cargo("publish --token sekrit") .arg("--target") .arg(&target) .with_stderr(&format!( diff --git a/tests/testsuite/owner.rs b/tests/testsuite/owner.rs index 6c03a642814..8f3f87260ed 100644 --- a/tests/testsuite/owner.rs +++ b/tests/testsuite/owner.rs @@ -4,7 +4,7 @@ use std::fs; use cargo_test_support::paths::CargoPathExt; use cargo_test_support::project; -use cargo_test_support::registry::{self, api_path, registry_url}; +use cargo_test_support::registry::{self, api_path}; fn setup(name: &str, content: Option<&str>) { let dir = api_path().join(format!("api/v1/crates/{}", name)); @@ -43,9 +43,7 @@ fn simple_list() { .file("src/main.rs", "fn main() {}") .build(); - p.cargo("owner -l --index") - .arg(registry_url().to_string()) - .run(); + p.cargo("owner -l --token sekrit").run(); } #[cargo_test] @@ -68,8 +66,7 @@ fn simple_add() { .file("src/main.rs", "fn main() {}") .build(); - p.cargo("owner -a username --index") - .arg(registry_url().to_string()) + p.cargo("owner -a username --token sekrit") .with_status(101) .with_stderr( " Updating `[..]` index @@ -98,8 +95,7 @@ fn simple_remove() { .file("src/main.rs", "fn main() {}") .build(); - p.cargo("owner -r username --index") - .arg(registry_url().to_string()) + p.cargo("owner -r username --token sekrit") .with_status(101) .with_stderr( " Updating `[..]` index diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index 7f4d1ad1854..ae90b7af4f3 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -89,8 +89,7 @@ fn simple() { .file("src/main.rs", "fn main() {}") .build(); - p.cargo("publish --no-verify --index") - .arg(registry_url().to_string()) + p.cargo("publish --no-verify --token sekrit") .with_stderr(&format!( "\ [UPDATING] `{reg}` index @@ -131,16 +130,17 @@ fn old_token_location() { fs::remove_file(&credentials).unwrap(); // Verify can't publish without a token. - p.cargo("publish --no-verify --index") - .arg(registry_url().to_string()) + p.cargo("publish --no-verify") .with_status(101) - .with_stderr_contains("[ERROR] no upload token found, please run `cargo login`") + .with_stderr_contains( + "[ERROR] no upload token found, \ + please run `cargo login` or pass `--token`", + ) .run(); fs::write(&credentials, r#"token = "api-token""#).unwrap(); - p.cargo("publish --no-verify --index") - .arg(registry_url().to_string()) + p.cargo("publish --no-verify") .with_stderr(&format!( "\ [UPDATING] `{reg}` index @@ -177,7 +177,7 @@ fn simple_with_host() { .file("src/main.rs", "fn main() {}") .build(); - p.cargo("publish --no-verify --host") + p.cargo("publish --no-verify --token sekrit --host") .arg(registry_url().to_string()) .with_stderr(&format!( "\ @@ -224,7 +224,7 @@ fn simple_with_index_and_host() { .file("src/main.rs", "fn main() {}") .build(); - p.cargo("publish --no-verify --index") + p.cargo("publish --no-verify --token sekrit --index") .arg(registry_url().to_string()) .arg("--host") .arg(registry_url().to_string()) @@ -274,8 +274,7 @@ fn git_deps() { .file("src/main.rs", "fn main() {}") .build(); - p.cargo("publish -v --no-verify --index") - .arg(registry_url().to_string()) + p.cargo("publish -v --no-verify --token sekrit") .with_status(101) .with_stderr( "\ @@ -313,8 +312,7 @@ fn path_dependency_no_version() { .file("bar/src/lib.rs", "") .build(); - p.cargo("publish --index") - .arg(registry_url().to_string()) + p.cargo("publish --token sekrit") .with_status(101) .with_stderr( "\ @@ -383,8 +381,7 @@ fn dont_publish_dirty() { .file("src/main.rs", "fn main() {}") .build(); - p.cargo("publish --index") - .arg(registry_url().to_string()) + p.cargo("publish --token sekrit") .with_status(101) .with_stderr( "\ @@ -424,9 +421,7 @@ fn publish_clean() { .file("src/main.rs", "fn main() {}") .build(); - p.cargo("publish --index") - .arg(registry_url().to_string()) - .run(); + p.cargo("publish --token sekrit").run(); validate_upload_foo_clean(); } @@ -455,11 +450,7 @@ fn publish_in_sub_repo() { .file("bar/src/main.rs", "fn main() {}") .build(); - p.cargo("publish") - .cwd("bar") - .arg("--index") - .arg(registry_url().to_string()) - .run(); + p.cargo("publish --token sekrit").cwd("bar").run(); validate_upload_foo_clean(); } @@ -489,9 +480,7 @@ fn publish_when_ignored() { .file(".gitignore", "baz") .build(); - p.cargo("publish --index") - .arg(registry_url().to_string()) - .run(); + p.cargo("publish --token sekrit").run(); publish::validate_upload( CLEAN_FOO_JSON, @@ -530,11 +519,7 @@ fn ignore_when_crate_ignored() { "#, ) .nocommit_file("bar/src/main.rs", "fn main() {}"); - p.cargo("publish") - .cwd("bar") - .arg("--index") - .arg(registry_url().to_string()) - .run(); + p.cargo("publish --token sekrit").cwd("bar").run(); publish::validate_upload( CLEAN_FOO_JSON, @@ -571,8 +556,7 @@ fn new_crate_rejected() { "#, ) .nocommit_file("src/main.rs", "fn main() {}"); - p.cargo("publish --index") - .arg(registry_url().to_string()) + p.cargo("publish --token sekrit") .with_status(101) .with_stderr_contains( "[ERROR] 3 files in the working directory contain \ @@ -821,8 +805,7 @@ fn publish_with_select_features() { ) .build(); - p.cargo("publish --features required --index") - .arg(registry_url().to_string()) + p.cargo("publish --features required --token sekrit") .with_stderr_contains("[UPLOADING] foo v0.0.1 ([CWD])") .run(); } @@ -855,8 +838,7 @@ fn publish_with_all_features() { ) .build(); - p.cargo("publish --all-features --index") - .arg(registry_url().to_string()) + p.cargo("publish --all-features --token sekrit") .with_stderr_contains("[UPLOADING] foo v0.0.1 ([CWD])") .run(); } @@ -889,8 +871,7 @@ fn publish_with_no_default_features() { ) .build(); - p.cargo("publish --no-default-features --index") - .arg(registry_url().to_string()) + p.cargo("publish --no-default-features --token sekrit") .with_stderr_contains("error: This crate requires `required` feature!") .with_status(101) .run(); @@ -931,8 +912,7 @@ fn publish_with_patch() { p.cargo("build").run(); // Check that verify fails with patched crate which has new functionality. - p.cargo("publish --index") - .arg(registry_url().to_string()) + p.cargo("publish --token sekrit") .with_stderr_contains("[..]newfunc[..]") .with_status(101) .run(); @@ -940,9 +920,7 @@ fn publish_with_patch() { // Remove the usage of new functionality and try again. p.change_file("src/main.rs", "extern crate bar; pub fn main() {}"); - p.cargo("publish --index") - .arg(registry_url().to_string()) - .run(); + p.cargo("publish --token sekrit").run(); // Note, use of `registry` in the deps here is an artifact that this // publishes to a fake, local registry that is pretending to be crates.io. @@ -1010,7 +988,10 @@ fn publish_checks_for_token_before_verify() { // Assert upload token error before the package is verified p.cargo("publish") .with_status(101) - .with_stderr_contains("[ERROR] no upload token found, please run `cargo login`") + .with_stderr_contains( + "[ERROR] no upload token found, \ + please run `cargo login` or pass `--token`", + ) .with_stderr_does_not_contain("[VERIFYING] foo v0.0.1 ([CWD])") .run(); @@ -1037,7 +1018,7 @@ fn publish_with_bad_source() { .file("src/lib.rs", "") .build(); - p.cargo("publish") + p.cargo("publish --token sekrit") .with_status(101) .with_stderr( "\ @@ -1058,7 +1039,7 @@ Check for a source-replacement in .cargo/config. "#, ); - p.cargo("publish") + p.cargo("publish --token sekrit") .with_status(101) .with_stderr( "\ @@ -1112,9 +1093,7 @@ fn publish_git_with_version() { .build(); p.cargo("run").with_stdout("2").run(); - p.cargo("publish --no-verify --index") - .arg(registry_url().to_string()) - .run(); + p.cargo("publish --no-verify --token sekrit").run(); publish::validate_upload_with_contents( r#" @@ -1203,8 +1182,7 @@ fn publish_dev_dep_no_version() { .file("bar/src/lib.rs", "") .build(); - p.cargo("publish --no-verify --index") - .arg(registry_url().to_string()) + p.cargo("publish --no-verify --token sekrit") .with_stderr( "\ [UPDATING] [..] @@ -1279,8 +1257,7 @@ fn credentials_ambiguous_filename() { .file("src/main.rs", "fn main() {}") .build(); - p.cargo("publish --no-verify --index") - .arg(registry_url().to_string()) + p.cargo("publish --no-verify --token sekrit") .with_stderr_contains( "\ [WARNING] Both `[..]/credentials` and `[..]/credentials.toml` exist. Using `[..]/credentials` @@ -1290,3 +1267,31 @@ fn credentials_ambiguous_filename() { validate_upload_foo(); } + +#[cargo_test] +fn index_requires_token() { + // --index will not load registry.token to avoid possibly leaking + // crates.io token to another server. + registry::init(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + license = "MIT" + description = "foo" + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("publish --no-verify --index") + .arg(registry_url().to_string()) + .with_status(101) + .with_stderr("[ERROR] command-line argument --index requires --token to be specified") + .run(); +} diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index 84f08786026..40da268f892 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -4,7 +4,7 @@ use cargo::util::paths::remove_dir_all; use cargo_test_support::cargo_process; use cargo_test_support::git; use cargo_test_support::paths::{self, CargoPathExt}; -use cargo_test_support::registry::{self, registry_path, registry_url, Dependency, Package}; +use cargo_test_support::registry::{self, registry_path, Dependency, Package}; use cargo_test_support::{basic_manifest, project, t}; use std::fs::{self, File}; use std::path::Path; @@ -896,8 +896,7 @@ fn bad_license_file() { ) .file("src/main.rs", "fn main() {}") .build(); - p.cargo("publish -v --index") - .arg(registry_url().to_string()) + p.cargo("publish -v --token sekrit") .with_status(101) .with_stderr_contains("[ERROR] the license file `foo` does not exist") .run(); diff --git a/tests/testsuite/yank.rs b/tests/testsuite/yank.rs index c697825320d..10462145d0f 100644 --- a/tests/testsuite/yank.rs +++ b/tests/testsuite/yank.rs @@ -4,10 +4,10 @@ use std::fs; use cargo_test_support::paths::CargoPathExt; use cargo_test_support::project; -use cargo_test_support::registry::{self, api_path, registry_url}; +use cargo_test_support::registry; fn setup(name: &str, version: &str) { - let dir = api_path().join(format!("api/v1/crates/{}/{}", name, version)); + let dir = registry::api_path().join(format!("api/v1/crates/{}/{}", name, version)); dir.mkdir_p(); fs::write(dir.join("yank"), r#"{"ok": true}"#).unwrap(); } @@ -32,12 +32,9 @@ fn simple() { .file("src/main.rs", "fn main() {}") .build(); - p.cargo("yank --vers 0.0.1 --index") - .arg(registry_url().to_string()) - .run(); + p.cargo("yank --vers 0.0.1 --token sekrit").run(); - p.cargo("yank --undo --vers 0.0.1 --index") - .arg(registry_url().to_string()) + p.cargo("yank --undo --vers 0.0.1 --token sekrit") .with_status(101) .with_stderr( " Updating `[..]` index From 65274ea7d5cc02d494a8d61986a2c4d4749c8cd9 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Fri, 6 Mar 2020 17:29:12 -0800 Subject: [PATCH 5/5] Add a warning when using `registry.token` with source replacement. --- Cargo.toml | 2 +- crates/crates-io/Cargo.toml | 2 +- crates/crates-io/lib.rs | 11 ++++-- src/cargo/ops/registry.rs | 73 +++++++++++++++++++++++++------------ tests/testsuite/publish.rs | 48 +++++++++++++++++++++++- 5 files changed, 106 insertions(+), 30 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3a7c372d65e..acec911029b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,7 +22,7 @@ path = "src/cargo/lib.rs" atty = "0.2" bytesize = "1.0" cargo-platform = { path = "crates/cargo-platform", version = "0.1.1" } -crates-io = { path = "crates/crates-io", version = "0.31" } +crates-io = { path = "crates/crates-io", version = "0.31.1" } crossbeam-utils = "0.7" crypto-hash = "0.3.1" curl = { version = "0.4.23", features = ["http2"] } diff --git a/crates/crates-io/Cargo.toml b/crates/crates-io/Cargo.toml index a4f93c8c0ff..3e24d4cdc85 100644 --- a/crates/crates-io/Cargo.toml +++ b/crates/crates-io/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "crates-io" -version = "0.31.0" +version = "0.31.1" edition = "2018" authors = ["Alex Crichton "] license = "MIT OR Apache-2.0" diff --git a/crates/crates-io/lib.rs b/crates/crates-io/lib.rs index 29bbd554639..dc5a9878e4e 100644 --- a/crates/crates-io/lib.rs +++ b/crates/crates-io/lib.rs @@ -139,9 +139,7 @@ impl Registry { } pub fn host_is_crates_io(&self) -> bool { - Url::parse(self.host()) - .map(|u| u.host_str() == Some("crates.io")) - .unwrap_or(false) + is_url_crates_io(&self.host) } pub fn add_owners(&mut self, krate: &str, owners: &[&str]) -> Result { @@ -420,3 +418,10 @@ fn reason(code: u32) -> &'static str { _ => "", } } + +/// Returns `true` if the host of the given URL is "crates.io". +pub fn is_url_crates_io(url: &str) -> bool { + Url::parse(url) + .map(|u| u.host_str() == Some("crates.io")) + .unwrap_or(false) +} diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index d5c635cc0f3..69e778f155d 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -7,7 +7,7 @@ use std::time::Duration; use std::{cmp, env}; use anyhow::{bail, format_err}; -use crates_io::{NewCrate, NewCrateDependency, Registry}; +use crates_io::{self, NewCrate, NewCrateDependency, Registry}; use curl::easy::{Easy, InfoType, SslOpt, SslVersion}; use log::{log, Level}; use percent_encoding::{percent_encode, NON_ALPHANUMERIC}; @@ -378,27 +378,8 @@ fn registry( token: token_config, index: index_config, } = registry_configuration(config, registry.clone())?; - let token = match (&index, &token, &token_config) { - // No token. - (None, None, None) => { - if validate_token { - bail!("no upload token found, please run `cargo login` or pass `--token`"); - } - None - } - // Token on command-line. - (_, Some(_), _) => token, - // Token in config, no --index, loading from config is OK for crates.io. - (None, None, Some(_)) => token_config, - // --index, no --token - (Some(_), None, _) => { - if validate_token { - bail!("command-line argument --index requires --token to be specified") - } - None - } - }; - let sid = get_source_id(config, index_config.or(index), registry)?; + let opt_index = index_config.as_ref().or(index.as_ref()); + let sid = get_source_id(config, opt_index, registry.as_ref())?; if !sid.is_remote_registry() { bail!( "{} does not support API commands.\n\ @@ -426,6 +407,50 @@ fn registry( cfg.and_then(|cfg| cfg.api) .ok_or_else(|| format_err!("{} does not support API commands", sid))? }; + let token = match (&index, &token, &token_config) { + // No token. + (None, None, None) => { + if validate_token { + bail!("no upload token found, please run `cargo login` or pass `--token`"); + } + None + } + // Token on command-line. + (_, Some(_), _) => token, + // Token in config, no --index, loading from config is OK for crates.io. + (None, None, Some(_)) => { + // Check `is_default_registry` so that the crates.io index can + // change config.json's "api" value, and this won't affect most + // people. It will affect those using source replacement, but + // hopefully that's a relatively small set of users. + if registry.is_none() + && !sid.is_default_registry() + && !crates_io::is_url_crates_io(&api_host) + { + if validate_token { + config.shell().warn( + "using `registry.token` config value with source \ + replacement is deprecated\n\ + This may become a hard error in the future; \ + see .\n\ + Use the --token command-line flag to remove this warning.", + )?; + token_config + } else { + None + } + } else { + token_config + } + } + // --index, no --token + (Some(_), None, _) => { + if validate_token { + bail!("command-line argument --index requires --token to be specified") + } + None + } + }; let handle = http_handle(config)?; Ok((Registry::new_handle(api_host, token, handle), sid)) } @@ -782,8 +807,8 @@ pub fn yank( /// If both are None, returns the source for crates.io. fn get_source_id( config: &Config, - index: Option, - reg: Option, + index: Option<&String>, + reg: Option<&String>, ) -> CargoResult { match (reg, index) { (Some(r), _) => SourceId::alt_registry(config, &r), diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index ae90b7af4f3..1632be5a02f 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -144,6 +144,9 @@ fn old_token_location() { .with_stderr(&format!( "\ [UPDATING] `{reg}` index +[WARNING] using `registry.token` config value with source replacement is deprecated +This may become a hard error in the future[..] +Use the --token command-line flag to remove this warning. [WARNING] manifest has no documentation, [..] See [..] [PACKAGING] foo v0.0.1 ([CWD]) @@ -1273,6 +1276,8 @@ fn index_requires_token() { // --index will not load registry.token to avoid possibly leaking // crates.io token to another server. registry::init(); + let credentials = paths::home().join(".cargo/credentials"); + fs::remove_file(&credentials).unwrap(); let p = project() .file( @@ -1292,6 +1297,47 @@ fn index_requires_token() { p.cargo("publish --no-verify --index") .arg(registry_url().to_string()) .with_status(101) - .with_stderr("[ERROR] command-line argument --index requires --token to be specified") + .with_stderr( + "\ +[UPDATING] [..] +[ERROR] command-line argument --index requires --token to be specified +", + ) + .run(); +} + +#[cargo_test] +fn registry_token_with_source_replacement() { + // publish with source replacement without --token + registry::init(); + + let p = project() + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + license = "MIT" + description = "foo" + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("publish --no-verify") + .with_stderr( + "\ +[UPDATING] [..] +[WARNING] using `registry.token` config value with source replacement is deprecated +This may become a hard error in the future[..] +Use the --token command-line flag to remove this warning. +[WARNING] manifest has no documentation, [..] +See [..] +[PACKAGING] foo v0.0.1 ([CWD]) +[UPLOADING] foo v0.0.1 ([CWD]) +", + ) .run(); }