diff --git a/spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequest.java index 7001df92a7..3743bb294a 100644 --- a/spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequest.java @@ -47,13 +47,16 @@ import org.springframework.util.StringUtils; */ class JdkClientHttpRequest extends AbstractStreamingClientHttpRequest { - /* - * The JDK HttpRequest doesn't allow all headers to be set. The named headers are taken from the default - * implementation for HttpRequest. - */ - protected static final Set DISALLOWED_HEADERS = getDisallowedHeaders(); + private static final Set DISALLOWED_HEADERS = disallowedHeaders(); - private static Set getDisallowedHeaders() { + /** + * By default, {@link HttpRequest} does not allow {@code Connection}, + * {@code Content-Length}, {@code Expect}, {@code Host}, or {@code Upgrade} + * headers to be set, but this can be overriden with the + * {@code jdk.httpclient.allowRestrictedHeaders} system property. + * @see jdk.internal.net.http.common.Utils#getDisallowedHeaders() + */ + private static Set disallowedHeaders() { TreeSet headers = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); headers.addAll(Set.of("connection", "content-length", "expect", "host", "upgrade")); @@ -65,6 +68,7 @@ class JdkClientHttpRequest extends AbstractStreamingClientHttpRequest { return Collections.unmodifiableSet(headers); } + private final HttpClient httpClient; private final HttpMethod method; @@ -123,11 +127,9 @@ class JdkClientHttpRequest extends AbstractStreamingClientHttpRequest { } headers.forEach((headerName, headerValues) -> { - if (!headerName.equalsIgnoreCase(HttpHeaders.CONTENT_LENGTH)) { - if (!DISALLOWED_HEADERS.contains(headerName.toLowerCase())) { - for (String headerValue : headerValues) { - builder.header(headerName, headerValue); - } + if (!DISALLOWED_HEADERS.contains(headerName.toLowerCase())) { + for (String headerValue : headerValues) { + builder.header(headerName, headerValue); } } }); diff --git a/spring-web/src/test/java/org/springframework/http/client/AbstractMockWebServerTests.java b/spring-web/src/test/java/org/springframework/http/client/AbstractMockWebServerTests.java index cbef27287d..1577c81e61 100644 --- a/spring-web/src/test/java/org/springframework/http/client/AbstractMockWebServerTests.java +++ b/spring-web/src/test/java/org/springframework/http/client/AbstractMockWebServerTests.java @@ -79,6 +79,11 @@ public abstract class AbstractMockWebServerTests { else if(request.getPath().equals("/status/notfound")) { return new MockResponse().setResponseCode(404); } + else if (request.getPath().equals("/status/299")) { + assertThat(request.getHeader("Expect")) + .contains("299"); + return new MockResponse().setResponseCode(299); + } else if(request.getPath().startsWith("/params")) { assertThat(request.getPath()).contains("param1=value"); assertThat(request.getPath()).contains("param2=value1¶m2=value2"); diff --git a/spring-web/src/test/java/org/springframework/http/client/JdkClientHttpRequestFactoryTests.java b/spring-web/src/test/java/org/springframework/http/client/JdkClientHttpRequestFactoryTests.java index d03fca2f77..cce3a37989 100644 --- a/spring-web/src/test/java/org/springframework/http/client/JdkClientHttpRequestFactoryTests.java +++ b/spring-web/src/test/java/org/springframework/http/client/JdkClientHttpRequestFactoryTests.java @@ -16,14 +16,16 @@ package org.springframework.http.client; +import java.io.IOException; import java.net.URI; -import java.net.http.HttpClient; -import java.time.Duration; -import java.util.concurrent.Executor; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatusCode; +import org.springframework.lang.Nullable; import static org.assertj.core.api.Assertions.assertThat; @@ -32,6 +34,25 @@ import static org.assertj.core.api.Assertions.assertThat; */ public class JdkClientHttpRequestFactoryTests extends AbstractHttpRequestFactoryTests { + @Nullable + private static String originalPropertyValue; + + @BeforeAll + public static void setProperty() { + originalPropertyValue = System.getProperty("jdk.httpclient.allowRestrictedHeaders"); + System.setProperty("jdk.httpclient.allowRestrictedHeaders", "expect"); + } + + @AfterAll + public static void restoreProperty() { + if (originalPropertyValue != null) { + System.setProperty("jdk.httpclient.allowRestrictedHeaders", originalPropertyValue); + } + else { + System.clearProperty("jdk.httpclient.allowRestrictedHeaders"); + } + } + @Override protected ClientHttpRequestFactory createRequestFactory() { return new JdkClientHttpRequestFactory(); @@ -45,25 +66,13 @@ public class JdkClientHttpRequestFactoryTests extends AbstractHttpRequestFactory } @Test - public void customizeDisallowedHeaders() { - String original = System.getProperty("jdk.httpclient.allowRestrictedHeaders"); - System.setProperty("jdk.httpclient.allowRestrictedHeaders", "host"); + public void customizeDisallowedHeaders() throws IOException { + ClientHttpRequest request = factory.createRequest(URI.create(this.baseUrl + "/status/299"), HttpMethod.PUT); + request.getHeaders().set("Expect", "299"); - assertThat(TestJdkClientHttpRequest.DISALLOWED_HEADERS).doesNotContain("host"); - - if (original != null) { - System.setProperty("jdk.httpclient.allowRestrictedHeaders", original); - } - else { - System.clearProperty("jdk.httpclient.allowRestrictedHeaders"); - } - } - - static class TestJdkClientHttpRequest extends JdkClientHttpRequest { - - public TestJdkClientHttpRequest(HttpClient httpClient, URI uri, HttpMethod method, Executor executor, Duration readTimeout) { - super(httpClient, uri, method, executor, readTimeout); - } + try (ClientHttpResponse response = request.execute()) { + assertThat(response.getStatusCode()).as("Invalid status code").isEqualTo(HttpStatusCode.valueOf(299)); + } } }