Skip to content

tests/util: Add Content-Type: application/json request headers #7895

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

Merged
merged 1 commit into from
Jan 9, 2024

Conversation

Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Jan 8, 2024

The axum::Json extractor requires this header to be present and rejects requests that don't have it. This change ensures we can move our code over from custom JSON parsing to the axum extractor eventually.

The `axum::Json` extractor requires this header to be present and rejects requests that don't have it. This change ensures we can move our code over from custom JSON parsing to the `axum` extractor eventually.
@Turbo87 Turbo87 added C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear A-backend ⚙️ labels Jan 8, 2024
@Turbo87
Copy link
Member Author

Turbo87 commented Jan 8, 2024

the cargo team confirmed via Zulip that this header is being sent by cargo and has been for a long time :)

@@ -139,8 +139,15 @@ pub trait RequestHelper {
/// Issue a PUT request
#[track_caller]
fn put<T>(&self, path: &str, body: impl Into<Bytes>) -> Response<T> {
let body = body.into();
let is_json = body.starts_with(b"{") && body.ends_with(b"}");
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this will eventually cause someone to have an annoying day writing tests if we ever add an endpoint that doesn't expect an object, but presumably that's not an issue right now.

I guess the other option would be to add a parallel put_json method that sets the header. I don't feel very strongly about this. 🤷

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, though arguably APIs shouldn't return raw arrays anyway and that extra friction might be good 😄

the only PUT that does not deal with Json currently is the publish endpoint. it felt not worth it to have a dedicated method just for that.

@Turbo87 Turbo87 merged commit d639f0c into rust-lang:main Jan 9, 2024
@Turbo87 Turbo87 deleted the ct-header branch January 9, 2024 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend ⚙️ C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants