Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 2 additions & 0 deletions src/bin/cargo/commands/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ pub fn cli() -> App {
}

pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
config.load_credentials()?;

let registry = args.registry(config)?;
let ws = args.workspace(config)?;
let index = args.index(config)?;
Expand Down
29 changes: 24 additions & 5 deletions src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,6 @@ impl Config {
})
.chain_err(|| "could not load Cargo configuration")?;

self.load_credentials(&mut cfg)?;
match cfg {
CV::Table(map, _) => Ok(map),
_ => unreachable!(),
Expand Down Expand Up @@ -979,9 +978,8 @@ impl Config {
Ok(url)
}

/// Loads credentials config from the credentials file into the `ConfigValue` object, if
/// present.
fn load_credentials(&self, cfg: &mut ConfigValue) -> CargoResult<()> {
/// Loads credentials config from the credentials file, if present.
pub fn load_credentials(&mut self) -> CargoResult<()> {
let home_path = self.home_path.clone().into_path_unlocked();
let credentials = match self.get_file_path(&home_path, "credentials", true)? {
Some(credentials) => credentials,
Expand All @@ -1006,7 +1004,28 @@ impl Config {
}
}

cfg.merge(value, true)?;
if let CV::Table(mut map, _) = value {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this not use merge?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is modified not to receive cfg: &mut ConfigValue as an argument, so I think that merge can not be used. However, I may simply not know how to use merge in Config, so could you tell me if there is any good way?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. Would it be possible to just loop over the key/values of the map and merge/insert all of them instead of hard-coding the keys here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your advice! I fixed it.

if map.contains_key("registry") {
if let Some(mut new_map) = self.values_mut()?.remove("registry") {
let token = map.remove("registry").unwrap();
new_map.merge(token, true)?;
self.values_mut()?.insert("registry".into(), new_map);
} else {
self.values_mut()?
.insert("registry".into(), map.remove("registry").unwrap());
}
}
if map.contains_key("registries") {
if let Some(mut new_map) = self.values_mut()?.remove("registries") {
let token = map.remove("registries").unwrap();
new_map.merge(token, true)?;
self.values_mut()?.insert("registries".into(), new_map);
} else {
self.values_mut()?
.insert("registries".into(), map.remove("registries").unwrap());
}
}
}

Ok(())
}
Expand Down
26 changes: 26 additions & 0 deletions tests/testsuite/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2237,6 +2237,32 @@ fn recompile_space_in_name() {
foo.cargo("build").with_stdout("").run();
}

#[cfg(unix)]
#[cargo_test]
fn credentials_is_unreadable() {
use cargo_test_support::paths::home;
use std::os::unix::prelude::*;
let p = project()
.file("Cargo.toml", &basic_manifest("foo", "0.1.0"))
.file("src/lib.rs", "")
.build();

let credentials = home().join(".cargo/credentials");
t!(fs::create_dir_all(credentials.parent().unwrap()));
t!(t!(File::create(&credentials)).write_all(
br#"
[registry]
token = "api-token"
"#
));
let stat = fs::metadata(credentials.as_path()).unwrap();
let mut perms = stat.permissions();
perms.set_mode(0o000);
fs::set_permissions(credentials, perms.clone()).unwrap();

p.cargo("build").run();
}

#[cfg(unix)]
#[cargo_test]
fn ignore_bad_directories() {
Expand Down
26 changes: 3 additions & 23 deletions tests/testsuite/login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,28 +111,6 @@ fn credentials_work_with_extension() {
assert!(check_token(TOKEN, None));
}

#[cargo_test]
fn credentials_ambiguous_filename() {
registry::init();
setup_new_credentials();
setup_new_credentials_toml();

cargo_process("login --host")
.arg(registry_url().to_string())
.arg(TOKEN)
.with_stderr_contains(
"\
[WARNING] Both `[..]/credentials` and `[..]/credentials.toml` exist. Using `[..]/credentials`
",
)
.run();

// It should use the value from the one without the extension
// for backwards compatibility. check_token explicitly checks
// 'credentials' without the extension, which is what we want.
assert!(check_token(TOKEN, None));
}

#[cargo_test]
fn login_with_old_and_new_credentials() {
setup_new_credentials();
Expand Down Expand Up @@ -161,7 +139,9 @@ fn new_credentials_is_used_instead_old() {
.arg(TOKEN)
.run();

let config = Config::new(Shell::new(), cargo_home(), cargo_home());
let mut config = Config::new(Shell::new(), cargo_home(), cargo_home());
let _ = config.values();
let _ = config.load_credentials();

let token = config.get_string("registry.token").unwrap().map(|p| p.val);
assert_eq!(token.unwrap(), TOKEN);
Expand Down
37 changes: 37 additions & 0 deletions tests/testsuite/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1261,3 +1261,40 @@ repository = "foo"
)],
);
}

#[cargo_test]
fn credentials_ambiguous_filename() {
registry::init();

let credentials_toml = paths::home().join(".cargo/credentials.toml");
File::create(&credentials_toml)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fs::write here too.

.unwrap()
.write_all(br#"token = "api-token""#)
.unwrap();

let p = project()
.file(
"Cargo.toml",
r#"
[project]
name = "foo"
version = "0.0.1"
authors = []
license = "MIT"
description = "foo"
"#,
)
.file("src/main.rs", "fn main() {}")
.build();

p.cargo("publish --no-verify --index")
.arg(registry_url().to_string())
.with_stderr_contains(
"\
[WARNING] Both `[..]/credentials` and `[..]/credentials.toml` exist. Using `[..]/credentials`
",
)
.run();

validate_upload_foo();
}