Skip to content

Commit dedecb9

Browse files
jhoellerjhoeller
committed
HttpEntityMethodProcessor lets entity headers override existing headers (again)
Issue: SPR-15952 (cherry picked from commit 5bdcb89)
1 parent 35af7ff commit dedecb9

File tree

2 files changed

+104
-48
lines changed

2 files changed

+104
-48
lines changed

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

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -172,15 +172,15 @@ public void handleReturnValue(Object returnValue, MethodParameter returnType,
172172

173173
HttpHeaders outputHeaders = outputMessage.getHeaders();
174174
HttpHeaders entityHeaders = responseEntity.getHeaders();
175-
if (outputHeaders.containsKey(HttpHeaders.VARY) && entityHeaders.containsKey(HttpHeaders.VARY)) {
176-
List<String> values = getVaryRequestHeadersToAdd(outputHeaders, entityHeaders);
177-
if (!values.isEmpty()) {
178-
outputHeaders.setVary(values);
179-
}
180-
}
181175
if (!entityHeaders.isEmpty()) {
182176
for (Map.Entry<String, List<String>> entry : entityHeaders.entrySet()) {
183-
if (!outputHeaders.containsKey(entry.getKey())) {
177+
if (HttpHeaders.VARY.equals(entry.getKey()) && outputHeaders.containsKey(HttpHeaders.VARY)) {
178+
List<String> values = getVaryRequestHeadersToAdd(outputHeaders, entityHeaders);
179+
if (!values.isEmpty()) {
180+
outputHeaders.setVary(values);
181+
}
182+
}
183+
else {
184184
outputHeaders.put(entry.getKey(), entry.getValue());
185185
}
186186
}
@@ -207,24 +207,25 @@ public void handleReturnValue(Object returnValue, MethodParameter returnType,
207207
}
208208

209209
private List<String> getVaryRequestHeadersToAdd(HttpHeaders responseHeaders, HttpHeaders entityHeaders) {
210-
if (!responseHeaders.containsKey(HttpHeaders.VARY)) {
211-
return entityHeaders.getVary();
212-
}
213210
List<String> entityHeadersVary = entityHeaders.getVary();
214-
List<String> result = new ArrayList<String>(entityHeadersVary);
215-
for (String header : responseHeaders.get(HttpHeaders.VARY)) {
216-
for (String existing : StringUtils.tokenizeToStringArray(header, ",")) {
217-
if ("*".equals(existing)) {
218-
return Collections.emptyList();
219-
}
220-
for (String value : entityHeadersVary) {
221-
if (value.equalsIgnoreCase(existing)) {
222-
result.remove(value);
211+
List<String> vary = responseHeaders.get(HttpHeaders.VARY);
212+
if (vary != null) {
213+
List<String> result = new ArrayList<String>(entityHeadersVary);
214+
for (String header : vary) {
215+
for (String existing : StringUtils.tokenizeToStringArray(header, ",")) {
216+
if ("*".equals(existing)) {
217+
return Collections.emptyList();
218+
}
219+
for (String value : entityHeadersVary) {
220+
if (value.equalsIgnoreCase(existing)) {
221+
result.remove(value);
222+
}
223223
}
224224
}
225225
}
226+
return result;
226227
}
227-
return result;
228+
return entityHeadersVary;
228229
}
229230

230231
private boolean isResourceNotModified(ServletServerHttpRequest inputMessage, ServletServerHttpResponse outputMessage) {

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

Lines changed: 82 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,18 +16,14 @@
1616

1717
package org.springframework.web.servlet.mvc.method.annotation;
1818

19-
import static org.junit.Assert.*;
20-
import static org.mockito.BDDMockito.*;
21-
import static org.springframework.web.servlet.HandlerMapping.*;
22-
2319
import java.lang.reflect.Method;
2420
import java.net.URI;
2521
import java.nio.charset.Charset;
22+
import java.nio.charset.StandardCharsets;
2623
import java.text.SimpleDateFormat;
27-
import java.util.ArrayList;
24+
import java.util.Arrays;
2825
import java.util.Collections;
2926
import java.util.Date;
30-
import java.util.List;
3127
import java.util.Locale;
3228
import java.util.TimeZone;
3329

@@ -58,6 +54,10 @@
5854
import org.springframework.web.context.request.ServletWebRequest;
5955
import org.springframework.web.method.support.ModelAndViewContainer;
6056

57+
import static org.junit.Assert.*;
58+
import static org.mockito.BDDMockito.*;
59+
import static org.springframework.web.servlet.HandlerMapping.*;
60+
6161
/**
6262
* Test fixture for {@link HttpEntityMethodProcessor} delegating to a mock
6363
* {@link HttpMessageConverter}.
@@ -81,6 +81,8 @@ public class HttpEntityMethodProcessorMockTests {
8181

8282
private HttpMessageConverter<Resource> resourceMessageConverter;
8383

84+
private HttpMessageConverter<Object> resourceRegionMessageConverter;
85+
8486
private MethodParameter paramHttpEntity;
8587

8688
private MethodParameter paramRequestEntity;
@@ -110,22 +112,21 @@ public class HttpEntityMethodProcessorMockTests {
110112
private ServletWebRequest webRequest;
111113

112114

113-
@SuppressWarnings("unchecked")
114115
@Before
115-
public void setUp() throws Exception {
116+
@SuppressWarnings("unchecked")
117+
public void setup() throws Exception {
116118
dateFormat = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss z", Locale.US);
117119
dateFormat.setTimeZone(TimeZone.getTimeZone("GMT"));
118120

119121
stringHttpMessageConverter = mock(HttpMessageConverter.class);
120122
given(stringHttpMessageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.TEXT_PLAIN));
121123
resourceMessageConverter = mock(HttpMessageConverter.class);
122124
given(resourceMessageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.ALL));
123-
List<HttpMessageConverter<?>> converters = new ArrayList<>();
124-
converters.add(stringHttpMessageConverter);
125-
converters.add(resourceMessageConverter);
126-
processor = new HttpEntityMethodProcessor(converters);
127-
reset(stringHttpMessageConverter);
128-
reset(resourceMessageConverter);
125+
resourceRegionMessageConverter = mock(HttpMessageConverter.class);
126+
given(resourceRegionMessageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.ALL));
127+
128+
processor = new HttpEntityMethodProcessor(
129+
Arrays.asList(stringHttpMessageConverter, resourceMessageConverter, resourceRegionMessageConverter));
129130

130131
Method handle1 = getClass().getMethod("handle1", HttpEntity.class, ResponseEntity.class,
131132
Integer.TYPE, RequestEntity.class);
@@ -172,7 +173,7 @@ public void shouldResolveHttpEntityArgument() throws Exception {
172173

173174
MediaType contentType = MediaType.TEXT_PLAIN;
174175
servletRequest.addHeader("Content-Type", contentType.toString());
175-
servletRequest.setContent(body.getBytes(Charset.forName("UTF-8")));
176+
servletRequest.setContent(body.getBytes(StandardCharsets.UTF_8));
176177

177178
given(stringHttpMessageConverter.canRead(String.class, contentType)).willReturn(true);
178179
given(stringHttpMessageConverter.read(eq(String.class), isA(HttpInputMessage.class))).willReturn(body);
@@ -259,8 +260,8 @@ public void shouldHandleReturnValueWithProducibleMediaType() throws Exception {
259260
verify(stringHttpMessageConverter).write(eq(body), eq(MediaType.TEXT_HTML), isA(HttpOutputMessage.class));
260261
}
261262

262-
@SuppressWarnings("unchecked")
263263
@Test
264+
@SuppressWarnings("unchecked")
264265
public void shouldHandleReturnValueWithResponseBodyAdvice() throws Exception {
265266
servletRequest.addHeader("Accept", "text/*");
266267
servletRequest.setAttribute(PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE, Collections.singleton(MediaType.TEXT_HTML));
@@ -310,7 +311,7 @@ public void shouldFailHandlingWhenConverterCannotWrite() throws Exception {
310311
processor.handleReturnValue(returnValue, returnTypeResponseEntityProduces, mavContainer, webRequest);
311312
}
312313

313-
@Test // SPR-9142
314+
@Test // SPR-9142
314315
public void shouldFailHandlingWhenAcceptHeaderIllegal() throws Exception {
315316
ResponseEntity<String> returnValue = new ResponseEntity<>("Body", HttpStatus.ACCEPTED);
316317
servletRequest.addHeader("Accept", "01");
@@ -371,7 +372,7 @@ public void handleEtagWithHttp304() throws Exception {
371372
assertConditionalResponse(HttpStatus.NOT_MODIFIED, null, etagValue, -1);
372373
}
373374

374-
@Test // SPR-14559
375+
@Test // SPR-14559
375376
public void shouldHandleInvalidIfNoneMatchWithHttp200() throws Exception {
376377
String etagValue = "\"deadb33f8badf00d\"";
377378
servletRequest.addHeader(HttpHeaders.IF_NONE_MATCH, "unquoted");
@@ -429,7 +430,7 @@ public void shouldHandleChangedETagAndLastModified() throws Exception {
429430
assertConditionalResponse(HttpStatus.OK, null, changedEtagValue, oneMinuteAgo);
430431
}
431432

432-
@Test // SPR-13496
433+
@Test // SPR-13496
433434
public void shouldHandleConditionalRequestIfNoneMatchWildcard() throws Exception {
434435
String wildcardValue = "*";
435436
String etagValue = "\"some-etag\"";
@@ -443,7 +444,7 @@ public void shouldHandleConditionalRequestIfNoneMatchWildcard() throws Exception
443444
assertConditionalResponse(HttpStatus.OK, "body", etagValue, -1);
444445
}
445446

446-
@Test // SPR-13626
447+
@Test // SPR-13626
447448
public void shouldHandleGetIfNoneMatchWildcard() throws Exception {
448449
String wildcardValue = "*";
449450
String etagValue = "\"some-etag\"";
@@ -456,7 +457,7 @@ public void shouldHandleGetIfNoneMatchWildcard() throws Exception {
456457
assertConditionalResponse(HttpStatus.OK, "body", etagValue, -1);
457458
}
458459

459-
@Test // SPR-13626
460+
@Test // SPR-13626
460461
public void shouldHandleIfNoneMatchIfMatch() throws Exception {
461462
String etagValue = "\"some-etag\"";
462463
servletRequest.addHeader(HttpHeaders.IF_NONE_MATCH, etagValue);
@@ -469,7 +470,7 @@ public void shouldHandleIfNoneMatchIfMatch() throws Exception {
469470
assertConditionalResponse(HttpStatus.NOT_MODIFIED, null, etagValue, -1);
470471
}
471472

472-
@Test // SPR-13626
473+
@Test // SPR-13626
473474
public void shouldHandleIfNoneMatchIfUnmodifiedSince() throws Exception {
474475
String etagValue = "\"some-etag\"";
475476
servletRequest.addHeader(HttpHeaders.IF_NONE_MATCH, etagValue);
@@ -485,7 +486,7 @@ public void shouldHandleIfNoneMatchIfUnmodifiedSince() throws Exception {
485486
@Test
486487
public void shouldHandleResource() throws Exception {
487488
ResponseEntity<Resource> returnValue = ResponseEntity
488-
.ok(new ByteArrayResource("Content".getBytes(Charset.forName("UTF-8"))));
489+
.ok(new ByteArrayResource("Content".getBytes(StandardCharsets.UTF_8)));
489490

490491
given(resourceMessageConverter.canWrite(ByteArrayResource.class, null)).willReturn(true);
491492
given(resourceMessageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.ALL));
@@ -498,7 +499,7 @@ public void shouldHandleResource() throws Exception {
498499
assertEquals(200, servletResponse.getStatus());
499500
}
500501

501-
@Test //SPR-14767
502+
@Test //SPR-14767
502503
public void shouldHandleValidatorHeadersInPutResponses() throws Exception {
503504
servletRequest.setMethod("PUT");
504505
String etagValue = "\"some-etag\"";
@@ -510,6 +511,60 @@ public void shouldHandleValidatorHeadersInPutResponses() throws Exception {
510511
assertConditionalResponse(HttpStatus.OK, "body", etagValue, -1);
511512
}
512513

514+
@Test
515+
public void varyHeader() throws Exception {
516+
String[] entityValues = {"Accept-Language", "User-Agent"};
517+
String[] existingValues = {};
518+
String[] expected = {"Accept-Language, User-Agent"};
519+
testVaryHeader(entityValues, existingValues, expected);
520+
}
521+
522+
@Test
523+
public void varyHeaderWithExistingWildcard() throws Exception {
524+
String[] entityValues = {"Accept-Language"};
525+
String[] existingValues = {"*"};
526+
String[] expected = {"*"};
527+
testVaryHeader(entityValues, existingValues, expected);
528+
}
529+
530+
@Test
531+
public void varyHeaderWithExistingCommaValues() throws Exception {
532+
String[] entityValues = {"Accept-Language", "User-Agent"};
533+
String[] existingValues = {"Accept-Encoding", "Accept-Language"};
534+
String[] expected = {"Accept-Encoding", "Accept-Language", "User-Agent"};
535+
testVaryHeader(entityValues, existingValues, expected);
536+
}
537+
538+
@Test
539+
public void varyHeaderWithExistingCommaSeparatedValues() throws Exception {
540+
String[] entityValues = {"Accept-Language", "User-Agent"};
541+
String[] existingValues = {"Accept-Encoding, Accept-Language"};
542+
String[] expected = {"Accept-Encoding, Accept-Language", "User-Agent"};
543+
testVaryHeader(entityValues, existingValues, expected);
544+
}
545+
546+
@Test
547+
public void handleReturnValueVaryHeader() throws Exception {
548+
String[] entityValues = {"Accept-Language", "User-Agent"};
549+
String[] existingValues = {"Accept-Encoding, Accept-Language"};
550+
String[] expected = {"Accept-Encoding, Accept-Language", "User-Agent"};
551+
testVaryHeader(entityValues, existingValues, expected);
552+
}
553+
554+
555+
private void testVaryHeader(String[] entityValues, String[] existingValues, String[] expected) throws Exception {
556+
ResponseEntity<String> returnValue = ResponseEntity.ok().varyBy(entityValues).body("Foo");
557+
for (String value : existingValues) {
558+
servletResponse.addHeader("Vary", value);
559+
}
560+
initStringMessageConversion(MediaType.TEXT_PLAIN);
561+
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
562+
563+
assertTrue(mavContainer.isRequestHandled());
564+
assertEquals(Arrays.asList(expected), servletResponse.getHeaders("Vary"));
565+
verify(stringHttpMessageConverter).write(eq("Foo"), eq(MediaType.TEXT_PLAIN), isA(HttpOutputMessage.class));
566+
}
567+
513568
private void initStringMessageConversion(MediaType accepted) {
514569
given(stringHttpMessageConverter.canWrite(String.class, null)).willReturn(true);
515570
given(stringHttpMessageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.TEXT_PLAIN));
@@ -521,8 +576,7 @@ private void assertResponseBody(String body) throws Exception {
521576
verify(stringHttpMessageConverter).write(eq(body), eq(MediaType.TEXT_PLAIN), outputMessage.capture());
522577
}
523578

524-
private void assertConditionalResponse(HttpStatus status, String body,
525-
String etag, long lastModified) throws Exception {
579+
private void assertConditionalResponse(HttpStatus status, String body, String etag, long lastModified) throws Exception {
526580
assertEquals(status.value(), servletResponse.getStatus());
527581
assertTrue(mavContainer.isRequestHandled());
528582
if (body != null) {
@@ -541,6 +595,7 @@ private void assertConditionalResponse(HttpStatus status, String body,
541595
}
542596
}
543597

598+
544599
@SuppressWarnings("unused")
545600
public ResponseEntity<String> handle1(HttpEntity<String> httpEntity, ResponseEntity<String> entity,
546601
int i, RequestEntity<String> requestEntity) {

0 commit comments

Comments
 (0)