diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java index 15015780499..a2d858f31f7 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2017 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. @@ -172,15 +172,15 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro HttpHeaders outputHeaders = outputMessage.getHeaders(); HttpHeaders entityHeaders = responseEntity.getHeaders(); - if (outputHeaders.containsKey(HttpHeaders.VARY) && entityHeaders.containsKey(HttpHeaders.VARY)) { - List values = getVaryRequestHeadersToAdd(outputHeaders, entityHeaders); - if (!values.isEmpty()) { - outputHeaders.setVary(values); - } - } if (!entityHeaders.isEmpty()) { for (Map.Entry> entry : entityHeaders.entrySet()) { - if (!outputHeaders.containsKey(entry.getKey())) { + if (HttpHeaders.VARY.equals(entry.getKey()) && outputHeaders.containsKey(HttpHeaders.VARY)) { + List values = getVaryRequestHeadersToAdd(outputHeaders, entityHeaders); + if (!values.isEmpty()) { + outputHeaders.setVary(values); + } + } + else { outputHeaders.put(entry.getKey(), entry.getValue()); } } @@ -207,24 +207,25 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro } private List getVaryRequestHeadersToAdd(HttpHeaders responseHeaders, HttpHeaders entityHeaders) { - if (!responseHeaders.containsKey(HttpHeaders.VARY)) { - return entityHeaders.getVary(); - } List entityHeadersVary = entityHeaders.getVary(); - List result = new ArrayList(entityHeadersVary); - for (String header : responseHeaders.get(HttpHeaders.VARY)) { - for (String existing : StringUtils.tokenizeToStringArray(header, ",")) { - if ("*".equals(existing)) { - return Collections.emptyList(); - } - for (String value : entityHeadersVary) { - if (value.equalsIgnoreCase(existing)) { - result.remove(value); + List vary = responseHeaders.get(HttpHeaders.VARY); + if (vary != null) { + List result = new ArrayList(entityHeadersVary); + for (String header : vary) { + for (String existing : StringUtils.tokenizeToStringArray(header, ",")) { + if ("*".equals(existing)) { + return Collections.emptyList(); + } + for (String value : entityHeadersVary) { + if (value.equalsIgnoreCase(existing)) { + result.remove(value); + } } } } + return result; } - return result; + return entityHeadersVary; } private boolean isResourceNotModified(ServletServerHttpRequest inputMessage, ServletServerHttpResponse outputMessage) { diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java index a0ed4881181..a83770776c9 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2017 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. @@ -16,18 +16,14 @@ package org.springframework.web.servlet.mvc.method.annotation; -import static org.junit.Assert.*; -import static org.mockito.BDDMockito.*; -import static org.springframework.web.servlet.HandlerMapping.*; - import java.lang.reflect.Method; import java.net.URI; import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.text.SimpleDateFormat; -import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Date; -import java.util.List; import java.util.Locale; import java.util.TimeZone; @@ -58,6 +54,10 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.context.request.ServletWebRequest; import org.springframework.web.method.support.ModelAndViewContainer; +import static org.junit.Assert.*; +import static org.mockito.BDDMockito.*; +import static org.springframework.web.servlet.HandlerMapping.*; + /** * Test fixture for {@link HttpEntityMethodProcessor} delegating to a mock * {@link HttpMessageConverter}. @@ -81,6 +81,8 @@ public class HttpEntityMethodProcessorMockTests { private HttpMessageConverter resourceMessageConverter; + private HttpMessageConverter resourceRegionMessageConverter; + private MethodParameter paramHttpEntity; private MethodParameter paramRequestEntity; @@ -110,9 +112,9 @@ public class HttpEntityMethodProcessorMockTests { private ServletWebRequest webRequest; - @SuppressWarnings("unchecked") @Before - public void setUp() throws Exception { + @SuppressWarnings("unchecked") + public void setup() throws Exception { dateFormat = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss z", Locale.US); dateFormat.setTimeZone(TimeZone.getTimeZone("GMT")); @@ -120,12 +122,11 @@ public class HttpEntityMethodProcessorMockTests { given(stringHttpMessageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.TEXT_PLAIN)); resourceMessageConverter = mock(HttpMessageConverter.class); given(resourceMessageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.ALL)); - List> converters = new ArrayList<>(); - converters.add(stringHttpMessageConverter); - converters.add(resourceMessageConverter); - processor = new HttpEntityMethodProcessor(converters); - reset(stringHttpMessageConverter); - reset(resourceMessageConverter); + resourceRegionMessageConverter = mock(HttpMessageConverter.class); + given(resourceRegionMessageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.ALL)); + + processor = new HttpEntityMethodProcessor( + Arrays.asList(stringHttpMessageConverter, resourceMessageConverter, resourceRegionMessageConverter)); Method handle1 = getClass().getMethod("handle1", HttpEntity.class, ResponseEntity.class, Integer.TYPE, RequestEntity.class); @@ -172,7 +173,7 @@ public class HttpEntityMethodProcessorMockTests { MediaType contentType = MediaType.TEXT_PLAIN; servletRequest.addHeader("Content-Type", contentType.toString()); - servletRequest.setContent(body.getBytes(Charset.forName("UTF-8"))); + servletRequest.setContent(body.getBytes(StandardCharsets.UTF_8)); given(stringHttpMessageConverter.canRead(String.class, contentType)).willReturn(true); given(stringHttpMessageConverter.read(eq(String.class), isA(HttpInputMessage.class))).willReturn(body); @@ -259,8 +260,8 @@ public class HttpEntityMethodProcessorMockTests { verify(stringHttpMessageConverter).write(eq(body), eq(MediaType.TEXT_HTML), isA(HttpOutputMessage.class)); } - @SuppressWarnings("unchecked") @Test + @SuppressWarnings("unchecked") public void shouldHandleReturnValueWithResponseBodyAdvice() throws Exception { servletRequest.addHeader("Accept", "text/*"); servletRequest.setAttribute(PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE, Collections.singleton(MediaType.TEXT_HTML)); @@ -310,7 +311,7 @@ public class HttpEntityMethodProcessorMockTests { processor.handleReturnValue(returnValue, returnTypeResponseEntityProduces, mavContainer, webRequest); } - @Test // SPR-9142 + @Test // SPR-9142 public void shouldFailHandlingWhenAcceptHeaderIllegal() throws Exception { ResponseEntity returnValue = new ResponseEntity<>("Body", HttpStatus.ACCEPTED); servletRequest.addHeader("Accept", "01"); @@ -371,7 +372,7 @@ public class HttpEntityMethodProcessorMockTests { assertConditionalResponse(HttpStatus.NOT_MODIFIED, null, etagValue, -1); } - @Test // SPR-14559 + @Test // SPR-14559 public void shouldHandleInvalidIfNoneMatchWithHttp200() throws Exception { String etagValue = "\"deadb33f8badf00d\""; servletRequest.addHeader(HttpHeaders.IF_NONE_MATCH, "unquoted"); @@ -429,7 +430,7 @@ public class HttpEntityMethodProcessorMockTests { assertConditionalResponse(HttpStatus.OK, null, changedEtagValue, oneMinuteAgo); } - @Test // SPR-13496 + @Test // SPR-13496 public void shouldHandleConditionalRequestIfNoneMatchWildcard() throws Exception { String wildcardValue = "*"; String etagValue = "\"some-etag\""; @@ -443,7 +444,7 @@ public class HttpEntityMethodProcessorMockTests { assertConditionalResponse(HttpStatus.OK, "body", etagValue, -1); } - @Test // SPR-13626 + @Test // SPR-13626 public void shouldHandleGetIfNoneMatchWildcard() throws Exception { String wildcardValue = "*"; String etagValue = "\"some-etag\""; @@ -456,7 +457,7 @@ public class HttpEntityMethodProcessorMockTests { assertConditionalResponse(HttpStatus.OK, "body", etagValue, -1); } - @Test // SPR-13626 + @Test // SPR-13626 public void shouldHandleIfNoneMatchIfMatch() throws Exception { String etagValue = "\"some-etag\""; servletRequest.addHeader(HttpHeaders.IF_NONE_MATCH, etagValue); @@ -469,7 +470,7 @@ public class HttpEntityMethodProcessorMockTests { assertConditionalResponse(HttpStatus.NOT_MODIFIED, null, etagValue, -1); } - @Test // SPR-13626 + @Test // SPR-13626 public void shouldHandleIfNoneMatchIfUnmodifiedSince() throws Exception { String etagValue = "\"some-etag\""; servletRequest.addHeader(HttpHeaders.IF_NONE_MATCH, etagValue); @@ -485,7 +486,7 @@ public class HttpEntityMethodProcessorMockTests { @Test public void shouldHandleResource() throws Exception { ResponseEntity returnValue = ResponseEntity - .ok(new ByteArrayResource("Content".getBytes(Charset.forName("UTF-8")))); + .ok(new ByteArrayResource("Content".getBytes(StandardCharsets.UTF_8))); given(resourceMessageConverter.canWrite(ByteArrayResource.class, null)).willReturn(true); given(resourceMessageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.ALL)); @@ -498,7 +499,7 @@ public class HttpEntityMethodProcessorMockTests { assertEquals(200, servletResponse.getStatus()); } - @Test //SPR-14767 + @Test //SPR-14767 public void shouldHandleValidatorHeadersInPutResponses() throws Exception { servletRequest.setMethod("PUT"); String etagValue = "\"some-etag\""; @@ -510,6 +511,60 @@ public class HttpEntityMethodProcessorMockTests { assertConditionalResponse(HttpStatus.OK, "body", etagValue, -1); } + @Test + public void varyHeader() throws Exception { + String[] entityValues = {"Accept-Language", "User-Agent"}; + String[] existingValues = {}; + String[] expected = {"Accept-Language, User-Agent"}; + testVaryHeader(entityValues, existingValues, expected); + } + + @Test + public void varyHeaderWithExistingWildcard() throws Exception { + String[] entityValues = {"Accept-Language"}; + String[] existingValues = {"*"}; + String[] expected = {"*"}; + testVaryHeader(entityValues, existingValues, expected); + } + + @Test + public void varyHeaderWithExistingCommaValues() throws Exception { + String[] entityValues = {"Accept-Language", "User-Agent"}; + String[] existingValues = {"Accept-Encoding", "Accept-Language"}; + String[] expected = {"Accept-Encoding", "Accept-Language", "User-Agent"}; + testVaryHeader(entityValues, existingValues, expected); + } + + @Test + public void varyHeaderWithExistingCommaSeparatedValues() throws Exception { + String[] entityValues = {"Accept-Language", "User-Agent"}; + String[] existingValues = {"Accept-Encoding, Accept-Language"}; + String[] expected = {"Accept-Encoding, Accept-Language", "User-Agent"}; + testVaryHeader(entityValues, existingValues, expected); + } + + @Test + public void handleReturnValueVaryHeader() throws Exception { + String[] entityValues = {"Accept-Language", "User-Agent"}; + String[] existingValues = {"Accept-Encoding, Accept-Language"}; + String[] expected = {"Accept-Encoding, Accept-Language", "User-Agent"}; + testVaryHeader(entityValues, existingValues, expected); + } + + + private void testVaryHeader(String[] entityValues, String[] existingValues, String[] expected) throws Exception { + ResponseEntity returnValue = ResponseEntity.ok().varyBy(entityValues).body("Foo"); + for (String value : existingValues) { + servletResponse.addHeader("Vary", value); + } + initStringMessageConversion(MediaType.TEXT_PLAIN); + processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest); + + assertTrue(mavContainer.isRequestHandled()); + assertEquals(Arrays.asList(expected), servletResponse.getHeaders("Vary")); + verify(stringHttpMessageConverter).write(eq("Foo"), eq(MediaType.TEXT_PLAIN), isA(HttpOutputMessage.class)); + } + private void initStringMessageConversion(MediaType accepted) { given(stringHttpMessageConverter.canWrite(String.class, null)).willReturn(true); given(stringHttpMessageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.TEXT_PLAIN)); @@ -521,8 +576,7 @@ public class HttpEntityMethodProcessorMockTests { verify(stringHttpMessageConverter).write(eq(body), eq(MediaType.TEXT_PLAIN), outputMessage.capture()); } - private void assertConditionalResponse(HttpStatus status, String body, - String etag, long lastModified) throws Exception { + private void assertConditionalResponse(HttpStatus status, String body, String etag, long lastModified) throws Exception { assertEquals(status.value(), servletResponse.getStatus()); assertTrue(mavContainer.isRequestHandled()); if (body != null) { @@ -541,6 +595,7 @@ public class HttpEntityMethodProcessorMockTests { } } + @SuppressWarnings("unused") public ResponseEntity handle1(HttpEntity httpEntity, ResponseEntity entity, int i, RequestEntity requestEntity) {