Skip to content
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.

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

crates-io: Set Content-Type: application/json only for requests with a body payload #13264

Merged
merged 2 commits into from
Jan 9, 2024

Conversation

Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Jan 8, 2024

What does this PR try to resolve?

The Content-Type request header is only supposed to be used if the request comes with a body payload. cargo is currently sending it unconditionally, even for GET requests that typically don't have a payload attached to them.

This PR changes the implementation to only attach the header if the request has a body payload.

@rustbot
Copy link
Collaborator

rustbot commented Jan 8, 2024

r? @ehuss

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

@rustbot rustbot added A-documenting-cargo-itself Area: Cargo's documentation A-interacts-with-crates.io Area: interaction with registries S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 8, 2024
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

Could you bump a patch version for crates-io package?

I am okay with this change and would be surprised if it breaks other registries, though seems like the RFC doesn't say anything about whether an empty body can have a Content-Type header.

@Turbo87 Turbo87 force-pushed the ct-header branch 2 times, most recently from b5232e6 to 1049aa1 Compare January 9, 2024 17:02
@Turbo87
Copy link
Member Author

Turbo87 commented Jan 9, 2024

Could you bump a patch version for crates-io package?

since it could be seen as a breaking change I opted for a minor version bump. let me know if that's okay with you or if you want me to change it :)

@weihanglo
Copy link
Member

It is not a SemVer breaking change, though I am fine with merging it as-is. Thank you :)
See #11820 (comment) for a similar discussion.

@bors r+

@bors
Copy link
Contributor

bors commented Jan 9, 2024

📌 Commit 6218d08 has been approved by weihanglo

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 Jan 9, 2024
@bors
Copy link
Contributor

bors commented Jan 9, 2024

⌛ Testing commit 6218d08 with merge 616bae2...

@bors
Copy link
Contributor

bors commented Jan 9, 2024

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 616bae2 to master...

@bors bors merged commit 616bae2 into rust-lang:master Jan 9, 2024
22 checks passed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 10, 2024
Update cargo

14 commits in 2ce45605d9db521b5fd6c1211ce8de6055fdb24e..3e428a38a34e820a461d2cc082e726d3bda71bcb
2024-01-04 18:04:13 +0000 to 2024-01-09 20:46:36 +0000
- refactor: replace `iter_join` with `itertools::join` (rust-lang/cargo#13275)
- docs(unstable): doc comments for items and fields (rust-lang/cargo#13274)
- crates-io: Set `Content-Type: application/json` only for requests with a body payload (rust-lang/cargo#13264)
- fix: only inherit workspace package table if the new package is a member (rust-lang/cargo#13261)
- feat(cli): add colors to `-Zhelp` console output (rust-lang/cargo#13269)
- chore(deps): update msrv (rust-lang/cargo#13266)
- refactor(toml): Make it more obvious to update package-dependent fields (rust-lang/cargo#13267)
- chore(ci): Fix MSRV:3 updates (rust-lang/cargo#13268)
- chore(ci): Shot-in-the-dark fix for MSRV updating (rust-lang/cargo#13265)
- fix: set OUT_DIR for all units with build scripts (rust-lang/cargo#13204)
- fix(manifest): Provide unused key warnings for lints table (rust-lang/cargo#13262)
- test(manifest): Verify we warn on unused workspace.package fields (rust-lang/cargo#13263)
- docs(changelog): Call out cargo-new lockfile change (rust-lang/cargo#13260)
- chore: Add dependency dashboard (rust-lang/cargo#13255)

r? ghost
@rustbot rustbot added this to the 1.77.0 milestone Jan 10, 2024
@Turbo87 Turbo87 deleted the ct-header branch January 10, 2024 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documenting-cargo-itself Area: Cargo's documentation A-interacts-with-crates.io Area: interaction with registries S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants