Skip to content
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 #32321

Closed
github-actions bot opened this issue Feb 23, 2024 · 4 comments
Closed
Assignees
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
Copy link

Backport of gh-32317

@github-actions 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
@github-actions github-actions bot added this to the 6.0.18 milestone Feb 23, 2024
@sbrannen sbrannen self-assigned this Feb 23, 2024
@sbrannen 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 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-32321
Copy link
Author

Fixed via ca602ef

@sbrannen
Copy link
Member

Reopening, awaiting community feedback.

See #32317 (comment) for details.

@sbrannen sbrannen reopened this Feb 26, 2024
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-32321
@sbrannen

This comment was marked as duplicate.

Copy link
Author

Fixed via d1b3107

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
Projects
None yet
Development

No branches or pull requests

1 participant