Do not process undefined conditional HTTP requests
Prior to this change, the HttpEntityMethodProcessor would try to process conditional requests that are undefined by the spec, such as: * an HTTP GET request with "If-None-Match:*" * a request with both "If-None-Match" and "If-Match" * a request with both "If-None-Match" and "If-Unmodified-Since" This commit skips the processing of those requests as conditional requests and continues with normal request handling. Issue: SPR-13626
This commit is contained in:
parent
9f4c9d7f3c
commit
889366320d
|
@ -172,7 +172,7 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro
|
||||||
Object body = responseEntity.getBody();
|
Object body = responseEntity.getBody();
|
||||||
if (responseEntity instanceof ResponseEntity) {
|
if (responseEntity instanceof ResponseEntity) {
|
||||||
outputMessage.setStatusCode(((ResponseEntity<?>) responseEntity).getStatusCode());
|
outputMessage.setStatusCode(((ResponseEntity<?>) responseEntity).getStatusCode());
|
||||||
if (inputMessage.getMethod() == HttpMethod.GET &&
|
if (inputMessage.getMethod().equals(HttpMethod.GET) &&
|
||||||
isResourceNotModified(inputMessage, outputMessage)) {
|
isResourceNotModified(inputMessage, outputMessage)) {
|
||||||
outputMessage.setStatusCode(HttpStatus.NOT_MODIFIED);
|
outputMessage.setStatusCode(HttpStatus.NOT_MODIFIED);
|
||||||
// Ensure headers are flushed, no body should be written.
|
// Ensure headers are flushed, no body should be written.
|
||||||
|
@ -196,7 +196,11 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro
|
||||||
long lastModified = outputMessage.getHeaders().getLastModified();
|
long lastModified = outputMessage.getHeaders().getLastModified();
|
||||||
boolean notModified = false;
|
boolean notModified = false;
|
||||||
|
|
||||||
if (lastModified != -1 && StringUtils.hasLength(eTag)) {
|
if (!ifNoneMatch.isEmpty() && (inputMessage.getHeaders().containsKey(HttpHeaders.IF_UNMODIFIED_SINCE)
|
||||||
|
|| inputMessage.getHeaders().containsKey(HttpHeaders.IF_MATCH))) {
|
||||||
|
// invalid conditional request, do not process
|
||||||
|
}
|
||||||
|
else if (lastModified != -1 && StringUtils.hasLength(eTag)) {
|
||||||
notModified = isETagNotModified(ifNoneMatch, eTag) && isTimeStampNotModified(ifModifiedSince, lastModified);
|
notModified = isETagNotModified(ifNoneMatch, eTag) && isTimeStampNotModified(ifModifiedSince, lastModified);
|
||||||
}
|
}
|
||||||
else if (lastModified != -1) {
|
else if (lastModified != -1) {
|
||||||
|
@ -213,8 +217,7 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro
|
||||||
for (String clientETag : ifNoneMatch) {
|
for (String clientETag : ifNoneMatch) {
|
||||||
// Compare weak/strong ETags as per https://tools.ietf.org/html/rfc7232#section-2.3
|
// Compare weak/strong ETags as per https://tools.ietf.org/html/rfc7232#section-2.3
|
||||||
if (StringUtils.hasLength(clientETag) &&
|
if (StringUtils.hasLength(clientETag) &&
|
||||||
(clientETag.replaceFirst("^W/", "").equals(etag.replaceFirst("^W/", "")) ||
|
(clientETag.replaceFirst("^W/", "").equals(etag.replaceFirst("^W/", "")))) {
|
||||||
clientETag.equals("*"))) {
|
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -349,11 +349,9 @@ public class HttpEntityMethodProcessorMockTests {
|
||||||
|
|
||||||
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
|
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
|
||||||
|
|
||||||
assertTrue(mavContainer.isRequestHandled());
|
assertResponseNotModified();
|
||||||
assertEquals(HttpStatus.NOT_MODIFIED.value(), servletResponse.getStatus());
|
|
||||||
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.LAST_MODIFIED).size());
|
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.LAST_MODIFIED).size());
|
||||||
assertEquals(dateFormat.format(oneMinuteAgo), servletResponse.getHeader(HttpHeaders.LAST_MODIFIED));
|
assertEquals(dateFormat.format(oneMinuteAgo), servletResponse.getHeader(HttpHeaders.LAST_MODIFIED));
|
||||||
assertEquals(0, servletResponse.getContentAsByteArray().length);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
@ -370,11 +368,9 @@ public class HttpEntityMethodProcessorMockTests {
|
||||||
|
|
||||||
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
|
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
|
||||||
|
|
||||||
assertTrue(mavContainer.isRequestHandled());
|
assertResponseNotModified();
|
||||||
assertEquals(HttpStatus.NOT_MODIFIED.value(), servletResponse.getStatus());
|
|
||||||
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
|
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
|
||||||
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
|
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
|
||||||
assertEquals(0, servletResponse.getContentAsByteArray().length);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
@ -395,13 +391,11 @@ public class HttpEntityMethodProcessorMockTests {
|
||||||
|
|
||||||
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
|
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
|
||||||
|
|
||||||
assertTrue(mavContainer.isRequestHandled());
|
assertResponseNotModified();
|
||||||
assertEquals(HttpStatus.NOT_MODIFIED.value(), servletResponse.getStatus());
|
|
||||||
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.LAST_MODIFIED).size());
|
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.LAST_MODIFIED).size());
|
||||||
assertEquals(dateFormat.format(oneMinuteAgo), servletResponse.getHeader(HttpHeaders.LAST_MODIFIED));
|
assertEquals(dateFormat.format(oneMinuteAgo), servletResponse.getHeader(HttpHeaders.LAST_MODIFIED));
|
||||||
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
|
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
|
||||||
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
|
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
|
||||||
assertEquals(0, servletResponse.getContentAsByteArray().length);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
@ -420,12 +414,16 @@ public class HttpEntityMethodProcessorMockTests {
|
||||||
|
|
||||||
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
|
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
|
||||||
|
|
||||||
assertTrue(mavContainer.isRequestHandled());
|
assertResponseNotModified();
|
||||||
assertEquals(HttpStatus.NOT_MODIFIED.value(), servletResponse.getStatus());
|
|
||||||
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.LAST_MODIFIED).size());
|
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.LAST_MODIFIED).size());
|
||||||
assertEquals(dateFormat.format(oneMinuteAgo), servletResponse.getHeader(HttpHeaders.LAST_MODIFIED));
|
assertEquals(dateFormat.format(oneMinuteAgo), servletResponse.getHeader(HttpHeaders.LAST_MODIFIED));
|
||||||
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
|
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
|
||||||
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
|
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
|
||||||
|
}
|
||||||
|
|
||||||
|
private void assertResponseNotModified() {
|
||||||
|
assertTrue(mavContainer.isRequestHandled());
|
||||||
|
assertEquals(HttpStatus.NOT_MODIFIED.value(), servletResponse.getStatus());
|
||||||
assertEquals(0, servletResponse.getContentAsByteArray().length);
|
assertEquals(0, servletResponse.getContentAsByteArray().length);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -472,15 +470,81 @@ public class HttpEntityMethodProcessorMockTests {
|
||||||
given(messageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.TEXT_PLAIN));
|
given(messageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.TEXT_PLAIN));
|
||||||
given(messageConverter.canWrite(String.class, MediaType.TEXT_PLAIN)).willReturn(true);
|
given(messageConverter.canWrite(String.class, MediaType.TEXT_PLAIN)).willReturn(true);
|
||||||
|
|
||||||
|
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
|
||||||
|
|
||||||
|
assertResponseOkWithBody("body");
|
||||||
|
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
|
||||||
|
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
|
||||||
|
}
|
||||||
|
|
||||||
|
// SPR-13626
|
||||||
|
@Test
|
||||||
|
public void handleReturnTypeGetIfNoneMatchWildcard() throws Exception {
|
||||||
|
String wildcardValue = "*";
|
||||||
|
String etagValue = "\"some-etag\"";
|
||||||
|
servletRequest.addHeader(HttpHeaders.IF_NONE_MATCH, wildcardValue);
|
||||||
|
HttpHeaders responseHeaders = new HttpHeaders();
|
||||||
|
responseHeaders.set(HttpHeaders.ETAG, etagValue);
|
||||||
|
ResponseEntity<String> returnValue = new ResponseEntity<String>("body", responseHeaders, HttpStatus.OK);
|
||||||
|
|
||||||
|
given(messageConverter.canWrite(String.class, null)).willReturn(true);
|
||||||
|
given(messageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.TEXT_PLAIN));
|
||||||
|
given(messageConverter.canWrite(String.class, MediaType.TEXT_PLAIN)).willReturn(true);
|
||||||
|
|
||||||
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
|
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
|
||||||
|
|
||||||
assertTrue(mavContainer.isRequestHandled());
|
assertResponseOkWithBody("body");
|
||||||
assertEquals(HttpStatus.OK.value(), servletResponse.getStatus());
|
|
||||||
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
|
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
|
||||||
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
|
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
|
||||||
|
}
|
||||||
|
|
||||||
|
// SPR-13626
|
||||||
|
@Test
|
||||||
|
public void handleReturnTypeIfNoneMatchIfMatch() throws Exception {
|
||||||
|
String etagValue = "\"some-etag\"";
|
||||||
|
servletRequest.addHeader(HttpHeaders.IF_NONE_MATCH, etagValue);
|
||||||
|
servletRequest.addHeader(HttpHeaders.IF_MATCH, "ifmatch");
|
||||||
|
HttpHeaders responseHeaders = new HttpHeaders();
|
||||||
|
responseHeaders.set(HttpHeaders.ETAG, etagValue);
|
||||||
|
ResponseEntity<String> returnValue = new ResponseEntity<String>("body", responseHeaders, HttpStatus.OK);
|
||||||
|
|
||||||
|
given(messageConverter.canWrite(String.class, null)).willReturn(true);
|
||||||
|
given(messageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.TEXT_PLAIN));
|
||||||
|
given(messageConverter.canWrite(String.class, MediaType.TEXT_PLAIN)).willReturn(true);
|
||||||
|
|
||||||
|
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
|
||||||
|
|
||||||
|
assertResponseOkWithBody("body");
|
||||||
|
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
|
||||||
|
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
|
||||||
|
}
|
||||||
|
|
||||||
|
// SPR-13626
|
||||||
|
@Test
|
||||||
|
public void handleReturnTypeIfNoneMatchIfUnmodifiedSince() throws Exception {
|
||||||
|
String etagValue = "\"some-etag\"";
|
||||||
|
servletRequest.addHeader(HttpHeaders.IF_NONE_MATCH, etagValue);
|
||||||
|
servletRequest.addHeader(HttpHeaders.IF_UNMODIFIED_SINCE, dateFormat.format(new Date().getTime()));
|
||||||
|
HttpHeaders responseHeaders = new HttpHeaders();
|
||||||
|
responseHeaders.set(HttpHeaders.ETAG, etagValue);
|
||||||
|
ResponseEntity<String> returnValue = new ResponseEntity<String>("body", responseHeaders, HttpStatus.OK);
|
||||||
|
|
||||||
|
given(messageConverter.canWrite(String.class, null)).willReturn(true);
|
||||||
|
given(messageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.TEXT_PLAIN));
|
||||||
|
given(messageConverter.canWrite(String.class, MediaType.TEXT_PLAIN)).willReturn(true);
|
||||||
|
|
||||||
|
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
|
||||||
|
|
||||||
|
assertResponseOkWithBody("body");
|
||||||
|
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
|
||||||
|
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
|
||||||
|
}
|
||||||
|
|
||||||
|
private void assertResponseOkWithBody(String body) throws Exception {
|
||||||
|
assertTrue(mavContainer.isRequestHandled());
|
||||||
|
assertEquals(HttpStatus.OK.value(), servletResponse.getStatus());
|
||||||
ArgumentCaptor<HttpOutputMessage> outputMessage = ArgumentCaptor.forClass(HttpOutputMessage.class);
|
ArgumentCaptor<HttpOutputMessage> outputMessage = ArgumentCaptor.forClass(HttpOutputMessage.class);
|
||||||
verify(messageConverter).write(eq("body"), eq(MediaType.TEXT_PLAIN), outputMessage.capture());
|
verify(messageConverter).write(eq("body"), eq(MediaType.TEXT_PLAIN), outputMessage.capture());
|
||||||
}
|
}
|
||||||
|
|
||||||
@SuppressWarnings("unused")
|
@SuppressWarnings("unused")
|
||||||
|
|
Loading…
Reference in New Issue