-
Notifications
You must be signed in to change notification settings - Fork 38.3k
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
ContentCachingResponseWrapper no longer honors Content-Type and Content-Length #32322
Labels
in: web
Issues in web modules (web, webmvc, webflux, websocket)
type: backport
An issue that is a backport of another issue to a maintenance branch
type: regression
A bug that is also a regression
Milestone
Comments
github-actions
bot
added
in: web
Issues in web modules (web, webmvc, webflux, websocket)
type: backport
An issue that is a backport of another issue to a maintenance branch
type: regression
A bug that is also a regression
labels
Feb 23, 2024
sbrannen
changed the title
ContentCachingResponseWrapper does not return Content-Type and Content-Length anymore
ContentCachingResponseWrapper no longer returns Content-Type and Content-Length
Feb 23, 2024
sbrannen
changed the title
ContentCachingResponseWrapper no longer returns Content-Type and Content-Length
ContentCachingResponseWrapper no longer honors Content-Type and Content-Length
Feb 25, 2024
sbrannen
added a commit
that referenced
this issue
Feb 25, 2024
Commit 375e0e6 introduced a regression in ContentCachingResponseWrapper (CCRW). Specifically, CCRW no longer honors Content-Type and Content-Length headers that have been set in the wrapped response and now incorrectly returns null for those header values if they have not been set directly in the CCRW. This commit fixes this regression as follows. - The Content-Type and Content-Length headers set in the wrapped response are honored in getContentType(), containsHeader(), getHeader(), and getHeaders() unless those headers have been set directly in the CCRW. - In copyBodyToResponse(), the Content-Type in the wrapped response is only overridden if the Content-Type has been set directly in the CCRW. Furthermore, prior to this commit, getHeaderNames() returned duplicates for the Content-Type and Content-Length headers if they were set in the wrapped response as well as in CCRW. This commit fixes that by returning a unique set from getHeaderNames(). This commit also updates ContentCachingResponseWrapperTests to verify the expected behavior for Content-Type and Content-Length headers that are set in the wrapped response as well as in CCRW. See gh-32039 See gh-32317 Closes gh-32322
Fixed via 380f5d5 |
Reopening, awaiting community feedback. See #32317 (comment) for details. |
sbrannen
added a commit
that referenced
this issue
Feb 28, 2024
Based on feedback from several members of the community, we have decided to revert the caching of the Content-Type header that was introduced in ContentCachingResponseWrapper in 375e0e6. This commit therefore completely removes Content-Type caching in ContentCachingResponseWrapper and updates the existing tests accordingly. To provide guards against future regressions in this area, this commit also introduces explicit tests for the 6 ways to set the content length in ContentCachingResponseWrapper and modifies a test in ShallowEtagHeaderFilterTests to ensure that a Content-Type header set directly on ContentCachingResponseWrapper is propagated to the underlying response even if content caching is disabled for the ShallowEtagHeaderFilter. See gh-32039 See gh-32317 Closes gh-32322
Fixed via 464fa7e |
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: backport
An issue that is a backport of another issue to a maintenance branch
type: regression
A bug that is also a regression
Backport of gh-32317
The text was updated successfully, but these errors were encountered: