Skip to content

Commit b2fe494

Browse files
rstoyanchevrstoyanchev
committed
Merge changes for ShallowEtagHeaderFilter
Closes gh-24635
2 parents e7df445 + a98bf30 commit b2fe494

File tree

7 files changed

+201
-90
lines changed

7 files changed

+201
-90
lines changed

‎spring-web/src/main/java/org/springframework/web/filter/ShallowEtagHeaderFilter.java

Lines changed: 48 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2019 the original author or authors.
2+
* Copyright 2002-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -31,6 +31,8 @@
3131
import org.springframework.http.HttpMethod;
3232
import org.springframework.util.Assert;
3333
import org.springframework.util.DigestUtils;
34+
import org.springframework.util.StringUtils;
35+
import org.springframework.web.context.request.ServletWebRequest;
3436
import org.springframework.web.util.ContentCachingResponseWrapper;
3537
import org.springframework.web.util.WebUtils;
3638

@@ -98,7 +100,7 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
98100

99101
HttpServletResponse responseToUse = response;
100102
if (!isAsyncDispatch(request) && !(response instanceof ContentCachingResponseWrapper)) {
101-
responseToUse = new HttpStreamingAwareContentCachingResponseWrapper(response, request);
103+
responseToUse = new ConditionalContentCachingResponseWrapper(response, request);
102104
}
103105

104106
filterChain.doFilter(request, responseToUse);
@@ -109,38 +111,35 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
109111
}
110112

111113
private void updateResponse(HttpServletRequest request, HttpServletResponse response) throws IOException {
112-
ContentCachingResponseWrapper responseWrapper =
114+
115+
ContentCachingResponseWrapper wrapper =
113116
WebUtils.getNativeResponse(response, ContentCachingResponseWrapper.class);
114-
Assert.notNull(responseWrapper, "ContentCachingResponseWrapper not found");
115-
HttpServletResponse rawResponse = (HttpServletResponse) responseWrapper.getResponse();
116-
int statusCode = responseWrapper.getStatus();
117117

118-
if (rawResponse.isCommitted()) {
119-
responseWrapper.copyBodyToResponse();
120-
}
121-
else if (isEligibleForEtag(request, responseWrapper, statusCode, responseWrapper.getContentInputStream())) {
122-
String responseETag = generateETagHeaderValue(responseWrapper.getContentInputStream(), this.writeWeakETag);
123-
rawResponse.setHeader(HttpHeaders.ETAG, responseETag);
124-
String requestETag = request.getHeader(HttpHeaders.IF_NONE_MATCH);
125-
if (requestETag != null && ("*".equals(requestETag) || compareETagHeaderValue(requestETag, responseETag))) {
126-
rawResponse.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
118+
Assert.notNull(wrapper, "ContentCachingResponseWrapper not found");
119+
HttpServletResponse rawResponse = (HttpServletResponse) wrapper.getResponse();
120+
121+
if (isEligibleForEtag(request, wrapper, wrapper.getStatus(), wrapper.getContentInputStream())) {
122+
String eTag = wrapper.getHeader(HttpHeaders.ETAG);
123+
if (!StringUtils.hasText(eTag)) {
124+
eTag = generateETagHeaderValue(wrapper.getContentInputStream(), this.writeWeakETag);
125+
rawResponse.setHeader(HttpHeaders.ETAG, eTag);
127126
}
128-
else {
129-
responseWrapper.copyBodyToResponse();
127+
if (new ServletWebRequest(request, rawResponse).checkNotModified(eTag)) {
128+
return;
130129
}
131130
}
132-
else {
133-
responseWrapper.copyBodyToResponse();
134-
}
131+
132+
wrapper.copyBodyToResponse();
135133
}
136134

137135
/**
138-
* Indicates whether the given request and response are eligible for ETag generation.
139-
* <p>The default implementation returns {@code true} if all conditions match:
136+
* Whether an ETag should be calculated for the given request and response
137+
* exchange. By default this is {@code true} if all of the following match:
140138
* <ul>
141-
* <li>response status codes in the {@code 2xx} series</li>
142-
* <li>request method is a GET</li>
143-
* <li>response Cache-Control header is not set or does not contain a "no-store" directive</li>
139+
* <li>Response is not committed.</li>
140+
* <li>Response status codes is in the {@code 2xx} series.</li>
141+
* <li>Request method is a GET.</li>
142+
* <li>Response Cache-Control header does not contain "no-store" (or is not present at all).</li>
144143
* </ul>
145144
* @param request the HTTP request
146145
* @param response the HTTP response
@@ -151,11 +150,14 @@ else if (isEligibleForEtag(request, responseWrapper, statusCode, responseWrapper
151150
protected boolean isEligibleForEtag(HttpServletRequest request, HttpServletResponse response,
152151
int responseStatusCode, InputStream inputStream) {
153152

154-
String method = request.getMethod();
155-
if (responseStatusCode >= 200 && responseStatusCode < 300 && HttpMethod.GET.matches(method)) {
153+
if (!response.isCommitted() &&
154+
responseStatusCode >= 200 && responseStatusCode < 300 &&
155+
HttpMethod.GET.matches(request.getMethod())) {
156+
156157
String cacheControl = response.getHeader(HttpHeaders.CACHE_CONTROL);
157158
return (cacheControl == null || !cacheControl.contains(DIRECTIVE_NO_STORE));
158159
}
160+
159161
return false;
160162
}
161163

@@ -191,10 +193,12 @@ private boolean compareETagHeaderValue(String requestETag, String responseETag)
191193

192194

193195
/**
194-
* This method can be used to disable the content caching response wrapper
195-
* of the ShallowEtagHeaderFilter. This can be done before the start of HTTP
196-
* streaming for example where the response will be written to asynchronously
197-
* and not in the context of a Servlet container thread.
196+
* This method can be used to suppress the content caching response wrapper
197+
* of the ShallowEtagHeaderFilter. The main reason for this is streaming
198+
* scenarios which are not to be cached and do not need an eTag.
199+
* <p><strong>Note:</strong> This method must be called before the response
200+
* is written to in order for the entire response content to be written
201+
* without caching.
198202
* @since 4.2
199203
*/
200204
public static void disableContentCaching(ServletRequest request) {
@@ -207,27 +211,34 @@ private static boolean isContentCachingDisabled(HttpServletRequest request) {
207211
}
208212

209213

210-
private static class HttpStreamingAwareContentCachingResponseWrapper extends ContentCachingResponseWrapper {
214+
/**
215+
* Returns the raw OutputStream, instead of the one that does caching,
216+
* if {@link #isContentCachingDisabled}.
217+
*/
218+
private static class ConditionalContentCachingResponseWrapper extends ContentCachingResponseWrapper {
211219

212220
private final HttpServletRequest request;
213221

214-
public HttpStreamingAwareContentCachingResponseWrapper(HttpServletResponse response, HttpServletRequest request) {
222+
223+
ConditionalContentCachingResponseWrapper(HttpServletResponse response, HttpServletRequest request) {
215224
super(response);
216225
this.request = request;
217226
}
218227

219228
@Override
220229
public ServletOutputStream getOutputStream() throws IOException {
221-
return (useRawResponse() ? getResponse().getOutputStream() : super.getOutputStream());
230+
return (isContentCachingDisabled(this.request) || hasETag() ?
231+
getResponse().getOutputStream() : super.getOutputStream());
222232
}
223233

224234
@Override
225235
public PrintWriter getWriter() throws IOException {
226-
return (useRawResponse() ? getResponse().getWriter() : super.getWriter());
236+
return (isContentCachingDisabled(this.request) || hasETag()?
237+
getResponse().getWriter() : super.getWriter());
227238
}
228239

229-
private boolean useRawResponse() {
230-
return isContentCachingDisabled(this.request);
240+
private boolean hasETag() {
241+
return StringUtils.hasText(getHeader(HttpHeaders.ETAG));
231242
}
232243
}
233244

‎spring-web/src/test/java/org/springframework/web/filter/ShallowEtagHeaderFilterTests.java

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2019 the original author or authors.
2+
* Copyright 2002-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.web.filter;
1818

19+
import java.nio.charset.StandardCharsets;
20+
1921
import javax.servlet.FilterChain;
2022
import javax.servlet.http.HttpServletResponse;
2123

@@ -62,7 +64,7 @@ public void filterNoMatch() throws Exception {
6264
final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hotels");
6365
MockHttpServletResponse response = new MockHttpServletResponse();
6466

65-
final byte[] responseBody = "Hello World".getBytes("UTF-8");
67+
final byte[] responseBody = "Hello World".getBytes(StandardCharsets.UTF_8);
6668
FilterChain filterChain = (filterRequest, filterResponse) -> {
6769
assertThat(filterRequest).as("Invalid request passed").isEqualTo(request);
6870
((HttpServletResponse) filterResponse).setStatus(HttpServletResponse.SC_OK);
@@ -71,7 +73,7 @@ public void filterNoMatch() throws Exception {
7173
filter.doFilter(request, response, filterChain);
7274

7375
assertThat(response.getStatus()).as("Invalid status").isEqualTo(200);
74-
assertThat(response.getHeader("ETag")).as("Invalid ETag header").isEqualTo("\"0b10a8db164e0754105b7a99be72e3fe5\"");
76+
assertThat(response.getHeader("ETag")).as("Invalid ETag").isEqualTo("\"0b10a8db164e0754105b7a99be72e3fe5\"");
7577
assertThat(response.getContentLength() > 0).as("Invalid Content-Length header").isTrue();
7678
assertThat(response.getContentAsByteArray()).as("Invalid content").isEqualTo(responseBody);
7779
}
@@ -82,7 +84,7 @@ public void filterNoMatchWeakETag() throws Exception {
8284
final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hotels");
8385
MockHttpServletResponse response = new MockHttpServletResponse();
8486

85-
final byte[] responseBody = "Hello World".getBytes("UTF-8");
87+
final byte[] responseBody = "Hello World".getBytes(StandardCharsets.UTF_8);
8688
FilterChain filterChain = (filterRequest, filterResponse) -> {
8789
assertThat(filterRequest).as("Invalid request passed").isEqualTo(request);
8890
((HttpServletResponse) filterResponse).setStatus(HttpServletResponse.SC_OK);
@@ -91,7 +93,7 @@ public void filterNoMatchWeakETag() throws Exception {
9193
filter.doFilter(request, response, filterChain);
9294

9395
assertThat(response.getStatus()).as("Invalid status").isEqualTo(200);
94-
assertThat(response.getHeader("ETag")).as("Invalid ETag header").isEqualTo("W/\"0b10a8db164e0754105b7a99be72e3fe5\"");
96+
assertThat(response.getHeader("ETag")).as("Invalid ETag").isEqualTo("W/\"0b10a8db164e0754105b7a99be72e3fe5\"");
9597
assertThat(response.getContentLength() > 0).as("Invalid Content-Length header").isTrue();
9698
assertThat(response.getContentAsByteArray()).as("Invalid content").isEqualTo(responseBody);
9799
}
@@ -105,14 +107,14 @@ public void filterMatch() throws Exception {
105107

106108
FilterChain filterChain = (filterRequest, filterResponse) -> {
107109
assertThat(filterRequest).as("Invalid request passed").isEqualTo(request);
108-
byte[] responseBody = "Hello World".getBytes("UTF-8");
110+
byte[] responseBody = "Hello World".getBytes(StandardCharsets.UTF_8);
109111
FileCopyUtils.copy(responseBody, filterResponse.getOutputStream());
110112
filterResponse.setContentLength(responseBody.length);
111113
};
112114
filter.doFilter(request, response, filterChain);
113115

114116
assertThat(response.getStatus()).as("Invalid status").isEqualTo(304);
115-
assertThat(response.getHeader("ETag")).as("Invalid ETag header").isEqualTo("\"0b10a8db164e0754105b7a99be72e3fe5\"");
117+
assertThat(response.getHeader("ETag")).as("Invalid ETag").isEqualTo("\"0b10a8db164e0754105b7a99be72e3fe5\"");
116118
assertThat(response.containsHeader("Content-Length")).as("Response has Content-Length header").isFalse();
117119
byte[] expecteds = new byte[0];
118120
assertThat(response.getContentAsByteArray()).as("Invalid content").isEqualTo(expecteds);
@@ -127,14 +129,14 @@ public void filterMatchWeakEtag() throws Exception {
127129

128130
FilterChain filterChain = (filterRequest, filterResponse) -> {
129131
assertThat(filterRequest).as("Invalid request passed").isEqualTo(request);
130-
byte[] responseBody = "Hello World".getBytes("UTF-8");
132+
byte[] responseBody = "Hello World".getBytes(StandardCharsets.UTF_8);
131133
FileCopyUtils.copy(responseBody, filterResponse.getOutputStream());
132134
filterResponse.setContentLength(responseBody.length);
133135
};
134136
filter.doFilter(request, response, filterChain);
135137

136138
assertThat(response.getStatus()).as("Invalid status").isEqualTo(304);
137-
assertThat(response.getHeader("ETag")).as("Invalid ETag header").isEqualTo("\"0b10a8db164e0754105b7a99be72e3fe5\"");
139+
assertThat(response.getHeader("ETag")).as("Invalid ETag").isEqualTo("\"0b10a8db164e0754105b7a99be72e3fe5\"");
138140
assertThat(response.containsHeader("Content-Length")).as("Response has Content-Length header").isFalse();
139141
byte[] expecteds = new byte[0];
140142
assertThat(response.getContentAsByteArray()).as("Invalid content").isEqualTo(expecteds);
@@ -156,7 +158,7 @@ public void filterWriter() throws Exception {
156158
filter.doFilter(request, response, filterChain);
157159

158160
assertThat(response.getStatus()).as("Invalid status").isEqualTo(304);
159-
assertThat(response.getHeader("ETag")).as("Invalid ETag header").isEqualTo("\"0b10a8db164e0754105b7a99be72e3fe5\"");
161+
assertThat(response.getHeader("ETag")).as("Invalid ETag").isEqualTo("\"0b10a8db164e0754105b7a99be72e3fe5\"");
160162
assertThat(response.containsHeader("Content-Length")).as("Response has Content-Length header").isFalse();
161163
byte[] expecteds = new byte[0];
162164
assertThat(response.getContentAsByteArray()).as("Invalid content").isEqualTo(expecteds);
@@ -167,7 +169,7 @@ public void filterWriterWithDisabledCaching() throws Exception {
167169
final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hotels");
168170
MockHttpServletResponse response = new MockHttpServletResponse();
169171

170-
final byte[] responseBody = "Hello World".getBytes("UTF-8");
172+
final byte[] responseBody = "Hello World".getBytes(StandardCharsets.UTF_8);
171173
FilterChain filterChain = (filterRequest, filterResponse) -> {
172174
assertThat(filterRequest).as("Invalid request passed").isEqualTo(request);
173175
((HttpServletResponse) filterResponse).setStatus(HttpServletResponse.SC_OK);
@@ -187,7 +189,7 @@ public void filterSendError() throws Exception {
187189
final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hotels");
188190
MockHttpServletResponse response = new MockHttpServletResponse();
189191

190-
final byte[] responseBody = "Hello World".getBytes("UTF-8");
192+
final byte[] responseBody = "Hello World".getBytes(StandardCharsets.UTF_8);
191193
FilterChain filterChain = (filterRequest, filterResponse) -> {
192194
assertThat(filterRequest).as("Invalid request passed").isEqualTo(request);
193195
response.setContentLength(100);
@@ -197,7 +199,7 @@ public void filterSendError() throws Exception {
197199
filter.doFilter(request, response, filterChain);
198200

199201
assertThat(response.getStatus()).as("Invalid status").isEqualTo(403);
200-
assertThat(response.getHeader("ETag")).as("Invalid ETag header").isNull();
202+
assertThat(response.getHeader("ETag")).as("Invalid ETag").isNull();
201203
assertThat(response.getContentLength()).as("Invalid Content-Length header").isEqualTo(100);
202204
assertThat(response.getContentAsByteArray()).as("Invalid content").isEqualTo(responseBody);
203205
}
@@ -207,7 +209,7 @@ public void filterSendErrorMessage() throws Exception {
207209
final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hotels");
208210
MockHttpServletResponse response = new MockHttpServletResponse();
209211

210-
final byte[] responseBody = "Hello World".getBytes("UTF-8");
212+
final byte[] responseBody = "Hello World".getBytes(StandardCharsets.UTF_8);
211213
FilterChain filterChain = (filterRequest, filterResponse) -> {
212214
assertThat(filterRequest).as("Invalid request passed").isEqualTo(request);
213215
response.setContentLength(100);
@@ -217,7 +219,7 @@ public void filterSendErrorMessage() throws Exception {
217219
filter.doFilter(request, response, filterChain);
218220

219221
assertThat(response.getStatus()).as("Invalid status").isEqualTo(403);
220-
assertThat(response.getHeader("ETag")).as("Invalid ETag header").isNull();
222+
assertThat(response.getHeader("ETag")).as("Invalid ETag").isNull();
221223
assertThat(response.getContentLength()).as("Invalid Content-Length header").isEqualTo(100);
222224
assertThat(response.getContentAsByteArray()).as("Invalid content").isEqualTo(responseBody);
223225
assertThat(response.getErrorMessage()).as("Invalid error message").isEqualTo("ERROR");
@@ -228,7 +230,7 @@ public void filterSendRedirect() throws Exception {
228230
final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hotels");
229231
MockHttpServletResponse response = new MockHttpServletResponse();
230232

231-
final byte[] responseBody = "Hello World".getBytes("UTF-8");
233+
final byte[] responseBody = "Hello World".getBytes(StandardCharsets.UTF_8);
232234
FilterChain filterChain = (filterRequest, filterResponse) -> {
233235
assertThat(filterRequest).as("Invalid request passed").isEqualTo(request);
234236
response.setContentLength(100);
@@ -238,19 +240,18 @@ public void filterSendRedirect() throws Exception {
238240
filter.doFilter(request, response, filterChain);
239241

240242
assertThat(response.getStatus()).as("Invalid status").isEqualTo(302);
241-
assertThat(response.getHeader("ETag")).as("Invalid ETag header").isNull();
243+
assertThat(response.getHeader("ETag")).as("Invalid ETag").isNull();
242244
assertThat(response.getContentLength()).as("Invalid Content-Length header").isEqualTo(100);
243245
assertThat(response.getContentAsByteArray()).as("Invalid content").isEqualTo(responseBody);
244246
assertThat(response.getRedirectedUrl()).as("Invalid redirect URL").isEqualTo("https://www.google.com");
245247
}
246248

247-
// SPR-13717
248-
@Test
249+
@Test // SPR-13717
249250
public void filterFlushResponse() throws Exception {
250251
final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hotels");
251252
MockHttpServletResponse response = new MockHttpServletResponse();
252253

253-
final byte[] responseBody = "Hello World".getBytes("UTF-8");
254+
final byte[] responseBody = "Hello World".getBytes(StandardCharsets.UTF_8);
254255
FilterChain filterChain = (filterRequest, filterResponse) -> {
255256
assertThat(filterRequest).as("Invalid request passed").isEqualTo(request);
256257
((HttpServletResponse) filterResponse).setStatus(HttpServletResponse.SC_OK);
@@ -260,7 +261,7 @@ public void filterFlushResponse() throws Exception {
260261
filter.doFilter(request, response, filterChain);
261262

262263
assertThat(response.getStatus()).as("Invalid status").isEqualTo(200);
263-
assertThat(response.getHeader("ETag")).as("Invalid ETag header").isEqualTo("\"0b10a8db164e0754105b7a99be72e3fe5\"");
264+
assertThat(response.getHeader("ETag")).as("Invalid ETag").isEqualTo("\"0b10a8db164e0754105b7a99be72e3fe5\"");
264265
assertThat(response.getContentLength() > 0).as("Invalid Content-Length header").isTrue();
265266
assertThat(response.getContentAsByteArray()).as("Invalid content").isEqualTo(responseBody);
266267
}

‎spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2019 the original author or authors.
2+
* Copyright 2002-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -21,10 +21,8 @@
2121
import java.lang.reflect.Type;
2222
import java.util.ArrayList;
2323
import java.util.Collections;
24-
import java.util.EnumSet;
2524
import java.util.List;
2625
import java.util.Map;
27-
import java.util.Set;
2826

2927
import javax.servlet.http.HttpServletRequest;
3028
import javax.servlet.http.HttpServletResponse;
@@ -49,7 +47,6 @@
4947
import org.springframework.web.bind.support.WebDataBinderFactory;
5048
import org.springframework.web.context.request.NativeWebRequest;
5149
import org.springframework.web.context.request.ServletWebRequest;
52-
import org.springframework.web.filter.ShallowEtagHeaderFilter;
5350
import org.springframework.web.method.support.ModelAndViewContainer;
5451
import org.springframework.web.servlet.mvc.support.RedirectAttributes;
5552
import org.springframework.web.servlet.support.RequestContextUtils;
@@ -70,8 +67,6 @@
7067
*/
7168
public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodProcessor {
7269

73-
private static final Set<HttpMethod> SAFE_METHODS = EnumSet.of(HttpMethod.GET, HttpMethod.HEAD);
74-
7570
/**
7671
* Basic constructor with converters only. Suitable for resolving
7772
* {@code HttpEntity}. For handling {@code ResponseEntity} consider also
@@ -205,12 +200,10 @@ public void handleReturnValue(@Nullable Object returnValue, MethodParameter retu
205200
int returnStatus = ((ResponseEntity<?>) responseEntity).getStatusCodeValue();
206201
outputMessage.getServletResponse().setStatus(returnStatus);
207202
if (returnStatus == 200) {
208-
if (SAFE_METHODS.contains(inputMessage.getMethod())
203+
HttpMethod method = inputMessage.getMethod();
204+
if ((HttpMethod.GET.equals(method) || HttpMethod.HEAD.equals(method))
209205
&& isResourceNotModified(inputMessage, outputMessage)) {
210-
// Ensure headers are flushed, no body should be written.
211206
outputMessage.flush();
212-
ShallowEtagHeaderFilter.disableContentCaching(inputMessage.getServletRequest());
213-
// Skip call to converters, as they may update the body.
214207
return;
215208
}
216209
}

0 commit comments

Comments
 (0)