fix(credential): trim newlines in tokens from stdin

[arlosi]

What does this PR try to resolve?

cargo login when using a credential provider other than cargo:token does not automatically trim whitespace from tokens.

This can lead to extra whitespace being included in the pasted token value (usually a trailing newline) that makes the token invalid.

How should we test and review this PR?

First commit adds a test showing the problematic behavior. Second commit fixes it.

[rustbot]

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[epage]

I know this is unlikely but is there any case where newlines could be significant to these tokens?

[arlosi]

I know this is unlikely but is there any case where newlines could be significant to these tokens?

The token is directly sent as the Authorization HTTP header, which doesn't allow newlines.

If you try to use it, you get:

cargo yank asdf --version 0.0.1
    Updating crates.io index
        Yank asdf@0.0.1
error: failed to yank from the registry at https://crates.io

Caused by:
  token contains invalid characters.
  Only printable ISO-8859-1 characters are allowed as it is sent in a HTTPS header.

[epage]

How is this test passing today?

[arlosi]

In the read_token function, if a token is provided by stdin or by the command line, then no prompt is made.

The change causes login_options.token to be None, instead of a Some("\n"), so Cargo now prints the "please paste" message in this case.
 

cargo/credential/cargo-credential/src/lib.rs

Lines 281 to 290 in 852a316

	pub fn read_token(
	login_options: &LoginOptions<'_>,
	registry: &RegistryInfo<'_>,
	) -> Result<Secret<String>, Error> {
	if let Some(token) = &login_options.token {
	return Ok(token.to_owned());
	}
	if let Some(url) = login_options.login_url {
	eprintln!("please paste the token found on {url} below");

[epage]

Could you move that change into the commit that caused it so that each commit passes tests?

[arlosi]

Fixed. I should have amended that instead of adding a commit.

[epage]

@bors r+

[bors]

📌 Commit 6207f93 has been approved by epage

It is now in the queue for this repository.

[bors]

⌛ Testing commit 6207f93 with merge 5afc53a...

[bors]

☀️ Test successful - checks-actions
Approved by: epage
Pushing 5afc53a to master...