From 794e859e68b4679b43f0a0e153b04715616f8c9e Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 23 Apr 2014 18:16:43 +0200 Subject: [PATCH] checkNotModified leniently handles IE-10-style If-Modified-Since values and silently proceeds if header value cannot be parsed at all Issue: SPR-11727 --- .../context/request/ServletWebRequest.java | 45 +++++++--- .../web/context/request/WebRequest.java | 13 +-- .../request/ServletWebRequestTests.java | 84 +++++++++++-------- 3 files changed, 88 insertions(+), 54 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java b/spring-web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java index e0a83370f7..d41b258b80 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java @@ -17,6 +17,7 @@ package org.springframework.web.context.request; import java.security.Principal; +import java.util.Date; import java.util.Iterator; import java.util.Locale; import java.util.Map; @@ -103,6 +104,14 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ } + /** + * Return the HTTP method of the request. + * @since 4.0.2 + */ + public HttpMethod getHttpMethod() { + return HttpMethod.valueOf(getRequest().getMethod().trim().toUpperCase()); + } + @Override public String getHeader(String headerName) { return getRequest().getHeader(headerName); @@ -170,10 +179,28 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ } @Override + @SuppressWarnings("deprecation") public boolean checkNotModified(long lastModifiedTimestamp) { if (lastModifiedTimestamp >= 0 && !this.notModified && (this.response == null || !this.response.containsHeader(HEADER_LAST_MODIFIED))) { - long ifModifiedSince = getRequest().getDateHeader(HEADER_IF_MODIFIED_SINCE); + long ifModifiedSince = -1; + try { + ifModifiedSince = getRequest().getDateHeader(HEADER_IF_MODIFIED_SINCE); + } + catch (IllegalArgumentException ex) { + String headerValue = getRequest().getHeader(HEADER_IF_MODIFIED_SINCE); + // Possibly an IE 10 style value: "Wed, 09 Apr 2014 09:57:42 GMT; length=13774" + int separatorIndex = headerValue.indexOf(';'); + if (separatorIndex != -1) { + String datePart = headerValue.substring(0, separatorIndex); + try { + ifModifiedSince = Date.parse(datePart); + } + catch (IllegalArgumentException ex2) { + // Giving up + } + } + } this.notModified = (ifModifiedSince >= (lastModifiedTimestamp / 1000 * 1000)); if (this.response != null) { if (this.notModified && supportsNotModifiedStatus()) { @@ -188,17 +215,17 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ } @Override - public boolean checkNotModified(String eTag) { - if (StringUtils.hasLength(eTag) && !this.notModified && + public boolean checkNotModified(String etag) { + if (StringUtils.hasLength(etag) && !this.notModified && (this.response == null || !this.response.containsHeader(HEADER_ETAG))) { String ifNoneMatch = getRequest().getHeader(HEADER_IF_NONE_MATCH); - this.notModified = eTag.equals(ifNoneMatch); + this.notModified = etag.equals(ifNoneMatch); if (this.response != null) { if (this.notModified && supportsNotModifiedStatus()) { this.response.setStatus(HttpServletResponse.SC_NOT_MODIFIED); } else { - this.response.setHeader(HEADER_ETAG, eTag); + this.response.setHeader(HEADER_ETAG, etag); } } } @@ -236,14 +263,6 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ return sb.toString(); } - /** - * Return the HTTP method of the request. - * @since 4.0.2 - */ - public HttpMethod getHttpMethod() { - return HttpMethod.valueOf(getRequest().getMethod().trim().toUpperCase()); - } - @Override public String toString() { diff --git a/spring-web/src/main/java/org/springframework/web/context/request/WebRequest.java b/spring-web/src/main/java/org/springframework/web/context/request/WebRequest.java index cce42d4c5f..401451494f 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/WebRequest.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/WebRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2014 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -142,8 +142,11 @@ public interface WebRequest extends RequestAttributes { * return "myViewName"; * } *

Note: that you typically want to use either - * this {@link #checkNotModified(long)} method; or + * this {@code #checkNotModified(long)} method; or * {@link #checkNotModified(String)}, but not both. + *

If the "If-Modified-Since" header is set but cannot be parsed + * to a date value, this method will ignore the header and proceed + * with setting the last-modified timestamp on the response. * @param lastModifiedTimestamp the last-modified timestamp that * the application determined for the underlying resource * @return whether the request qualifies as not modified, @@ -170,16 +173,16 @@ public interface WebRequest extends RequestAttributes { * return "myViewName"; * } *

Note: that you typically want to use either - * this {@link #checkNotModified(String)} method; or + * this {@code #checkNotModified(String)} method; or * {@link #checkNotModified(long)}, but not both. - * @param eTag the entity tag that the application determined + * @param etag the entity tag that the application determined * for the underlying resource. This parameter will be padded * with quotes (") if necessary. * @return whether the request qualifies as not modified, * allowing to abort request processing and relying on the response * telling the client that the content has not been modified */ - boolean checkNotModified(String eTag); + boolean checkNotModified(String etag); /** * Get a short description of this request, diff --git a/spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestTests.java b/spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestTests.java index 540160b6fc..dc3ad4b062 100644 --- a/spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestTests.java +++ b/spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestTests.java @@ -19,7 +19,6 @@ package org.springframework.web.context.request; import java.util.Date; import java.util.Locale; import java.util.Map; - import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; @@ -118,75 +117,90 @@ public class ServletWebRequestTests { } @Test - public void checkNotModifiedTimeStampForGET() { + public void checkNotModifiedTimestampForGET() { long currentTime = new Date().getTime(); servletRequest.setMethod("GET"); servletRequest.addHeader("If-Modified-Since", currentTime); - request.checkNotModified(currentTime); - + assertTrue(request.checkNotModified(currentTime)); assertEquals(304, servletResponse.getStatus()); } @Test - public void checkModifiedTimeStampForGET() { + public void checkModifiedTimestampForGET() { long currentTime = new Date().getTime(); long oneMinuteAgo = currentTime - (1000 * 60); servletRequest.setMethod("GET"); servletRequest.addHeader("If-Modified-Since", oneMinuteAgo); - request.checkNotModified(currentTime); + assertFalse(request.checkNotModified(currentTime)); + assertEquals(200, servletResponse.getStatus()); + assertEquals("" + currentTime, servletResponse.getHeader("Last-Modified")); + } + @Test + public void checkNotModifiedTimestampForHEAD() { + long currentTime = new Date().getTime(); + servletRequest.setMethod("HEAD"); + servletRequest.addHeader("If-Modified-Since", currentTime); + + assertTrue(request.checkNotModified(currentTime)); + assertEquals(304, servletResponse.getStatus()); + } + + @Test + public void checkModifiedTimestampForHEAD() { + long currentTime = new Date().getTime(); + long oneMinuteAgo = currentTime - (1000 * 60); + servletRequest.setMethod("HEAD"); + servletRequest.addHeader("If-Modified-Since", oneMinuteAgo); + + assertFalse(request.checkNotModified(currentTime)); assertEquals(200, servletResponse.getStatus()); assertEquals(""+currentTime, servletResponse.getHeader("Last-Modified")); } @Test - public void checkNotModifiedETagForGET() { - String eTag = "\"Foo\""; + public void checkNotModifiedTimestampWithLengthPart() { + long currentTime = Date.parse("Wed, 09 Apr 2014 09:57:42 GMT"); servletRequest.setMethod("GET"); - servletRequest.addHeader("If-None-Match", eTag ); - - request.checkNotModified(eTag); + servletRequest.addHeader("If-Modified-Since", "Wed, 09 Apr 2014 09:57:42 GMT; length=13774"); + assertTrue(request.checkNotModified(currentTime)); assertEquals(304, servletResponse.getStatus()); } @Test - public void checkModifiedETagForGET() { - String currentETag = "\"Foo\""; - String oldEtag = "Bar"; + public void checkModifiedTimestampWithLengthPart() { + long currentTime = Date.parse("Wed, 09 Apr 2014 09:57:42 GMT"); servletRequest.setMethod("GET"); - servletRequest.addHeader("If-None-Match", oldEtag); - - request.checkNotModified(currentETag); + servletRequest.addHeader("If-Modified-Since", "Wed, 08 Apr 2014 09:57:42 GMT; length=13774"); + assertFalse(request.checkNotModified(currentTime)); assertEquals(200, servletResponse.getStatus()); - assertEquals(currentETag, servletResponse.getHeader("ETag")); + assertEquals("" + currentTime, servletResponse.getHeader("Last-Modified")); } @Test - public void checkNotModifiedTimeStampForHEAD() { - long currentTime = new Date().getTime(); - servletRequest.setMethod("HEAD"); - servletRequest.addHeader("If-Modified-Since", currentTime); - - request.checkNotModified(currentTime); + public void checkNotModifiedETagForGET() { + String eTag = "\"Foo\""; + servletRequest.setMethod("GET"); + servletRequest.addHeader("If-None-Match", eTag ); + assertTrue(request.checkNotModified(eTag)); assertEquals(304, servletResponse.getStatus()); } @Test - public void checkModifiedTimeStampForHEAD() { - long currentTime = new Date().getTime(); - long oneMinuteAgo = currentTime - (1000 * 60); - servletRequest.setMethod("HEAD"); - servletRequest.addHeader("If-Modified-Since", oneMinuteAgo); - - request.checkNotModified(currentTime); + public void checkModifiedETagForGET() { + String currentETag = "\"Foo\""; + String oldEtag = "Bar"; + servletRequest.setMethod("GET"); + servletRequest.addHeader("If-None-Match", oldEtag); + assertFalse(request.checkNotModified(currentETag)); assertEquals(200, servletResponse.getStatus()); - assertEquals(""+currentTime, servletResponse.getHeader("Last-Modified")); + assertEquals(currentETag, servletResponse.getHeader("ETag")); } @Test @@ -195,8 +209,7 @@ public class ServletWebRequestTests { servletRequest.setMethod("HEAD"); servletRequest.addHeader("If-None-Match", eTag ); - request.checkNotModified(eTag); - + assertTrue(request.checkNotModified(eTag)); assertEquals(304, servletResponse.getStatus()); } @@ -207,8 +220,7 @@ public class ServletWebRequestTests { servletRequest.setMethod("HEAD"); servletRequest.addHeader("If-None-Match", oldEtag); - request.checkNotModified(currentETag); - + assertFalse(request.checkNotModified(currentETag)); assertEquals(200, servletResponse.getStatus()); assertEquals(currentETag, servletResponse.getHeader("ETag")); }