Skip to content

fix: add error for unsupported credential provider version #12590

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 30, 2023
Merged

fix: add error for unsupported credential provider version #12590

merged 1 commit into from
Aug 30, 2023

Conversation

@arlosi
Copy link
Contributor

@arlosi arlosi commented Aug 29, 2023

Cargo currently ignores the version in the CredentialHello message, and proceeds to use version 1 regardless of what the credential provider claims it can support.

This change does the following:

  • Adds a new error if Cargo doesn't support any of the supported protocol versions offered by the provider.
  • Kills the credential provider subprocess if it fails. This prevents it from hanging or printing spurious errors such as "broken pipe" when it's attempting to read the next JSON message.
  • Adds a new test for an unsupported credential provider protocol.

@rustbot
Copy link
Collaborator

rustbot commented Aug 29, 2023

r? @ehuss

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-registry-authentication Area: registry authentication and authorization (authn authz) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 29, 2023
@arlosi arlosi added the A-credential-provider Area: credential provider for storing and retreiving credentials label Aug 29, 2023
epage
epage approved these changes Aug 29, 2023
View reviewed changes
Copy link
Contributor

@epage epage left a comment

Choose a reason for hiding this comment

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

Feel free to r= me after the version bump

@arlosi arlosi force-pushed the cred-unsupported-error branch from 24e4214 to 286f350 Compare August 29, 2023 21:00
@arlosi
Copy link
Contributor Author

arlosi commented Aug 29, 2023

@bors r=epage

@bors
Copy link
Contributor

bors commented Aug 29, 2023

📌 Commit 286f350 has been approved by epage

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 29, 2023
@bors
Copy link
Contributor

bors commented Aug 29, 2023

⌛ Testing commit 286f350 with merge e45f5d2...

bors added a commit that referenced this pull request Aug 29, 2023
@bors
Auto merge of #12590 - arlosi:cred-unsupported-error, r=epage
e45f5d2 
fix: add error for unsupported credential provider version

Cargo currently ignores the version in the `CredentialHello` message, and proceeds to use version `1` regardless of what the credential provider claims it can support.

This change does the following:
* Adds a new error if Cargo doesn't support any of the supported protocol versions offered by the provider.
* Kills the credential provider subprocess if it fails. This prevents it from hanging or printing spurious errors such as "broken pipe" when it's attempting to read the next JSON message.
* Adds a new test for an unsupported credential provider protocol.
@bors
Copy link
Contributor

bors commented Aug 29, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 29, 2023
@arlosi arlosi force-pushed the cred-unsupported-error branch from 286f350 to 39db61e Compare August 30, 2023 02:22
@arlosi
Copy link
Contributor Author

arlosi commented Aug 30, 2023

@bors r=epage

@bors
Copy link
Contributor

bors commented Aug 30, 2023

📌 Commit 39db61e has been approved by epage

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 30, 2023
@bors
Copy link
Contributor

bors commented Aug 30, 2023

⌛ Testing commit 39db61e with merge 40f1f67...

@bors
Copy link
Contributor

bors commented Aug 30, 2023

☀️ Test successful - checks-actions
Approved by: epage
Pushing 40f1f67 to master...

@bors bors merged commit 40f1f67 into rust-lang:master Aug 30, 2023
@weihanglo weihanglo mentioned this pull request Sep 6, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 6, 2023
@bors
Auto merge of rust-lang#115593 - weihanglo:update-cargo, r=weihanglo
3767e31 
Update cargo

21 commits in 96fe1c9e1aecd8f57063e3753969bb6418fd2fd5..d14c85f4e6e7671673b1a1bc87231ff7164761e1
2023-08-29 20:10:34 +0000 to 2023-09-05 22:28:10 +0000
- fix(resolver): Make resolver behavior independent of package order (rust-lang/cargo#12602)
- cargo-credential: change serialization of cache expiration (rust-lang/cargo#12622)
- Update registry-web-api.md yank/unyank comments (rust-lang/cargo#12619)
- test: new options of debuginfo are no longer unstable (rust-lang/cargo#12618)
- use split_once for cleaner code (rust-lang/cargo#12615)
- stop using lazy_static (rust-lang/cargo#12616)
- doc: adjust all doc headings one level up (rust-lang/cargo#12595)
- chore(deps): update compatible (rust-lang/cargo#12609)
- chore(deps): update rust crate cargo_metadata to 0.17.0 (rust-lang/cargo#12610)
- Prepare for partial-version package specs (rust-lang/cargo#12591)
- refactor: Use more serde_untagged (rust-lang/cargo#12581)
- fix(cli): Help users know possible `--target` values (rust-lang/cargo#12607)
- Tab completion for --target uses rustup but fallsback to rustc (rust-lang/cargo#12606)
- Fewer temporary needless strings (rust-lang/cargo#12604)
- fix(help): Provide better commands heading for styling (rust-lang/cargo#12593)
- fix(update): Clarify meaning of --aggressive as --recursive (rust-lang/cargo#12544)
- docs(changelog): Clarify language for Cargo.lock policy (rust-lang/cargo#12601)
- fix typo: "default branch branch" -> "default branch" (rust-lang/cargo#12598)
- fix: add error for unsupported credential provider version (rust-lang/cargo#12590)
- fix(help): Explain --explain (rust-lang/cargo#12592)
- fix(help): Remove redundant information from new/init (rust-lang/cargo#12594)

r? ghost
@ehuss ehuss mentioned this pull request Sep 7, 2023
Closed
16 tasks
@ehuss ehuss added this to the 1.74.0 milestone Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@epage epage epage approved these changes

Assignees

@ehuss ehuss

Labels

A-credential-provider Area: credential provider for storing and retreiving credentials A-registry-authentication Area: registry authentication and authorization (authn authz) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

1.74.0

Development

Successfully merging this pull request may close these issues.

5 participants

@arlosi @rustbot @bors @epage @ehuss