Polishing external contribution

See gh-30787
Closes gh-30788

Author: poutsma

spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequest.java

@@ -47,13 +47,16 @@
  */
 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.
+	private static final Set<String> DISALLOWED_HEADERS = disallowedHeaders();
+
+	/**
+	 * 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()
 	 */
-	protected static final Set<String> DISALLOWED_HEADERS = getDisallowedHeaders();
-
-	private static Set<String> getDisallowedHeaders() {
+	private static Set<String> disallowedHeaders() {
 		TreeSet<String> headers = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
 		headers.addAll(Set.of("connection", "content-length", "expect", "host", "upgrade"));
 
@@ -65,6 +68,7 @@ private static Set<String> getDisallowedHeaders() {
 		return Collections.unmodifiableSet(headers);
 	}
 
+
 	private final HttpClient httpClient;
 
 	private final HttpMethod method;
@@ -123,11 +127,9 @@ private HttpRequest buildRequest(HttpHeaders headers, @Nullable Body body) {
 		}
 
 		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);
 				}
 			}
 		});

spring-web/src/test/java/org/springframework/http/client/AbstractMockWebServerTests.java

@@ -79,6 +79,11 @@ else if(request.getPath().equals("/status/ok")) {
 				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&param2=value2");

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 @@
  */
 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 void httpMethods() throws Exception {
 	}
 
 	@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));
+			}
 	}
 
 }