Skip to content

Commit 8893663

Browse files
bclozelbclozel
committed
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
1 parent 9f4c9d7 commit 8893663

File tree

2 files changed

+85
-18
lines changed

2 files changed

+85
-18
lines changed

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ public void handleReturnValue(Object returnValue, MethodParameter returnType,
172172
Object body = responseEntity.getBody();
173173
if (responseEntity instanceof ResponseEntity) {
174174
outputMessage.setStatusCode(((ResponseEntity<?>) responseEntity).getStatusCode());
175-
if (inputMessage.getMethod() == HttpMethod.GET &&
175+
if (inputMessage.getMethod().equals(HttpMethod.GET) &&
176176
isResourceNotModified(inputMessage, outputMessage)) {
177177
outputMessage.setStatusCode(HttpStatus.NOT_MODIFIED);
178178
// Ensure headers are flushed, no body should be written.
@@ -196,7 +196,11 @@ private boolean isResourceNotModified(ServletServerHttpRequest inputMessage, Ser
196196
long lastModified = outputMessage.getHeaders().getLastModified();
197197
boolean notModified = false;
198198

199-
if (lastModified != -1 && StringUtils.hasLength(eTag)) {
199+
if (!ifNoneMatch.isEmpty() && (inputMessage.getHeaders().containsKey(HttpHeaders.IF_UNMODIFIED_SINCE)
200+
|| inputMessage.getHeaders().containsKey(HttpHeaders.IF_MATCH))) {
201+
// invalid conditional request, do not process
202+
}
203+
else if (lastModified != -1 && StringUtils.hasLength(eTag)) {
200204
notModified = isETagNotModified(ifNoneMatch, eTag) && isTimeStampNotModified(ifModifiedSince, lastModified);
201205
}
202206
else if (lastModified != -1) {
@@ -213,8 +217,7 @@ private boolean isETagNotModified(List<String> ifNoneMatch, String etag) {
213217
for (String clientETag : ifNoneMatch) {
214218
// Compare weak/strong ETags as per https://tools.ietf.org/html/rfc7232#section-2.3
215219
if (StringUtils.hasLength(clientETag) &&
216-
(clientETag.replaceFirst("^W/", "").equals(etag.replaceFirst("^W/", "")) ||
217-
clientETag.equals("*"))) {
220+
(clientETag.replaceFirst("^W/", "").equals(etag.replaceFirst("^W/", "")))) {
218221
return true;
219222
}
220223
}

‎spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java‎

Lines changed: 78 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -349,11 +349,9 @@ public void handleReturnTypeLastModified() throws Exception {
349349

350350
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
351351

352-
assertTrue(mavContainer.isRequestHandled());
353-
assertEquals(HttpStatus.NOT_MODIFIED.value(), servletResponse.getStatus());
352+
assertResponseNotModified();
354353
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.LAST_MODIFIED).size());
355354
assertEquals(dateFormat.format(oneMinuteAgo), servletResponse.getHeader(HttpHeaders.LAST_MODIFIED));
356-
assertEquals(0, servletResponse.getContentAsByteArray().length);
357355
}
358356

359357
@Test
@@ -370,11 +368,9 @@ public void handleReturnTypeEtag() throws Exception {
370368

371369
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
372370

373-
assertTrue(mavContainer.isRequestHandled());
374-
assertEquals(HttpStatus.NOT_MODIFIED.value(), servletResponse.getStatus());
371+
assertResponseNotModified();
375372
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
376373
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
377-
assertEquals(0, servletResponse.getContentAsByteArray().length);
378374
}
379375

380376
@Test
@@ -395,13 +391,11 @@ public void handleReturnTypeETagAndLastModified() throws Exception {
395391

396392
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
397393

398-
assertTrue(mavContainer.isRequestHandled());
399-
assertEquals(HttpStatus.NOT_MODIFIED.value(), servletResponse.getStatus());
394+
assertResponseNotModified();
400395
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.LAST_MODIFIED).size());
401396
assertEquals(dateFormat.format(oneMinuteAgo), servletResponse.getHeader(HttpHeaders.LAST_MODIFIED));
402397
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
403398
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
404-
assertEquals(0, servletResponse.getContentAsByteArray().length);
405399
}
406400

407401
@Test
@@ -420,12 +414,16 @@ public void handleReturnTypeNotModified() throws Exception {
420414

421415
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
422416

423-
assertTrue(mavContainer.isRequestHandled());
424-
assertEquals(HttpStatus.NOT_MODIFIED.value(), servletResponse.getStatus());
417+
assertResponseNotModified();
425418
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.LAST_MODIFIED).size());
426419
assertEquals(dateFormat.format(oneMinuteAgo), servletResponse.getHeader(HttpHeaders.LAST_MODIFIED));
427420
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
428421
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
422+
}
423+
424+
private void assertResponseNotModified() {
425+
assertTrue(mavContainer.isRequestHandled());
426+
assertEquals(HttpStatus.NOT_MODIFIED.value(), servletResponse.getStatus());
429427
assertEquals(0, servletResponse.getContentAsByteArray().length);
430428
}
431429

@@ -472,15 +470,81 @@ public void handleReturnTypePostRequestWithIfNotModified() throws Exception {
472470
given(messageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.TEXT_PLAIN));
473471
given(messageConverter.canWrite(String.class, MediaType.TEXT_PLAIN)).willReturn(true);
474472

473+
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
474+
475+
assertResponseOkWithBody("body");
476+
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
477+
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
478+
}
479+
480+
// SPR-13626
481+
@Test
482+
public void handleReturnTypeGetIfNoneMatchWildcard() throws Exception {
483+
String wildcardValue = "*";
484+
String etagValue = "\"some-etag\"";
485+
servletRequest.addHeader(HttpHeaders.IF_NONE_MATCH, wildcardValue);
486+
HttpHeaders responseHeaders = new HttpHeaders();
487+
responseHeaders.set(HttpHeaders.ETAG, etagValue);
488+
ResponseEntity<String> returnValue = new ResponseEntity<String>("body", responseHeaders, HttpStatus.OK);
489+
490+
given(messageConverter.canWrite(String.class, null)).willReturn(true);
491+
given(messageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.TEXT_PLAIN));
492+
given(messageConverter.canWrite(String.class, MediaType.TEXT_PLAIN)).willReturn(true);
475493

476494
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
477495

478-
assertTrue(mavContainer.isRequestHandled());
479-
assertEquals(HttpStatus.OK.value(), servletResponse.getStatus());
496+
assertResponseOkWithBody("body");
480497
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
481498
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
499+
}
500+
501+
// SPR-13626
502+
@Test
503+
public void handleReturnTypeIfNoneMatchIfMatch() throws Exception {
504+
String etagValue = "\"some-etag\"";
505+
servletRequest.addHeader(HttpHeaders.IF_NONE_MATCH, etagValue);
506+
servletRequest.addHeader(HttpHeaders.IF_MATCH, "ifmatch");
507+
HttpHeaders responseHeaders = new HttpHeaders();
508+
responseHeaders.set(HttpHeaders.ETAG, etagValue);
509+
ResponseEntity<String> returnValue = new ResponseEntity<String>("body", responseHeaders, HttpStatus.OK);
510+
511+
given(messageConverter.canWrite(String.class, null)).willReturn(true);
512+
given(messageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.TEXT_PLAIN));
513+
given(messageConverter.canWrite(String.class, MediaType.TEXT_PLAIN)).willReturn(true);
514+
515+
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
516+
517+
assertResponseOkWithBody("body");
518+
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
519+
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
520+
}
521+
522+
// SPR-13626
523+
@Test
524+
public void handleReturnTypeIfNoneMatchIfUnmodifiedSince() throws Exception {
525+
String etagValue = "\"some-etag\"";
526+
servletRequest.addHeader(HttpHeaders.IF_NONE_MATCH, etagValue);
527+
servletRequest.addHeader(HttpHeaders.IF_UNMODIFIED_SINCE, dateFormat.format(new Date().getTime()));
528+
HttpHeaders responseHeaders = new HttpHeaders();
529+
responseHeaders.set(HttpHeaders.ETAG, etagValue);
530+
ResponseEntity<String> returnValue = new ResponseEntity<String>("body", responseHeaders, HttpStatus.OK);
531+
532+
given(messageConverter.canWrite(String.class, null)).willReturn(true);
533+
given(messageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.TEXT_PLAIN));
534+
given(messageConverter.canWrite(String.class, MediaType.TEXT_PLAIN)).willReturn(true);
535+
536+
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
537+
538+
assertResponseOkWithBody("body");
539+
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
540+
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
541+
}
542+
543+
private void assertResponseOkWithBody(String body) throws Exception {
544+
assertTrue(mavContainer.isRequestHandled());
545+
assertEquals(HttpStatus.OK.value(), servletResponse.getStatus());
482546
ArgumentCaptor<HttpOutputMessage> outputMessage = ArgumentCaptor.forClass(HttpOutputMessage.class);
483-
verify(messageConverter).write(eq("body"), eq(MediaType.TEXT_PLAIN), outputMessage.capture());
547+
verify(messageConverter).write(eq("body"), eq(MediaType.TEXT_PLAIN), outputMessage.capture());
484548
}
485549

486550
@SuppressWarnings("unused")

0 commit comments

Comments
 (0)