From 80a0cf71f4b7d244e1832656a76810d4c79b3f1a Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Sun, 5 Nov 2017 16:12:21 +0100 Subject: [PATCH] MockHttpServletResponse.getDateHeader returns -1 for non-existent header Includes consistent getDateHeader results in both MockHttpServletResponse variants (spring-test and spring-web) Issue: SPR-16160 --- .../mock/web/MockHttpServletResponse.java | 25 +-- .../web/MockHttpServletResponseTests.java | 6 + .../web/test/MockHttpServletResponse.java | 51 +++--- .../ServletWebRequestHttpMethodsTests.java | 151 +++++++----------- 4 files changed, 113 insertions(+), 120 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java b/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java index 3bd86b1d19e..5433cee745c 100644 --- a/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java +++ b/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java @@ -23,6 +23,7 @@ import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.io.UnsupportedEncodingException; import java.io.Writer; +import java.text.DateFormat; import java.text.ParseException; import java.text.SimpleDateFormat; import java.util.ArrayList; @@ -517,27 +518,33 @@ public class MockHttpServletResponse implements HttpServletResponse { setHeaderValue(name, formatDate(value)); } + @Override + public void addDateHeader(String name, long value) { + addHeaderValue(name, formatDate(value)); + } + public long getDateHeader(String name) { - SimpleDateFormat dateFormat = new SimpleDateFormat(DATE_FORMAT, Locale.US); - dateFormat.setTimeZone(GMT); + String headerValue = getHeader(name); + if (headerValue == null) { + return -1; + } try { - return dateFormat.parse(getHeader(name)).getTime(); + return newDateFormat().parse(getHeader(name)).getTime(); } catch (ParseException ex) { throw new IllegalArgumentException( - "Value for header '" + name + "' is not a valid Date: " + getHeader(name)); + "Value for header '" + name + "' is not a valid Date: " + headerValue); } } - @Override - public void addDateHeader(String name, long value) { - addHeaderValue(name, formatDate(value)); + private String formatDate(long date) { + return newDateFormat().format(new Date(date)); } - private String formatDate(long date) { + private DateFormat newDateFormat() { SimpleDateFormat dateFormat = new SimpleDateFormat(DATE_FORMAT, Locale.US); dateFormat.setTimeZone(GMT); - return dateFormat.format(new Date(date)); + return dateFormat; } @Override diff --git a/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletResponseTests.java b/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletResponseTests.java index fef993a0a1e..ec9029c8f69 100644 --- a/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletResponseTests.java +++ b/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletResponseTests.java @@ -291,6 +291,12 @@ public class MockHttpServletResponseTests { response.getDateHeader("Last-Modified"); } + @Test // SPR-16160 + public void getNonExistentDateHeader() { + assertNull(response.getHeader("Last-Modified")); + assertEquals(-1, response.getDateHeader("Last-Modified")); + } + @Test // SPR-10414 public void modifyStatusAfterSendError() throws IOException { response.sendError(HttpServletResponse.SC_NOT_FOUND); diff --git a/spring-web/src/test/java/org/springframework/mock/web/test/MockHttpServletResponse.java b/spring-web/src/test/java/org/springframework/mock/web/test/MockHttpServletResponse.java index a43c1aaf802..c79dad7a3c4 100644 --- a/spring-web/src/test/java/org/springframework/mock/web/test/MockHttpServletResponse.java +++ b/spring-web/src/test/java/org/springframework/mock/web/test/MockHttpServletResponse.java @@ -23,16 +23,17 @@ import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.io.UnsupportedEncodingException; import java.io.Writer; -import java.time.Instant; -import java.time.ZoneId; -import java.time.ZonedDateTime; -import java.time.format.DateTimeParseException; +import java.text.DateFormat; +import java.text.ParseException; +import java.text.SimpleDateFormat; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.Date; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.TimeZone; import javax.servlet.ServletOutputStream; import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletResponse; @@ -44,8 +45,6 @@ import org.springframework.util.LinkedCaseInsensitiveMap; import org.springframework.util.StringUtils; import org.springframework.web.util.WebUtils; -import static java.time.format.DateTimeFormatter.*; - /** * Mock implementation of the {@link javax.servlet.http.HttpServletResponse} interface. * @@ -60,7 +59,9 @@ public class MockHttpServletResponse implements HttpServletResponse { private static final String CHARSET_PREFIX = "charset="; - private static final ZoneId GMT = ZoneId.of("GMT"); + private static final String DATE_FORMAT = "EEE, dd MMM yyyy HH:mm:ss zzz"; + + private static final TimeZone GMT = TimeZone.getTimeZone("GMT"); //--------------------------------------------------------------------- @@ -292,7 +293,7 @@ public class MockHttpServletResponse implements HttpServletResponse { this.characterEncoding = null; this.contentLength = 0; this.contentType = null; - this.locale = null; + this.locale = Locale.getDefault(); this.cookies.clear(); this.headers.clear(); this.status = HttpServletResponse.SC_OK; @@ -302,9 +303,7 @@ public class MockHttpServletResponse implements HttpServletResponse { @Override public void setLocale(Locale locale) { this.locale = locale; - if (locale != null) { - doAddHeaderValue(HttpHeaders.ACCEPT_LANGUAGE, locale.toLanguageTag(), true); - } + doAddHeaderValue(HttpHeaders.ACCEPT_LANGUAGE, locale.toLanguageTag(), true); } @Override @@ -507,25 +506,33 @@ public class MockHttpServletResponse implements HttpServletResponse { setHeaderValue(name, formatDate(value)); } + @Override + public void addDateHeader(String name, long value) { + addHeaderValue(name, formatDate(value)); + } + public long getDateHeader(String name) { + String headerValue = getHeader(name); + if (headerValue == null) { + return -1; + } try { - return ZonedDateTime.parse(getHeader(name), RFC_1123_DATE_TIME).toInstant().toEpochMilli(); + return newDateFormat().parse(getHeader(name)).getTime(); } - catch (DateTimeParseException ex) { + catch (ParseException ex) { throw new IllegalArgumentException( - "Value for header '" + name + "' is not a valid Date: " + getHeader(name)); + "Value for header '" + name + "' is not a valid Date: " + headerValue); } } - @Override - public void addDateHeader(String name, long value) { - addHeaderValue(name, formatDate(value)); + private String formatDate(long date) { + return newDateFormat().format(new Date(date)); } - private String formatDate(long date) { - Instant instant = Instant.ofEpochMilli(date); - ZonedDateTime zonedDateTime = ZonedDateTime.ofInstant(instant, GMT); - return RFC_1123_DATE_TIME.format(zonedDateTime); + private DateFormat newDateFormat() { + SimpleDateFormat dateFormat = new SimpleDateFormat(DATE_FORMAT, Locale.US); + dateFormat.setTimeZone(GMT); + return dateFormat; } @Override @@ -576,7 +583,7 @@ public class MockHttpServletResponse implements HttpServletResponse { HttpHeaders headers = new HttpHeaders(); headers.add(HttpHeaders.ACCEPT_LANGUAGE, value.toString()); List locales = headers.getAcceptLanguageAsLocales(); - setLocale(locales.isEmpty() ? null : locales.get(0)); + this.locale = (!locales.isEmpty() ? locales.get(0) : Locale.getDefault()); return true; } else { diff --git a/spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestHttpMethodsTests.java b/spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestHttpMethodsTests.java index ae0a9dfdee4..969d66499f3 100644 --- a/spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestHttpMethodsTests.java +++ b/spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestHttpMethodsTests.java @@ -16,8 +16,6 @@ package org.springframework.web.context.request; -import java.time.Instant; -import java.time.ZoneId; import java.time.ZonedDateTime; import java.util.Arrays; import java.util.Date; @@ -36,7 +34,8 @@ import static java.time.format.DateTimeFormatter.*; import static org.junit.Assert.*; /** - * Parameterized tests for ServletWebRequest + * Parameterized tests for {@link ServletWebRequest}. + * * @author Juergen Hoeller * @author Brian Clozel * @author Markus Malkusch @@ -44,8 +43,6 @@ import static org.junit.Assert.*; @RunWith(Parameterized.class) public class ServletWebRequestHttpMethodsTests { - private static final ZoneId GMT = ZoneId.of("GMT"); - private static final String CURRENT_TIME = "Wed, 9 Apr 2014 09:57:42 GMT"; private MockHttpServletRequest servletRequest; @@ -62,8 +59,7 @@ public class ServletWebRequestHttpMethodsTests { @Parameters(name = "{0}") static public Iterable safeMethods() { return Arrays.asList(new Object[][] { - {"GET"}, - {"HEAD"} + {"GET"}, {"HEAD"} }); } @@ -99,11 +95,11 @@ public class ServletWebRequestHttpMethodsTests { @Test // SPR-14559 public void checkNotModifiedInvalidIfNoneMatchHeader() { - String eTag = "\"etagvalue\""; + String etag = "\"etagvalue\""; servletRequest.addHeader("If-None-Match", "missingquotes"); - assertFalse(request.checkNotModified(eTag)); + assertFalse(request.checkNotModified(etag)); assertEquals(200, servletResponse.getStatus()); - assertEquals(eTag, servletResponse.getHeader("ETag")); + assertEquals(etag, servletResponse.getHeader("ETag")); } @Test @@ -119,15 +115,13 @@ public class ServletWebRequestHttpMethodsTests { } @Test - public void checkNotModifiedTimestamp() throws Exception { + public void checkNotModifiedTimestamp() { long epochTime = currentDate.getTime(); servletRequest.addHeader("If-Modified-Since", epochTime); assertTrue(request.checkNotModified(epochTime)); - assertEquals(304, servletResponse.getStatus()); - assertEquals(RFC_1123_DATE_TIME.format(Instant.ofEpochMilli(epochTime).atZone(GMT)), - servletResponse.getHeader("Last-Modified")); + assertEquals(currentDate.getTime() / 1000, servletResponse.getDateHeader("Last-Modified") / 1000); } @Test @@ -136,191 +130,170 @@ public class ServletWebRequestHttpMethodsTests { servletRequest.addHeader("If-Modified-Since", oneMinuteAgo); assertFalse(request.checkNotModified(currentDate.getTime())); - assertEquals(200, servletResponse.getStatus()); - assertEquals(RFC_1123_DATE_TIME.format(currentDate.toInstant().atZone(GMT)), - servletResponse.getHeader("Last-Modified")); + assertEquals(currentDate.getTime() / 1000, servletResponse.getDateHeader("Last-Modified") / 1000); } @Test public void checkNotModifiedETag() { - String eTag = "\"Foo\""; - servletRequest.addHeader("If-None-Match", eTag); - - assertTrue(request.checkNotModified(eTag)); + String etag = "\"Foo\""; + servletRequest.addHeader("If-None-Match", etag); + assertTrue(request.checkNotModified(etag)); assertEquals(304, servletResponse.getStatus()); - assertEquals(eTag, servletResponse.getHeader("ETag")); + assertEquals(etag, servletResponse.getHeader("ETag")); } @Test public void checkNotModifiedETagWithSeparatorChars() { - String eTag = "\"Foo, Bar\""; - servletRequest.addHeader("If-None-Match", eTag); - - assertTrue(request.checkNotModified(eTag)); + String etag = "\"Foo, Bar\""; + servletRequest.addHeader("If-None-Match", etag); + assertTrue(request.checkNotModified(etag)); assertEquals(304, servletResponse.getStatus()); - assertEquals(eTag, servletResponse.getHeader("ETag")); + assertEquals(etag, servletResponse.getHeader("ETag")); } @Test public void checkModifiedETag() { String currentETag = "\"Foo\""; - String oldEtag = "Bar"; - servletRequest.addHeader("If-None-Match", oldEtag); + String oldETag = "Bar"; + servletRequest.addHeader("If-None-Match", oldETag); assertFalse(request.checkNotModified(currentETag)); - assertEquals(200, servletResponse.getStatus()); assertEquals(currentETag, servletResponse.getHeader("ETag")); } @Test public void checkNotModifiedUnpaddedETag() { - String eTag = "Foo"; - String paddedEtag = String.format("\"%s\"", eTag); - servletRequest.addHeader("If-None-Match", paddedEtag); - - assertTrue(request.checkNotModified(eTag)); + String etag = "Foo"; + String paddedETag = String.format("\"%s\"", etag); + servletRequest.addHeader("If-None-Match", paddedETag); + assertTrue(request.checkNotModified(etag)); assertEquals(304, servletResponse.getStatus()); - assertEquals(paddedEtag, servletResponse.getHeader("ETag")); + assertEquals(paddedETag, servletResponse.getHeader("ETag")); } @Test public void checkModifiedUnpaddedETag() { String currentETag = "Foo"; - String oldEtag = "Bar"; - servletRequest.addHeader("If-None-Match", oldEtag); + String oldETag = "Bar"; + servletRequest.addHeader("If-None-Match", oldETag); assertFalse(request.checkNotModified(currentETag)); - assertEquals(200, servletResponse.getStatus()); assertEquals(String.format("\"%s\"", currentETag), servletResponse.getHeader("ETag")); } @Test public void checkNotModifiedWildcardIsIgnored() { - String eTag = "\"Foo\""; + String etag = "\"Foo\""; servletRequest.addHeader("If-None-Match", "*"); - assertFalse(request.checkNotModified(eTag)); - + assertFalse(request.checkNotModified(etag)); assertEquals(200, servletResponse.getStatus()); - assertEquals(eTag, servletResponse.getHeader("ETag")); + assertEquals(etag, servletResponse.getHeader("ETag")); } @Test public void checkNotModifiedETagAndTimestamp() { - String eTag = "\"Foo\""; - servletRequest.addHeader("If-None-Match", eTag); + String etag = "\"Foo\""; + servletRequest.addHeader("If-None-Match", etag); servletRequest.addHeader("If-Modified-Since", currentDate.getTime()); - assertTrue(request.checkNotModified(eTag, currentDate.getTime())); - + assertTrue(request.checkNotModified(etag, currentDate.getTime())); assertEquals(304, servletResponse.getStatus()); - assertEquals(eTag, servletResponse.getHeader("ETag")); - assertEquals(RFC_1123_DATE_TIME.format(currentDate.toInstant().atZone(GMT)), - servletResponse.getHeader("Last-Modified")); + assertEquals(etag, servletResponse.getHeader("ETag")); + assertEquals(currentDate.getTime() / 1000, servletResponse.getDateHeader("Last-Modified") / 1000); } @Test // SPR-14224 public void checkNotModifiedETagAndModifiedTimestamp() { - String eTag = "\"Foo\""; - servletRequest.addHeader("If-None-Match", eTag); + String etag = "\"Foo\""; + servletRequest.addHeader("If-None-Match", etag); long currentEpoch = currentDate.getTime(); long oneMinuteAgo = currentEpoch - (1000 * 60); servletRequest.addHeader("If-Modified-Since", oneMinuteAgo); - assertTrue(request.checkNotModified(eTag, currentEpoch)); - + assertTrue(request.checkNotModified(etag, currentEpoch)); assertEquals(304, servletResponse.getStatus()); - assertEquals(eTag, servletResponse.getHeader("ETag")); - assertEquals(RFC_1123_DATE_TIME.format(Instant.ofEpochMilli(currentEpoch).atZone(GMT)), - servletResponse.getHeader("Last-Modified")); + assertEquals(etag, servletResponse.getHeader("ETag")); + assertEquals(currentDate.getTime() / 1000, servletResponse.getDateHeader("Last-Modified") / 1000); } @Test - public void checkModifiedETagAndNotModifiedTimestamp() throws Exception { + public void checkModifiedETagAndNotModifiedTimestamp() { String currentETag = "\"Foo\""; - String oldEtag = "\"Bar\""; - servletRequest.addHeader("If-None-Match", oldEtag); + String oldETag = "\"Bar\""; + servletRequest.addHeader("If-None-Match", oldETag); long epochTime = currentDate.getTime(); servletRequest.addHeader("If-Modified-Since", epochTime); assertFalse(request.checkNotModified(currentETag, epochTime)); - assertEquals(200, servletResponse.getStatus()); assertEquals(currentETag, servletResponse.getHeader("ETag")); - assertEquals(RFC_1123_DATE_TIME.format(Instant.ofEpochMilli(epochTime).atZone(GMT)), - servletResponse.getHeader("Last-Modified")); + assertEquals(currentDate.getTime() / 1000, servletResponse.getDateHeader("Last-Modified") / 1000); } @Test public void checkNotModifiedETagWeakStrong() { - String eTag = "\"Foo\""; - String weakEtag = String.format("W/%s", eTag); - servletRequest.addHeader("If-None-Match", eTag); - - assertTrue(request.checkNotModified(weakEtag)); + String etag = "\"Foo\""; + String weakETag = String.format("W/%s", etag); + servletRequest.addHeader("If-None-Match", etag); + assertTrue(request.checkNotModified(weakETag)); assertEquals(304, servletResponse.getStatus()); - assertEquals(weakEtag, servletResponse.getHeader("ETag")); + assertEquals(weakETag, servletResponse.getHeader("ETag")); } @Test public void checkNotModifiedETagStrongWeak() { - String eTag = "\"Foo\""; - servletRequest.addHeader("If-None-Match", String.format("W/%s", eTag)); - - assertTrue(request.checkNotModified(eTag)); + String etag = "\"Foo\""; + servletRequest.addHeader("If-None-Match", String.format("W/%s", etag)); + assertTrue(request.checkNotModified(etag)); assertEquals(304, servletResponse.getStatus()); - assertEquals(eTag, servletResponse.getHeader("ETag")); + assertEquals(etag, servletResponse.getHeader("ETag")); } @Test public void checkNotModifiedMultipleETags() { - String eTag = "\"Bar\""; - String multipleETags = String.format("\"Foo\", %s", eTag); + String etag = "\"Bar\""; + String multipleETags = String.format("\"Foo\", %s", etag); servletRequest.addHeader("If-None-Match", multipleETags); - assertTrue(request.checkNotModified(eTag)); - + assertTrue(request.checkNotModified(etag)); assertEquals(304, servletResponse.getStatus()); - assertEquals(eTag, servletResponse.getHeader("ETag")); + assertEquals(etag, servletResponse.getHeader("ETag")); } @Test - public void checkNotModifiedTimestampWithLengthPart() throws Exception { + public void checkNotModifiedTimestampWithLengthPart() { long epochTime = ZonedDateTime.parse(CURRENT_TIME, RFC_1123_DATE_TIME).toInstant().toEpochMilli(); servletRequest.setMethod("GET"); servletRequest.addHeader("If-Modified-Since", "Wed, 09 Apr 2014 09:57:42 GMT; length=13774"); assertTrue(request.checkNotModified(epochTime)); - assertEquals(304, servletResponse.getStatus()); - assertEquals(RFC_1123_DATE_TIME.format(Instant.ofEpochMilli(epochTime).atZone(GMT)), - servletResponse.getHeader("Last-Modified")); + assertEquals(epochTime / 1000, servletResponse.getDateHeader("Last-Modified") / 1000); } @Test - public void checkModifiedTimestampWithLengthPart() throws Exception { + public void checkModifiedTimestampWithLengthPart() { long epochTime = ZonedDateTime.parse(CURRENT_TIME, RFC_1123_DATE_TIME).toInstant().toEpochMilli(); servletRequest.setMethod("GET"); servletRequest.addHeader("If-Modified-Since", "Wed, 08 Apr 2014 09:57:42 GMT; length=13774"); assertFalse(request.checkNotModified(epochTime)); - assertEquals(200, servletResponse.getStatus()); - assertEquals(RFC_1123_DATE_TIME.format(Instant.ofEpochMilli(epochTime).atZone(GMT)), - servletResponse.getHeader("Last-Modified")); + assertEquals(epochTime / 1000, servletResponse.getDateHeader("Last-Modified") / 1000); } @Test - public void checkNotModifiedTimestampConditionalPut() throws Exception { + public void checkNotModifiedTimestampConditionalPut() { long currentEpoch = currentDate.getTime(); long oneMinuteAgo = currentEpoch - (1000 * 60); servletRequest.setMethod("PUT"); @@ -332,7 +305,7 @@ public class ServletWebRequestHttpMethodsTests { } @Test - public void checkNotModifiedTimestampConditionalPutConflict() throws Exception { + public void checkNotModifiedTimestampConditionalPutConflict() { long currentEpoch = currentDate.getTime(); long oneMinuteAgo = currentEpoch - (1000 * 60); servletRequest.setMethod("PUT");