Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

axum/routing: Add content-length header assertions to middleware_adding_body test #3033

Merged
merged 1 commit into from
Nov 16, 2024

Conversation

Turbo87
Copy link
Collaborator

@Turbo87 Turbo87 commented Nov 16, 2024

I noticed in our crates.io test suite, that the content-length header was no longer set on responses after the latest update and wondered if #2897 had broken it. I quickly extended the corresponding test to check, and it looks like everything is fine.

I noticed in our crates.io test suite, that the content-length header was no longer set on responses

We are using router.oneshot(request).await over there, which seems to have changed behavior due to the PR linked above. The axum test suite is using real reqwest calls though, which don't appear to show the same behavior.

@Turbo87
Copy link
Collaborator Author

Turbo87 commented Nov 16, 2024

looks like the change is not just in our (crates.io) test suite, but also when running the server regularly (see #2897 (comment)) 🤔

@mladedav
Copy link
Collaborator

Can you also check this on the 0.7.x branch (or specifically on the release commit) before the merge to main? That's where the release was created so maybe it's broken there and after merging back to main it works ok.

In either case I'll take a look tomorrow unless someone else figures this out in the meantime.

@mladedav mladedav merged commit 9ec18b9 into tokio-rs:main Nov 16, 2024
18 checks passed
@Turbo87
Copy link
Collaborator Author

Turbo87 commented Nov 16, 2024

Can you also check this on the 0.7.x branch

seems to pass on that branch as well.

we use quite a few middlewares in the crates.io codebase, so maybe that somehow makes this trigger while it doesn't in the tests here?

@Turbo87 Turbo87 deleted the header-assertions branch November 16, 2024 23:12
@mladedav
Copy link
Collaborator

I've added a bit of description to the crates.io's upgrade PR but basically the compression layer (and possibly others) started using chunked transfer instead of content type. More info will be in #3035

Thanks again for notifying us.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants