Skip to content

Update Content-Length when body changed by Interceptor #33459

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

Closed
wants to merge 3 commits into from

Conversation

imzhoukunqiang
Copy link
Contributor

@imzhoukunqiang imzhoukunqiang commented Sep 1, 2024

Hi maintainers, while working with the framework, I discovered a bug that I believe warrants attention.

In the org.springframework.http.client.InterceptingClientHttpRequest , the request body may be changed by interceptors, but Content-Length in the request headers didn't. This may cause the server receive the incorrect request body.
I created a simple demo to show this bug . DEMO

The fix for this is to reset the Content-Length before sending the request at org.springframework.http.client.InterceptingClientHttpRequest.InterceptingRequestExecution#execute

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 1, 2024
@bclozel bclozel added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Sep 1, 2024
@bclozel
Copy link
Member

bclozel commented Sep 1, 2024

I am not sure we should do this. Do we really set the content length header in all cases and for all converters? If an interceptor is changing the request body I would argue it needs to change the content type as well as the content length of it was set already.
I think some http clients can send bodies with chunked transfer encoding.

@imzhoukunqiang
Copy link
Contributor Author

I am not sure we should do this. Do we really set the content length header in all cases and for all converters? If an interceptor is changing the request body I would argue it needs to change the content type as well as the content length of it was set already. I think some http clients can send bodies with chunked transfer encoding.

I agree with your point of view. I'm not sure whether setting Content-Length is the responsibility of the user or the framework. However, typically when we use RestTemplate, we do not set Content-Length, and it is handled by the framework.

And if after executing the interceptor chain, the Content-Length exists in the request header and does not equal body.length, should it be corrected to the correct value?

@bclozel
Copy link
Member

bclozel commented Sep 1, 2024

Your description makes sense. However, isn't your PR setting it in all cases?

@imzhoukunqiang
Copy link
Contributor Author

Your description makes sense. However, isn't your PR setting it in all cases?

Updated.

@bclozel bclozel added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Sep 2, 2024
@bclozel bclozel self-assigned this Sep 2, 2024
@bclozel bclozel added this to the 6.2.0-RC1 milestone Sep 2, 2024
@bclozel bclozel closed this in aca2649 Sep 2, 2024
@bclozel
Copy link
Member

bclozel commented Sep 2, 2024

Thanks for your contribution @imzhoukunqiang , this is now merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants