diff --git a/spring-web/src/main/java/org/springframework/http/HttpHeaders.java b/spring-web/src/main/java/org/springframework/http/HttpHeaders.java index ad81e95ff8c..3eda7f19023 100644 --- a/spring-web/src/main/java/org/springframework/http/HttpHeaders.java +++ b/spring-web/src/main/java/org/springframework/http/HttpHeaders.java @@ -75,11 +75,7 @@ public class HttpHeaders implements MultiValueMap, 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 Section 5.3.2 of RFC 7231 @@ -381,6 +377,12 @@ public class HttpHeaders implements MultiValueMap, 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 Section 2.3 of RFC 7232 @@ -409,15 +411,15 @@ public class HttpHeaders implements MultiValueMap, 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 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, Serializable * @since 5.0 */ public void setAcceptLanguage(List 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 values = languages.stream() .map(range -> @@ -555,7 +557,7 @@ public class HttpHeaders implements MultiValueMap, 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, 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, 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, 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, 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, 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, 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, Serializable set(HOST, value); } else { - set(HOST, null); + remove(HOST, null); } } @@ -1160,7 +1165,7 @@ public class HttpHeaders implements MultiValueMap, 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, 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, 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, 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, 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, Serializable List 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, Serializable */ protected String toCommaDelimitedString(List headerValues) { StringBuilder builder = new StringBuilder(); - for (Iterator it = headerValues.iterator(); it.hasNext(); ) { + for (Iterator 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 diff --git a/spring-web/src/main/java/org/springframework/http/ResponseEntity.java b/spring-web/src/main/java/org/springframework/http/ResponseEntity.java index 3d9e0039d97..c35c3e9e7e7 100644 --- a/spring-web/src/main/java/org/springframework/http/ResponseEntity.java +++ b/spring-web/src/main/java/org/springframework/http/ResponseEntity.java @@ -537,10 +537,7 @@ public class ResponseEntity extends HttpEntity { @Override public BodyBuilder cacheControl(CacheControl cacheControl) { - String ccValue = cacheControl.getHeaderValue(); - if (ccValue != null) { - this.headers.setCacheControl(cacheControl.getHeaderValue()); - } + this.headers.setCacheControl(cacheControl); return this; } diff --git a/spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java b/spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java index 3dd7f912aaf..fecd698db69 100644 --- a/spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java +++ b/spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java @@ -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. emptySet()); + headers.setAllow(Collections.emptySet()); assertThat(headers.getAllow(), Matchers.emptyCollectionOf(HttpMethod.class)); } diff --git a/spring-web/src/test/java/org/springframework/http/client/SimpleClientHttpRequestFactoryTests.java b/spring-web/src/test/java/org/springframework/http/client/SimpleClientHttpRequestFactoryTests.java index 1157376fc18..03f20f93171 100644 --- a/spring-web/src/test/java/org/springframework/http/client/SimpleClientHttpRequestFactoryTests.java +++ b/spring-web/src/test/java/org/springframework/http/client/SimpleClientHttpRequestFactoryTests.java @@ -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(); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerResponseBuilder.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerResponseBuilder.java index 621e1840f19..b41dbac8120 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerResponseBuilder.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerResponseBuilder.java @@ -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; } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java index fdca1fe2286..0f89ea84f1e 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java @@ -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