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

Don't get ContentCachingResponseWrapper in ShallowEtagHeaderFilter so that users can extend this wrapper #24976

Closed
medusar opened this issue Apr 26, 2020 · 0 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@medusar
Copy link

medusar commented Apr 26, 2020

Issue Description:
Code in ShallowEtagHeaderFilter:
WebUtils.getNativeResponse(response, ContentCachingResponseWrapper.class);
should be replaced into
WebUtils.getNativeResponse(response, ConditionalContentCachingResponseWrapper.class); so that users can implement their own business logic based on ContentCachingResponseWrapper.

Change ContentCachingResponseWrapper to ConditionalContentCachingResponseWrapper so that it gets the right one.

private void updateResponse(HttpServletRequest request, HttpServletResponse response) throws IOException {

	ContentCachingResponseWrapper wrapper = 
// WebUtils.getNativeResponse(response, ContentCachingResponseWrapper.class);
   WebUtils.getNativeResponse(response, ConditionalContentCachingResponseWrapper.class);

		Assert.notNull(wrapper, "ContentCachingResponseWrapper not found");
		HttpServletResponse rawResponse = (HttpServletResponse) wrapper.getResponse();

		if (isEligibleForEtag(request, wrapper, wrapper.getStatus(), wrapper.getContentInputStream())) {
			String eTag = wrapper.getHeader(HttpHeaders.ETAG);
			if (!StringUtils.hasText(eTag)) {
				eTag = generateETagHeaderValue(wrapper.getContentInputStream(), this.writeWeakETag);
				rawResponse.setHeader(HttpHeaders.ETAG, eTag);
			}
			if (new ServletWebRequest(request, rawResponse).checkNotModified(eTag)) {
				return;
			}
		}

		wrapper.copyBodyToResponse();
	}

I have implemented my own Filter, and in my own Filter, i have implemented my own ResponseWrapper which extends ContentCachingResponseWrapper, everything works fine if ShallowEtagHeaderFilter is not loaded, but when it works with ShallowEtagHeaderFilter, when ShallowEtagHeaderFilter.updateResponse is invoked, the real HttpServletResponse get through WebUtils.getNativeResponse(response, ContentCachingResponseWrapper.class); become my own ResponseWrapper, so my own logic will be invoked inside ShallowEtagHeaderFilter, which is a mistake.
By changing the code to WebUtils.getNativeResponse(response, ConditionalContentCachingResponseWrapper.class);, everything will be fine.

Affects: <5.2.5.RELEASE and others>

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 26, 2020
@medusar medusar changed the title ConditionalContentCachingResponseWrapper should be used instead of ContentCachingResponseWrapper in ShallowEtagHeaderFilter Don't use ContentCachingResponseWrapper in ShallowEtagHeaderFilter so that users can extend this wrapper Apr 30, 2020
@medusar medusar changed the title Don't use ContentCachingResponseWrapper in ShallowEtagHeaderFilter so that users can extend this wrapper Don't get ContentCachingResponseWrapper in ShallowEtagHeaderFilter so that users can extend this wrapper Apr 30, 2020
@jhoeller jhoeller self-assigned this Jul 19, 2020
@jhoeller jhoeller added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jul 19, 2020
@jhoeller jhoeller added this to the 5.2.8 milestone Jul 19, 2020
FelixFly pushed a commit to FelixFly/spring-framework that referenced this issue Aug 16, 2020
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

No branches or pull requests

3 participants