As of Spring Framework 6.1.4, ContentCachingResponseWrapper returns null for Content-Type and Content-Length via getHeader("Content-Type"), getContentType(), etc.
Comment From: yikuo123
It may be caused by this commit https://github.com/spring-projects/spring-framework/commit/375e0e6827216ceb38e9bcf0b97b46bb79d79de6
Comment From: amuttsch
We have the problem that our rest controller, which sits behind a ShallowEtagHeaderFilter, does not return the Content-Type header anymore. I guess this is also related to this change.
Comment From: jhoeller
Do you specifically see a difference for programmatic calls on the HttpServletResponse there? Where do the header values (that you expect to see there) come from, are they being set programmatically elsewhere (outside of ContentCachingResponseWrapper)?
Comment From: tjalling-ran
spring-framework 5.3.32 may also be affected.
After upgrading to it (because of CVE-2024-22243), we no longer had content type headers in responses. So I downgraded back to 5.3.29. (luckily, we don't think we have any endpoints that are vulnerable to the CVE)
Comment From: amuttsch
@jhoeller We set them through the @GetMapping(produces = [MediaType.APPLICATION_JSON_VALUE]) annotation. We return a ResponseEntity.ok() and also tried to set the content type using .contentType(MediaType.APPLICATION_JSON_VALUE), which did not work either.
Comment From: sbrannen
Thanks for all of the feedback!
We're looking into a fix.
Comment From: sbrannen
Current work on this issue can be viewed in the following feature branch.
https://github.com/spring-projects/spring-framework/compare/main...sbrannen:spring-framework:issues/gh-32317-ContentCachingResponseWrapper-and-preset-headers
Comment From: sbrannen
I've pushed a potential fix to 6.1.x and main (see commit 5680d2063744380583bdc58fca0a17ac382beaa4).
@yikuo123, @amuttsch, @tjalling-ran, and anybody else interested in helping out...
We would be grateful if you could try out one of the upcoming 6.1.5-SNAPSHOT builds and let us know if the proposed fix addresses the regressions you've encountered!
If you confirm the fix works, we'll then backport to 6.0.x and 5.3.x.
Cheers,
Sam
p.s. you can find information on accessing snapshot builds here.
Comment From: tjallingran
@yikuo123, @amuttsch, @tjalling-ran, and anybody else interested in helping out...
We would be grateful if you could try out one of the upcoming
6.1.5-SNAPSHOTbuilds and let us know if the proposed fix addresses the regressions you've encountered!If you confirm the fix works, we'll then backport to 6.0.x and 5.3.x.
That was fast! π Appreciate the clear analysis / explanation in 5680d20
I'd be happy to try out a 5.3.x backport build if/when testers are desired for that. Unfortunately, we don't have any 6+ project that I can test with.
Comment From: sbrannen
That was fast! π
π
Appreciate the clear analysis / explanation in 5680d20
You're welcome, and glad you found it useful.
I'd be happy to try out a 5.3.x backport build if/when testers are desired for that. Unfortunately, we don't have any 6+ project that I can test with.
Thanks for offering.
We'll let you know as soon as we've backported anything to 5.3.x.
Comment From: sbrannen
Update: In be45481d701581efb69a990116d190a3879d0d1e, I fixed an additional minor issue with getHeaderNames() and introduced extensive tests to verify the expected behavior for all recent changes in ContentCachingResponseWrapper.
Comment From: sbrannen
@yikuo123, @amuttsch, @tjalling-ran, and anybody else interested in helping out...
The proposed fix is now available in 6.1.5-SNAPSHOT builds and will be available in upcoming 6.0.18-SNAPSHOT and 5.3.33-SNAPSHOT builds.
As mentioned in https://github.com/spring-projects/spring-framework/issues/32317#issuecomment-1962732906, we'd be grateful if you can test against the snapshots and report back here to let us know if the fixes work for you.
Thanks!
Sam
p.s. In the interim, we are leaving this issue "open" until we receive confirmations that the fixes work.
Comment From: tjalling-ran
@sbrannen I tried the 5.3.33-SNAPSHOT build shown in the screenshot below. It still has the issue - that is, there is no ContentType header in the response of the GET-request I tested with, whereas the same request on 5.3.29 does result in a response with the expected ContentType.
(screenshot of https://repo.spring.io/ui/native/snapshot/org/springframework/spring-framework-bom/5.3.33-SNAPSHOT/)
Judging from the decompiled class in IntelliJ, the snapshot build already has the fix:
Comment From: tjalling-ran
I may have found the issue. (Though I'm not familiar enough with the code to be fully confident that I didn't miss anything.)
As you know, ConditionalContentCachingResponseWrapper.setContentType() recently started setting its own ContentType instead of delegating to the wrapped Response.
As long as ConditionalContentCachingResponseWrapper.copyBodyToResponse() does indeed do copying, this is fine, as it will copy its ContentType to the wrapped response. However, when ConditionalContentCachingResponseWrapper.content.size() is 0, no copying is done.
So then the question is: how can ConditionalContentCachingResponseWrapper.content.size() be 0? The answer is that ConditionalContentCachingResponseWrapper.getOutputStream() only returns its own output stream if content caching is enabled. If content caching is disabled, it will delegate to the wrapped response's output stream. Thus, ConditionalContentCachingResponseWrapper will not have any content.
It will not delegate to the wrapped response's setContentType(), however (as stated above). So now we have a situation where the content is stored in the wrapped response, but the content type is stored in the wrapper.
And because a wrapper without content doesn't copy any of its own properties to the wrapped response, the wrapped response's content type (and possibly other properties) will remain null.
Comment From: yikuo123
I've pushed a potential fix to
6.1.xandmain(see commit 5680d20).@yikuo123, @amuttsch, @tjalling-ran, and anybody else interested in helping out...
We would be grateful if you could try out one of the upcoming
6.1.5-SNAPSHOTbuilds and let us know if the proposed fix addresses the regressions you've encountered!If you confirm the fix works, we'll then backport to
6.0.xand5.3.x.Cheers,
Sam
p.s. you can find information on accessing snapshot builds here.
Thank you very much!
I tried the 6.1.5-SNAPSHOT build and the fix works.
Comment From: amuttsch
@yikuo123, @amuttsch, @tjalling-ran, and anybody else interested in helping out...
The proposed fix is now available in
6.1.5-SNAPSHOTbuilds and will be available in upcoming6.0.18-SNAPSHOTand5.3.33-SNAPSHOTbuilds.As mentioned in #32317 (comment), we'd be grateful if you can test against the snapshots and report back here to let us know if the fixes work for you.
Thanks!
Sam
p.s. In the interim, we are leaving this issue "open" until we receive confirmations that the fixes work.
I tried the 6.1.5-SNAPSHOT but it did not work, the Content-Type header is still missing. To rule out any configuration error on my side: I added the repository
repositories {
maven { url = uri("https://repo.spring.io/snapshot") }
}
and overwrote the version using ext["spring-framework.version"] = "6.1.5-SNAPSHOT". At least IntelliJ shows me that the ShallowEtagHeaderFilter was loaded from spring-web-6.1.5-SNAPSHOT and I synced, cleared and rebuild my gradle project.
Comment From: amuttsch
I created a reproducible setup here: https://github.com/amuttsch/spring-boot-etag-content-header
Comment From: sbrannen
Hi everybody,
Thanks a lot for all of the prompt feedback!
In the end, we decided to revert the Content-Type caching in ContentCachingResponseWrapper.
So, hopefully that will address all remaining issues. π€
See cca440eb8e46c14d19df22df837ed4b90dd38673 for details.
The latest fix is now available in 6.1.5-SNAPSHOT builds.
So, if you're using 6.1.x, please try out the snapshot and let us know if the latest fix addresses the regressions you've encountered.
Once the fix is confirmed by a number of you, we'll then backport to 6.0.x and 5.3.x.
Cheers,
Sam
Comment From: amuttsch
Thanks for the quick response. My demo code works now, I can check my production code tomorrow. But I think it will most likely work too.
Comment From: sbrannen
Thanks for the quick response.
You're very welcome.
My demo code works now,
Awesome! Thanks for testing that with lightning fast speed and reporting back. π
I can check my production code tomorrow. But I think it will most likely work too.
That would be much appreciated.
Thanks
Comment From: resmo
Just for the record: as mentioned in https://github.com/spring-projects/spring-boot/issues/39745#issuecomment-1964865263: spring-core-6.1.5-20240226.173247-18.jar fixes my issue, umlauts are shown correctly (content-length size is as it was in 6.1.3)
Thanks!
Comment From: amuttsch
I can check my production code tomorrow. But I think it will most likely work too.
That would be much appreciated.
My production code works as well π
Comment From: sbrannen
@amuttsch and @resmo, thanks for confirming that the latest snapshots work! π
I'll likely backport to 6.0.x and 5.3.x tomorrow.
Comment From: sbrannen
The fix has now been backported to 6.0.x and 5.3.x and will be available in upcoming 6.0.18-SNAPSHOT and 5.3.33-SNAPSHOT builds.
If you can test your applications against those snapshots and report back here, that would be great.
Thanks,
Sam
Comment From: tjalling-ran
I can confirm that the issue is fixed in today's 5.3.33-SNAPSHOT build :sunglasses:
Thank you for the prompt fix @sbrannen.
Comment From: sbrannen
Glad to hear it, @tjalling-ran!
And thanks for letting us know.
Comment From: pperalta
Just tested with 5.3.33-SNAPSHOT, works for us too!
Comment From: sbrannen
Thanks for letting us know it works for you with the 5.3.33 snapshots, @pperalta!