From 6cbde6e2bff28866aa0ebacc6de8826830682b02 Mon Sep 17 00:00:00 2001 From: Takayuki Nakata Date: Fri, 13 Dec 2019 21:54:07 +0900 Subject: [PATCH 1/6] Fix overwriting alternate registry token When executing `cargo login`, 2nd alternate registry token overwrites 1st alternate registry token. Fixes #7701. --- crates/cargo-test-support/src/registry.rs | 43 ++++++++++++++++++++++- src/cargo/util/config/mod.rs | 10 ++++-- tests/testsuite/login.rs | 15 ++++++++ 3 files changed, 65 insertions(+), 3 deletions(-) diff --git a/crates/cargo-test-support/src/registry.rs b/crates/cargo-test-support/src/registry.rs index 88b24672f99..cb4ff8fc394 100644 --- a/crates/cargo-test-support/src/registry.rs +++ b/crates/cargo-test-support/src/registry.rs @@ -44,6 +44,12 @@ pub fn alt_registry_path() -> PathBuf { pub fn alt_registry_url() -> Url { Url::from_file_path(alt_registry_path()).ok().unwrap() } +pub fn alt2_registry_path() -> PathBuf { + paths::root().join("alternative2-registry") +} +pub fn alt2_registry_url() -> Url { + Url::from_file_path(alt_registry_path()).ok().unwrap() +} /// Gets the alternative-registry version of `dl_path`. pub fn alt_dl_path() -> PathBuf { paths::root().join("alt_dl") @@ -52,6 +58,13 @@ pub fn alt_dl_url() -> String { let base = Url::from_file_path(alt_dl_path()).ok().unwrap(); format!("{}/{{crate}}/{{version}}/{{crate}}-{{version}}.crate", base) } +pub fn alt2_dl_path() -> PathBuf { + paths::root().join("alt2_dl") +} +pub fn alt2_dl_url() -> String { + let base = Url::from_file_path(alt2_dl_path()).ok().unwrap(); + format!("{}/{{crate}}/{{version}}/{{crate}}-{{version}}.crate", base) +} /// Gets the alternative-registry version of `api_path`. pub fn alt_api_path() -> PathBuf { paths::root().join("alt_api") @@ -59,6 +72,12 @@ pub fn alt_api_path() -> PathBuf { pub fn alt_api_url() -> Url { Url::from_file_path(alt_api_path()).ok().unwrap() } +pub fn alt2_api_path() -> PathBuf { + paths::root().join("alt2_api") +} +pub fn alt2_api_url() -> Url { + Url::from_file_path(alt2_api_path()).ok().unwrap() +} /// A builder for creating a new package in a registry. /// @@ -166,9 +185,13 @@ pub fn init() { [registries.alternative] index = '{alt}' + + [registries.alternative2] + index = '{alt2}' "#, reg = registry_url(), - alt = alt_registry_url() + alt = alt_registry_url(), + alt2 = alt2_registry_url() ) .as_bytes() )); @@ -180,6 +203,9 @@ pub fn init() { [registries.alternative] token = "api-token" + + [registries.alternative2] + token = "api-token" "# )); @@ -212,6 +238,21 @@ pub fn init() { ) .build(); fs::create_dir_all(alt_api_path().join("api/v1/crates")).unwrap(); + + // Initialize an alternative2 registry. + repo(&alt2_registry_path()) + .file( + "config.json", + &format!( + r#" + {{"dl":"{}","api":"{}"}} + "#, + alt2_dl_url(), + alt2_api_url() + ), + ) + .build(); + fs::create_dir_all(alt2_api_path().join("api/v1/crates")).unwrap(); } impl Package { diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index c8de80eef7d..89d7aec0775 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -1314,14 +1314,14 @@ pub fn save_credentials(cfg: &Config, token: String, registry: Option) - .open_rw(filename, cfg, "credentials' config file")? }; - let (key, value) = { + let (key, mut value) = { let key = "token".to_string(); let value = ConfigValue::String(token, file.path().to_path_buf()); let mut map = HashMap::new(); map.insert(key, value); let table = CV::Table(map, file.path().to_path_buf()); - if let Some(registry) = registry { + if let Some(registry) = registry.clone() { let mut map = HashMap::new(); map.insert(registry, table); ( @@ -1352,6 +1352,12 @@ pub fn save_credentials(cfg: &Config, token: String, registry: Option) - .insert("registry".into(), map.into()); } + if let Some(_) = registry { + if let Some(table) = toml.as_table_mut().unwrap().remove("registries") { + let v = CV::from_toml(file.path(), table)?; + value.merge(v)?; + } + } toml.as_table_mut().unwrap().insert(key, value.into_toml()); let contents = toml.to_string(); diff --git a/tests/testsuite/login.rs b/tests/testsuite/login.rs index 01fe7e556cb..4cfc72c8aee 100644 --- a/tests/testsuite/login.rs +++ b/tests/testsuite/login.rs @@ -12,6 +12,7 @@ use cargo_test_support::{cargo_process, t}; use toml; const TOKEN: &str = "test-token"; +const TOKEN2: &str = "test-token2"; const ORIGINAL_TOKEN: &str = "api-token"; fn setup_new_credentials() { @@ -185,4 +186,18 @@ fn registry_credentials() { // Also ensure that we get the new token for the registry assert!(check_token(TOKEN, Some(reg))); + + let reg2 = "alternative2"; + cargo_process("login --registry") + .arg(reg2) + .arg(TOKEN2) + .arg("-Zunstable-options") + .masquerade_as_nightly_cargo() + .run(); + + // Ensure not overwriting 1st alternate registry token with + // 2nd alternate registry token (see rust-lang/cargo#7701). + assert!(check_token(ORIGINAL_TOKEN, None)); + assert!(check_token(TOKEN, Some(reg))); + assert!(check_token(TOKEN2, Some(reg2))); } From a718ed61d6b88d9240f1e188b4563354367e4790 Mon Sep 17 00:00:00 2001 From: Takayuki Nakata Date: Sat, 14 Dec 2019 12:42:00 +0900 Subject: [PATCH 2/6] Refactoring --- crates/cargo-test-support/src/registry.rs | 41 +++++++++-------------- tests/testsuite/login.rs | 1 + 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/crates/cargo-test-support/src/registry.rs b/crates/cargo-test-support/src/registry.rs index cb4ff8fc394..47b930447bb 100644 --- a/crates/cargo-test-support/src/registry.rs +++ b/crates/cargo-test-support/src/registry.rs @@ -44,12 +44,6 @@ pub fn alt_registry_path() -> PathBuf { pub fn alt_registry_url() -> Url { Url::from_file_path(alt_registry_path()).ok().unwrap() } -pub fn alt2_registry_path() -> PathBuf { - paths::root().join("alternative2-registry") -} -pub fn alt2_registry_url() -> Url { - Url::from_file_path(alt_registry_path()).ok().unwrap() -} /// Gets the alternative-registry version of `dl_path`. pub fn alt_dl_path() -> PathBuf { paths::root().join("alt_dl") @@ -58,13 +52,6 @@ pub fn alt_dl_url() -> String { let base = Url::from_file_path(alt_dl_path()).ok().unwrap(); format!("{}/{{crate}}/{{version}}/{{crate}}-{{version}}.crate", base) } -pub fn alt2_dl_path() -> PathBuf { - paths::root().join("alt2_dl") -} -pub fn alt2_dl_url() -> String { - let base = Url::from_file_path(alt2_dl_path()).ok().unwrap(); - format!("{}/{{crate}}/{{version}}/{{crate}}-{{version}}.crate", base) -} /// Gets the alternative-registry version of `api_path`. pub fn alt_api_path() -> PathBuf { paths::root().join("alt_api") @@ -72,11 +59,16 @@ pub fn alt_api_path() -> PathBuf { pub fn alt_api_url() -> Url { Url::from_file_path(alt_api_path()).ok().unwrap() } -pub fn alt2_api_path() -> PathBuf { - paths::root().join("alt2_api") + +fn generate_path(name: &str) -> PathBuf { + paths::root().join(name) } -pub fn alt2_api_url() -> Url { - Url::from_file_path(alt2_api_path()).ok().unwrap() +fn generate_url(name: &str) -> Url { + Url::from_file_path(generate_path(name)).ok().unwrap() +} +fn generate_dl_url(name: &str) -> String { + let base = Url::from_file_path(generate_path(name)).ok().unwrap(); + format!("{}/{{crate}}/{{version}}/{{crate}}-{{version}}.crate", base) } /// A builder for creating a new package in a registry. @@ -191,7 +183,7 @@ pub fn init() { "#, reg = registry_url(), alt = alt_registry_url(), - alt2 = alt2_registry_url() + alt2 = generate_url("alternative2-registry") ) .as_bytes() )); @@ -203,9 +195,6 @@ pub fn init() { [registries.alternative] token = "api-token" - - [registries.alternative2] - token = "api-token" "# )); @@ -238,21 +227,23 @@ pub fn init() { ) .build(); fs::create_dir_all(alt_api_path().join("api/v1/crates")).unwrap(); +} +pub fn init_alt2_registry() { // Initialize an alternative2 registry. - repo(&alt2_registry_path()) + repo(&generate_path("alternative2-registry")) .file( "config.json", &format!( r#" {{"dl":"{}","api":"{}"}} "#, - alt2_dl_url(), - alt2_api_url() + generate_dl_url("alt2_dl"), + generate_url("alt2_api") ), ) .build(); - fs::create_dir_all(alt2_api_path().join("api/v1/crates")).unwrap(); + fs::create_dir_all(generate_path("alt2_api").join("api/v1/crates")).unwrap(); } impl Package { diff --git a/tests/testsuite/login.rs b/tests/testsuite/login.rs index 4cfc72c8aee..0f3d3b4dbb6 100644 --- a/tests/testsuite/login.rs +++ b/tests/testsuite/login.rs @@ -170,6 +170,7 @@ fn new_credentials_is_used_instead_old() { #[cargo_test] fn registry_credentials() { registry::init(); + registry::init_alt2_registry(); setup_new_credentials(); let reg = "alternative"; From 75d06de80c318ddbec80fd708cf29b27f7eaec4d Mon Sep 17 00:00:00 2001 From: Takayuki Nakata Date: Tue, 17 Dec 2019 08:33:48 +0900 Subject: [PATCH 3/6] Remove unneeded flags --- tests/testsuite/login.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/tests/testsuite/login.rs b/tests/testsuite/login.rs index 0f3d3b4dbb6..cb609feaf9d 100644 --- a/tests/testsuite/login.rs +++ b/tests/testsuite/login.rs @@ -175,12 +175,7 @@ fn registry_credentials() { let reg = "alternative"; - cargo_process("login --registry") - .arg(reg) - .arg(TOKEN) - .arg("-Zunstable-options") - .masquerade_as_nightly_cargo() - .run(); + cargo_process("login --registry").arg(reg).arg(TOKEN).run(); // Ensure that we have not updated the default token assert!(check_token(ORIGINAL_TOKEN, None)); @@ -192,8 +187,6 @@ fn registry_credentials() { cargo_process("login --registry") .arg(reg2) .arg(TOKEN2) - .arg("-Zunstable-options") - .masquerade_as_nightly_cargo() .run(); // Ensure not overwriting 1st alternate registry token with From 2a4d1dc40aac70568d9b6bbf7116c815c1187ec0 Mon Sep 17 00:00:00 2001 From: Takayuki Nakata Date: Tue, 17 Dec 2019 09:59:45 +0900 Subject: [PATCH 4/6] Append `registries` to config --- crates/cargo-test-support/src/registry.rs | 8 ++------ tests/testsuite/login.rs | 18 ++++++++++++++++-- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/crates/cargo-test-support/src/registry.rs b/crates/cargo-test-support/src/registry.rs index 47b930447bb..09d6414acae 100644 --- a/crates/cargo-test-support/src/registry.rs +++ b/crates/cargo-test-support/src/registry.rs @@ -63,7 +63,7 @@ pub fn alt_api_url() -> Url { fn generate_path(name: &str) -> PathBuf { paths::root().join(name) } -fn generate_url(name: &str) -> Url { +pub fn generate_url(name: &str) -> Url { Url::from_file_path(generate_path(name)).ok().unwrap() } fn generate_dl_url(name: &str) -> String { @@ -177,13 +177,9 @@ pub fn init() { [registries.alternative] index = '{alt}' - - [registries.alternative2] - index = '{alt2}' "#, reg = registry_url(), - alt = alt_registry_url(), - alt2 = generate_url("alternative2-registry") + alt = alt_registry_url() ) .as_bytes() )); diff --git a/tests/testsuite/login.rs b/tests/testsuite/login.rs index cb609feaf9d..86c0c56c6b4 100644 --- a/tests/testsuite/login.rs +++ b/tests/testsuite/login.rs @@ -1,6 +1,6 @@ //! Tests for the `cargo login` command. -use std::fs::{self, File}; +use std::fs::{self, File, OpenOptions}; use std::io::prelude::*; use std::path::PathBuf; @@ -8,7 +8,7 @@ use cargo::core::Shell; use cargo::util::config::Config; use cargo_test_support::install::cargo_home; use cargo_test_support::registry::{self, registry_url}; -use cargo_test_support::{cargo_process, t}; +use cargo_test_support::{cargo_process, paths, t}; use toml; const TOKEN: &str = "test-token"; @@ -170,6 +170,20 @@ fn new_credentials_is_used_instead_old() { #[cargo_test] fn registry_credentials() { registry::init(); + + let config = paths::home().join(".cargo/config"); + let mut f = OpenOptions::new().append(true).open(config).unwrap(); + t!(f.write_all( + format!( + r#" + [registries.alternative2] + index = '{}' + "#, + registry::generate_url("alternative2-registry") + ) + .as_bytes(), + )); + registry::init_alt2_registry(); setup_new_credentials(); From 3613999aca04fd176921311a8545d9bb1f724dca Mon Sep 17 00:00:00 2001 From: Takayuki Nakata Date: Tue, 17 Dec 2019 12:31:01 +0900 Subject: [PATCH 5/6] Refactoring to call new `generate_*` functions instead of duplicating codes --- crates/cargo-test-support/src/registry.rs | 31 +++++++++++------------ 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/crates/cargo-test-support/src/registry.rs b/crates/cargo-test-support/src/registry.rs index 09d6414acae..8388275fc39 100644 --- a/crates/cargo-test-support/src/registry.rs +++ b/crates/cargo-test-support/src/registry.rs @@ -15,58 +15,57 @@ use url::Url; /// initialized with a `config.json` file pointing to `dl_path` for downloads /// and `api_path` for uploads. pub fn registry_path() -> PathBuf { - paths::root().join("registry") + generate_path("registry") } pub fn registry_url() -> Url { - Url::from_file_path(registry_path()).ok().unwrap() + generate_url("registry") } /// Gets the path for local web API uploads. Cargo will place the contents of a web API /// request here. For example, `api/v1/crates/new` is the result of publishing a crate. pub fn api_path() -> PathBuf { - paths::root().join("api") + generate_path("api") } pub fn api_url() -> Url { - Url::from_file_path(api_path()).ok().unwrap() + generate_url("api") } /// Gets the path where crates can be downloaded using the web API endpoint. Crates /// should be organized as `{name}/{version}/download` to match the web API /// endpoint. This is rarely used and must be manually set up. pub fn dl_path() -> PathBuf { - paths::root().join("dl") + generate_path("dl") } pub fn dl_url() -> Url { - Url::from_file_path(dl_path()).ok().unwrap() + generate_url("dl") } /// Gets the alternative-registry version of `registry_path`. pub fn alt_registry_path() -> PathBuf { - paths::root().join("alternative-registry") + generate_path("alternative-registry") } pub fn alt_registry_url() -> Url { - Url::from_file_path(alt_registry_path()).ok().unwrap() + generate_url("alternative-registry") } /// Gets the alternative-registry version of `dl_path`. pub fn alt_dl_path() -> PathBuf { - paths::root().join("alt_dl") + generate_path("alt_dl") } pub fn alt_dl_url() -> String { - let base = Url::from_file_path(alt_dl_path()).ok().unwrap(); - format!("{}/{{crate}}/{{version}}/{{crate}}-{{version}}.crate", base) + generate_alt_dl_url("alt_dl") } /// Gets the alternative-registry version of `api_path`. pub fn alt_api_path() -> PathBuf { - paths::root().join("alt_api") + generate_path("alt_api") } pub fn alt_api_url() -> Url { - Url::from_file_path(alt_api_path()).ok().unwrap() + generate_url("alt_api") } -fn generate_path(name: &str) -> PathBuf { +pub fn generate_path(name: &str) -> PathBuf { paths::root().join(name) } pub fn generate_url(name: &str) -> Url { Url::from_file_path(generate_path(name)).ok().unwrap() } -fn generate_dl_url(name: &str) -> String { +pub fn generate_alt_dl_url(name: &str) -> String { let base = Url::from_file_path(generate_path(name)).ok().unwrap(); format!("{}/{{crate}}/{{version}}/{{crate}}-{{version}}.crate", base) } @@ -234,7 +233,7 @@ pub fn init_alt2_registry() { r#" {{"dl":"{}","api":"{}"}} "#, - generate_dl_url("alt2_dl"), + generate_alt_dl_url("alt2_dl"), generate_url("alt2_api") ), ) From b7bc069fbb3fda9226dc3d6e0484f6469caf34bf Mon Sep 17 00:00:00 2001 From: Takayuki Nakata Date: Tue, 17 Dec 2019 14:12:27 +0900 Subject: [PATCH 6/6] Refactoring of creating registry --- crates/cargo-test-support/src/registry.rs | 49 ++++++++--------------- tests/testsuite/login.rs | 7 +++- 2 files changed, 23 insertions(+), 33 deletions(-) diff --git a/crates/cargo-test-support/src/registry.rs b/crates/cargo-test-support/src/registry.rs index 8388275fc39..ba4173afdd0 100644 --- a/crates/cargo-test-support/src/registry.rs +++ b/crates/cargo-test-support/src/registry.rs @@ -194,51 +194,36 @@ pub fn init() { )); // Initialize a new registry. - let _ = repo(®istry_path()) - .file( - "config.json", - &format!( - r#" - {{"dl":"{}","api":"{}"}} - "#, - dl_url(), - api_url() - ), - ) - .build(); - fs::create_dir_all(api_path().join("api/v1/crates")).unwrap(); + init_registry( + registry_path(), + dl_url().into_string(), + api_url(), + api_path(), + ); // Initialize an alternative registry. - repo(&alt_registry_path()) - .file( - "config.json", - &format!( - r#" - {{"dl":"{}","api":"{}"}} - "#, - alt_dl_url(), - alt_api_url() - ), - ) - .build(); - fs::create_dir_all(alt_api_path().join("api/v1/crates")).unwrap(); + init_registry( + alt_registry_path(), + alt_dl_url(), + alt_api_url(), + alt_api_path(), + ); } -pub fn init_alt2_registry() { - // Initialize an alternative2 registry. - repo(&generate_path("alternative2-registry")) +pub fn init_registry(registry_path: PathBuf, dl_url: String, api_url: Url, api_path: PathBuf) { + // Initialize a new registry. + repo(®istry_path) .file( "config.json", &format!( r#" {{"dl":"{}","api":"{}"}} "#, - generate_alt_dl_url("alt2_dl"), - generate_url("alt2_api") + dl_url, api_url ), ) .build(); - fs::create_dir_all(generate_path("alt2_api").join("api/v1/crates")).unwrap(); + fs::create_dir_all(api_path.join("api/v1/crates")).unwrap(); } impl Package { diff --git a/tests/testsuite/login.rs b/tests/testsuite/login.rs index 86c0c56c6b4..461f1b155ae 100644 --- a/tests/testsuite/login.rs +++ b/tests/testsuite/login.rs @@ -184,7 +184,12 @@ fn registry_credentials() { .as_bytes(), )); - registry::init_alt2_registry(); + registry::init_registry( + registry::generate_path("alternative2-registry"), + registry::generate_alt_dl_url("alt2_dl"), + registry::generate_url("alt2_api"), + registry::generate_path("alt2_api"), + ); setup_new_credentials(); let reg = "alternative";