Skip to content

Commit 798d866

Browse files
committed
Fix server errors for invalid If-None-Match request headers
HttpEntityMethodProcessor should not throw IllegalArgumentExceptions for invalid If-None-Match headers. For those cases, this commit makes sure that both `HttpEntityMethodProcessor` and `ServletWebRequest` have a consistent behavior and stop processing the request as conditional and leave the handler handle it. Issue: SPR-14559
1 parent 2a82b8f commit 798d866

File tree

3 files changed

+44
-16
lines changed

3 files changed

+44
-16
lines changed

spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestHttpMethodsTests.java

+9
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,15 @@ public void checkNotModifiedInvalidStatus() {
9999
assertEquals(dateFormat.format(epochTime), servletResponse.getHeader("Last-Modified"));
100100
}
101101

102+
@Test // SPR-14559
103+
public void checkNotModifiedInvalidIfNoneMatchHeader() {
104+
String eTag = "\"etagvalue\"";
105+
servletRequest.addHeader("If-None-Match", "missingquotes");
106+
assertFalse(request.checkNotModified(eTag));
107+
assertEquals(200, servletResponse.getStatus());
108+
assertEquals(eTag, servletResponse.getHeader("ETag"));
109+
}
110+
102111
@Test
103112
public void checkNotModifiedHeaderAlreadySet() {
104113
long epochTime = currentDate.getTime();

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

+20-16
Original file line numberDiff line numberDiff line change
@@ -223,24 +223,28 @@ private List<String> getVaryRequestHeadersToAdd(HttpHeaders responseHeaders, Htt
223223
}
224224

225225
private boolean isResourceNotModified(ServletServerHttpRequest inputMessage, ServletServerHttpResponse outputMessage) {
226-
List<String> ifNoneMatch = inputMessage.getHeaders().getIfNoneMatch();
227-
long ifModifiedSince = inputMessage.getHeaders().getIfModifiedSince();
228-
String eTag = addEtagPadding(outputMessage.getHeaders().getETag());
229-
long lastModified = outputMessage.getHeaders().getLastModified();
230226
boolean notModified = false;
231-
232-
if (!ifNoneMatch.isEmpty() && (inputMessage.getHeaders().containsKey(HttpHeaders.IF_UNMODIFIED_SINCE)
233-
|| inputMessage.getHeaders().containsKey(HttpHeaders.IF_MATCH))) {
234-
// invalid conditional request, do not process
235-
}
236-
else if (lastModified != -1 && StringUtils.hasLength(eTag)) {
237-
notModified = isETagNotModified(ifNoneMatch, eTag) && isTimeStampNotModified(ifModifiedSince, lastModified);
238-
}
239-
else if (lastModified != -1) {
240-
notModified = isTimeStampNotModified(ifModifiedSince, lastModified);
227+
try {
228+
long ifModifiedSince = inputMessage.getHeaders().getIfModifiedSince();
229+
String eTag = addEtagPadding(outputMessage.getHeaders().getETag());
230+
long lastModified = outputMessage.getHeaders().getLastModified();
231+
List<String> ifNoneMatch = inputMessage.getHeaders().getIfNoneMatch();
232+
if (!ifNoneMatch.isEmpty() && (inputMessage.getHeaders().containsKey(HttpHeaders.IF_UNMODIFIED_SINCE)
233+
|| inputMessage.getHeaders().containsKey(HttpHeaders.IF_MATCH))) {
234+
// invalid conditional request, do not process
235+
}
236+
else if (lastModified != -1 && StringUtils.hasLength(eTag)) {
237+
notModified = isETagNotModified(ifNoneMatch, eTag) && isTimeStampNotModified(ifModifiedSince, lastModified);
238+
}
239+
else if (lastModified != -1) {
240+
notModified = isTimeStampNotModified(ifModifiedSince, lastModified);
241+
}
242+
else if (StringUtils.hasLength(eTag)) {
243+
notModified = isETagNotModified(ifNoneMatch, eTag);
244+
}
241245
}
242-
else if (StringUtils.hasLength(eTag)) {
243-
notModified = isETagNotModified(ifNoneMatch, eTag);
246+
catch (IllegalArgumentException exc) {
247+
// invalid conditional request, do not process
244248
}
245249
return notModified;
246250
}

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

+15
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,21 @@ public void handleReturnValueEtag() throws Exception {
379379
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
380380
}
381381

382+
@Test // SPR-14559
383+
public void handleReturnValueEtagInvalidIfNoneMatch() throws Exception {
384+
String etagValue = "\"deadb33f8badf00d\"";
385+
servletRequest.addHeader(HttpHeaders.IF_NONE_MATCH, "unquoted");
386+
HttpHeaders responseHeaders = new HttpHeaders();
387+
responseHeaders.set(HttpHeaders.ETAG, etagValue);
388+
ResponseEntity<String> returnValue = new ResponseEntity<>("body", responseHeaders, HttpStatus.OK);
389+
390+
initStringMessageConversion(MediaType.TEXT_PLAIN);
391+
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
392+
393+
assertTrue(mavContainer.isRequestHandled());
394+
assertEquals(HttpStatus.OK.value(), servletResponse.getStatus());
395+
}
396+
382397
@Test
383398
public void handleReturnValueETagAndLastModified() throws Exception {
384399
long currentTime = new Date().getTime();

0 commit comments

Comments
 (0)