Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Consistent handling of null header values in HttpHeaders

Browse Source 
Issue: SPR-17588
This commit is contained in:
Juergen Hoeller
2018-12-12 11:16:45 +01:00
parent 4b4503085a
commit 5bbbc82e19
6 changed files with 69 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

84
spring-web/src/main/java/org/springframework/http/HttpHeaders.java
View File

@@ -75,11 +75,7 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
private static final long serialVersionUID = -8578554704772377436L;
/**
* The empty {@code HttpHeaders} instance (immutable).
*/
public static final HttpHeaders EMPTY =
new ReadOnlyHttpHeaders(new HttpHeaders(new LinkedMultiValueMap<>(0)));
/**
* The HTTP {@code Accept} header field name.
* @see <a href="http://tools.ietf.org/html/rfc7231#section-5.3.2">Section 5.3.2 of RFC 7231</a>
@@ -381,6 +377,12 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
*/
public static final String WWW_AUTHENTICATE = "WWW-Authenticate";
/**
* The empty {@code HttpHeaders} instance (immutable).
*/
public static final HttpHeaders EMPTY = new ReadOnlyHttpHeaders(new HttpHeaders(new LinkedMultiValueMap<>(0)));
/**
* Pattern matching ETag multiple field values in headers such as "If-Match", "If-None-Match".
* @see <a href="https://tools.ietf.org/html/rfc7232#section-2.3">Section 2.3 of RFC 7232</a>
@@ -409,15 +411,15 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
* Construct a new, empty instance of the {@code HttpHeaders} object.
*/
public HttpHeaders() {
this(CollectionUtils.toMultiValueMap(
new LinkedCaseInsensitiveMap<>(8, Locale.ENGLISH)));
this(CollectionUtils.toMultiValueMap(new LinkedCaseInsensitiveMap<>(8, Locale.ENGLISH)));
}
/**
* Construct a new {@code HttpHeaders} instance backed by an existing map.
* @since 5.1
*/
public HttpHeaders(MultiValueMap<String, String> headers) {
Assert.notNull(headers, "headers must not be null");
Assert.notNull(headers, "MultiValueMap must not be null");
this.headers = headers;
}
@@ -445,7 +447,7 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
* @since 5.0
*/
public void setAcceptLanguage(List<Locale.LanguageRange> languages) {
Assert.notNull(languages, "'languages' must not be null");
Assert.notNull(languages, "LanguageRange List must not be null");
DecimalFormat decimal = new DecimalFormat("0.0", DECIMAL_FORMAT_SYMBOLS);
List<String> values = languages.stream()
.map(range ->
@@ -555,7 +557,7 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
* Set the (new) value of the {@code Access-Control-Allow-Origin} response header.
*/
public void setAccessControlAllowOrigin(@Nullable String allowedOrigin) {
set(ACCESS_CONTROL_ALLOW_ORIGIN, allowedOrigin);
setOrRemove(ACCESS_CONTROL_ALLOW_ORIGIN, allowedOrigin);
}
/**
@@ -614,7 +616,7 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
* Set the (new) value of the {@code Access-Control-Request-Method} request header.
*/
public void setAccessControlRequestMethod(@Nullable HttpMethod requestMethod) {
set(ACCESS_CONTROL_REQUEST_METHOD, (requestMethod != null ? requestMethod.name() : null));
setOrRemove(ACCESS_CONTROL_REQUEST_METHOD, (requestMethod != null ? requestMethod.name() : null));
}
/**
@@ -766,14 +768,14 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
* @since 5.0.5
*/
public void setCacheControl(CacheControl cacheControl) {
set(CACHE_CONTROL, cacheControl.getHeaderValue());
setOrRemove(CACHE_CONTROL, cacheControl.getHeaderValue());
}
/**
* Set the (new) value of the {@code Cache-Control} header.
*/
public void setCacheControl(@Nullable String cacheControl) {
set(CACHE_CONTROL, cacheControl);
setOrRemove(CACHE_CONTROL, cacheControl);
}
/**
@@ -817,7 +819,7 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
* @see #getContentDisposition()
*/
public void setContentDispositionFormData(String name, @Nullable String filename) {
Assert.notNull(name, "'name' must not be null");
Assert.notNull(name, "Name must not be null");
ContentDisposition.Builder disposition = ContentDisposition.builder("form-data").name(name);
if (filename != null) {
disposition.filename(filename);
@@ -860,7 +862,7 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
* @since 5.0
*/
public void setContentLanguage(@Nullable Locale locale) {
set(CONTENT_LANGUAGE, (locale != null ? locale.toLanguageTag() : null));
setOrRemove(CONTENT_LANGUAGE, (locale != null ? locale.toLanguageTag() : null));
}
/**
@@ -904,12 +906,12 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
*/
public void setContentType(@Nullable MediaType mediaType) {
if (mediaType != null) {
Assert.isTrue(!mediaType.isWildcardType(), "'Content-Type' cannot contain wildcard type '*'");
Assert.isTrue(!mediaType.isWildcardSubtype(), "'Content-Type' cannot contain wildcard subtype '*'");
Assert.isTrue(!mediaType.isWildcardType(), "Content-Type cannot contain wildcard type '*'");
Assert.isTrue(!mediaType.isWildcardSubtype(), "Content-Type cannot contain wildcard subtype '*'");
set(CONTENT_TYPE, mediaType.toString());
}
else {
set(CONTENT_TYPE, null);
remove(CONTENT_TYPE);
}
}
@@ -953,8 +955,11 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
Assert.isTrue(etag.startsWith("\"") || etag.startsWith("W/"),
"Invalid ETag: does not start with W/ or \"");
Assert.isTrue(etag.endsWith("\""), "Invalid ETag: does not end with \"");
set(ETAG, etag);
}
else {
remove(ETAG);
}
set(ETAG, etag);
}
/**
@@ -1012,7 +1017,7 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
set(HOST, value);
}
else {
set(HOST, null);
remove(HOST, null);
}
}
@@ -1160,7 +1165,7 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
* @since 5.1.4
*/
public void setLastModified(ZonedDateTime lastModified) {
setZonedDateTime(LAST_MODIFIED, lastModified.withZoneSameInstant(ZoneId.of("GMT")));
setZonedDateTime(LAST_MODIFIED, lastModified.withZoneSameInstant(GMT));
}
/**
@@ -1179,7 +1184,7 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
* as specified by the {@code Location} header.
*/
public void setLocation(@Nullable URI location) {
set(LOCATION, (location != null ? location.toASCIIString() : null));
setOrRemove(LOCATION, (location != null ? location.toASCIIString() : null));
}
/**
@@ -1197,7 +1202,7 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
* Set the (new) value of the {@code Origin} header.
*/
public void setOrigin(@Nullable String origin) {
set(ORIGIN, origin);
setOrRemove(ORIGIN, origin);
}
/**
@@ -1212,7 +1217,7 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
* Set the (new) value of the {@code Pragma} header.
*/
public void setPragma(@Nullable String pragma) {
set(PRAGMA, pragma);
setOrRemove(PRAGMA, pragma);
}
/**
@@ -1244,7 +1249,7 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
* Set the (new) value of the {@code Upgrade} header.
*/
public void setUpgrade(@Nullable String upgrade) {
set(UPGRADE, upgrade);
setOrRemove(UPGRADE, upgrade);
}
/**
@@ -1406,10 +1411,7 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
List<String> result = new ArrayList<>();
for (String value : values) {
if (value != null) {
String[] tokens = StringUtils.tokenizeToStringArray(value, ",");
for (String token : tokens) {
result.add(token);
}
Collections.addAll(result, StringUtils.tokenizeToStringArray(value, ","));
}
}
return result;
@@ -1468,16 +1470,32 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
*/
protected String toCommaDelimitedString(List<String> headerValues) {
StringBuilder builder = new StringBuilder();
for (Iterator<String> it = headerValues.iterator(); it.hasNext(); ) {
for (Iterator<String> it = headerValues.iterator(); it.hasNext();) {
String val = it.next();
builder.append(val);
if (it.hasNext()) {
builder.append(", ");
if (val != null) {
builder.append(val);
if (it.hasNext()) {
builder.append(", ");
}
}
}
return builder.toString();
}
/**
* Set the given header value, or remove the header if {@code null}.
* @param headerName the header name
* @param headerValue the header value, or {@code null} for none
*/
private void setOrRemove(String headerName, @Nullable String headerValue) {
if (headerValue != null) {
set(headerName, headerValue);
}
else {
remove(headerName);
}
}
// MultiValueMap implementation

5
spring-web/src/main/java/org/springframework/http/ResponseEntity.java
View File

@@ -537,10 +537,7 @@ public class ResponseEntity<T> extends HttpEntity<T> {
@Override
public BodyBuilder cacheControl(CacheControl cacheControl) {
String ccValue = cacheControl.getHeaderValue();
if (ccValue != null) {
this.headers.setCacheControl(cacheControl.getHeaderValue());
}
this.headers.setCacheControl(cacheControl);
return this;
}

15
spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java
View File

@@ -38,8 +38,8 @@ import java.util.TimeZone;
import org.hamcrest.Matchers;
import org.junit.Test;
import static java.time.format.DateTimeFormatter.RFC_1123_DATE_TIME;
import static org.hamcrest.Matchers.is;
import static java.time.format.DateTimeFormatter.*;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;
/**
@@ -355,11 +355,18 @@ public class HttpHeadersTests {
assertEquals("Invalid Cache-Control header", "no-cache", headers.getFirst("cache-control"));
}
@Test
public void cacheControlEmpty() {
headers.setCacheControl(CacheControl.empty());
assertNull("Invalid Cache-Control header", headers.getCacheControl());
assertNull("Invalid Cache-Control header", headers.getFirst("cache-control"));
}
@Test
public void cacheControlAllValues() {
headers.add(HttpHeaders.CACHE_CONTROL, "max-age=1000, public");
headers.add(HttpHeaders.CACHE_CONTROL, "s-maxage=1000");
assertThat(headers.getCacheControl(), is("max-age=1000, public, s-maxage=1000"));
assertEquals("max-age=1000, public, s-maxage=1000", headers.getCacheControl());
}
@Test
@@ -375,7 +382,7 @@ public class HttpHeadersTests {
@Test // SPR-11917
public void getAllowEmptySet() {
headers.setAllow(Collections.<HttpMethod> emptySet());
headers.setAllow(Collections.emptySet());
assertThat(headers.getAllow(), Matchers.emptyCollectionOf(HttpMethod.class));
}

4
spring-web/src/test/java/org/springframework/http/client/SimpleClientHttpRequestFactoryTests.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2015 the original author or authors.
* Copyright 2002-2018 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.
@@ -29,7 +29,7 @@ import static org.mockito.Mockito.*;
*/
public class SimpleClientHttpRequestFactoryTests {
@Test // SPR-13225
@Test // SPR-13225
public void headerWithNullValue() {
HttpURLConnection urlConnection = mock(HttpURLConnection.class);
HttpHeaders headers = new HttpHeaders();

5
spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerResponseBuilder.java
View File

@@ -177,10 +177,7 @@ class DefaultServerResponseBuilder implements ServerResponse.BodyBuilder {
@Override
public ServerResponse.BodyBuilder cacheControl(CacheControl cacheControl) {
String ccValue = cacheControl.getHeaderValue();
if (ccValue != null) {
this.headers.setCacheControl(cacheControl.getHeaderValue());
}
this.headers.setCacheControl(cacheControl);
return this;
}

8
spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java
View File

@@ -345,11 +345,9 @@ public class ResourceWebHandler implements WebHandler, InitializingBean {
}
// Apply cache settings, if any
if (getCacheControl() != null) {
String value = getCacheControl().getHeaderValue();
if (value != null) {
exchange.getResponse().getHeaders().setCacheControl(value);
}
CacheControl cacheControl = getCacheControl();
if (cacheControl != null) {
exchange.getResponse().getHeaders().setCacheControl(cacheControl);
}
// Check the media type for the resource