mirror of
https://github.com/spring-projects/spring-framework.git
synced 2026-10-09 00:29:04 +00:00
Test JdkClientHttpRequest header filtering in isolation
Prior to this commit, JdkClientHttpRequestFactoryTests verified the `jdk.httpclient.allowRestrictedHeaders` support by setting the system property in a @BeforeAll method and sending a request with an "Expect" header. However, the JDK HttpClient reads that property only once, in a static initializer, and JdkClientHttpRequest caches it in a static field as well. Consequently, the test failed whenever another test class (such as ClientHttpConnectorTests) initialized the HttpClient first in the same JVM, making the build dependent on test class execution order. That test also exercised the JDK's own enforcement of restricted headers more than any logic in Spring, so it provided little value for the cost. Since we aim for reliable builds, in order to avoid problems with the Spring portfolio release trains, such order-dependent failures are not acceptable. This commit therefore removes the end-to-end test, makes JdkClientHttpRequest.disallowedHeaders() package-private, and tests it directly, setting and restoring the system property around each call.
This commit is contained in:
@@ -216,7 +216,7 @@ class JdkClientHttpRequest extends AbstractStreamingClientHttpRequest {
|
||||
* {@code jdk.httpclient.allowRestrictedHeaders} system property.
|
||||
* @see jdk.internal.net.http.common.Utils#getDisallowedHeaders()
|
||||
*/
|
||||
private static Set<String> disallowedHeaders() {
|
||||
static Set<String> disallowedHeaders() {
|
||||
TreeSet<String> headers = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
|
||||
headers.addAll(Set.of("connection", "content-length", "expect", "host", "upgrade"));
|
||||
|
||||
|
||||
-35
@@ -21,9 +21,6 @@ import java.net.URI;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.time.Duration;
|
||||
|
||||
import org.jspecify.annotations.Nullable;
|
||||
import org.junit.jupiter.api.AfterAll;
|
||||
import org.junit.jupiter.api.BeforeAll;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.condition.EnabledForJreRange;
|
||||
import org.junit.jupiter.api.condition.JRE;
|
||||
@@ -32,7 +29,6 @@ import org.junit.jupiter.params.provider.ValueSource;
|
||||
|
||||
import org.springframework.http.HttpMethod;
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.http.HttpStatusCode;
|
||||
import org.springframework.util.StreamUtils;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
@@ -45,26 +41,6 @@ import static org.assertj.core.api.Assertions.assertThat;
|
||||
*/
|
||||
class JdkClientHttpRequestFactoryTests extends AbstractHttpRequestFactoryTests {
|
||||
|
||||
private static @Nullable String originalPropertyValue;
|
||||
|
||||
|
||||
@BeforeAll
|
||||
static void setProperty() {
|
||||
originalPropertyValue = System.getProperty("jdk.httpclient.allowRestrictedHeaders");
|
||||
System.setProperty("jdk.httpclient.allowRestrictedHeaders", "expect");
|
||||
}
|
||||
|
||||
@AfterAll
|
||||
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();
|
||||
@@ -77,17 +53,6 @@ class JdkClientHttpRequestFactoryTests extends AbstractHttpRequestFactoryTests {
|
||||
assertHttpMethod("patch", HttpMethod.PATCH);
|
||||
}
|
||||
|
||||
@Test
|
||||
void customizeDisallowedHeaders() throws IOException {
|
||||
URI uri = URI.create(this.baseUrl + "/status/299");
|
||||
ClientHttpRequest request = this.factory.createRequest(uri, HttpMethod.PUT);
|
||||
request.getHeaders().set("Expect", "299");
|
||||
|
||||
try (ClientHttpResponse response = request.execute()) {
|
||||
assertThat(response.getStatusCode()).as("Invalid status code").isEqualTo(HttpStatusCode.valueOf(299));
|
||||
}
|
||||
}
|
||||
|
||||
@Test // gh-31451
|
||||
void contentLength0() throws IOException {
|
||||
URI uri = URI.create(this.baseUrl + "/methods/get");
|
||||
|
||||
+50
@@ -24,16 +24,19 @@ import java.net.http.HttpRequest;
|
||||
import java.net.http.HttpResponse;
|
||||
import java.net.http.HttpTimeoutException;
|
||||
import java.time.Duration;
|
||||
import java.util.Set;
|
||||
import java.util.concurrent.CompletableFuture;
|
||||
import java.util.concurrent.ExecutorService;
|
||||
import java.util.concurrent.Executors;
|
||||
|
||||
import org.jspecify.annotations.Nullable;
|
||||
import org.junit.jupiter.api.AutoClose;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import org.springframework.http.HttpHeaders;
|
||||
import org.springframework.http.HttpMethod;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.Mockito.mock;
|
||||
@@ -71,6 +74,53 @@ class JdkClientHttpRequestTests {
|
||||
.isExactlyInstanceOf(IOException.class);
|
||||
}
|
||||
|
||||
@Test
|
||||
void disallowedHeadersByDefault() {
|
||||
assertThat(disallowedHeaders(null))
|
||||
.containsExactlyInAnyOrder("connection", "content-length", "expect", "host", "upgrade");
|
||||
}
|
||||
|
||||
@Test
|
||||
void disallowedHeadersWithSingleHeaderAllowed() {
|
||||
assertThat(disallowedHeaders("expect"))
|
||||
.containsExactlyInAnyOrder("connection", "content-length", "host", "upgrade");
|
||||
}
|
||||
|
||||
@Test
|
||||
void disallowedHeadersWithMultipleHeadersAllowed() {
|
||||
assertThat(disallowedHeaders("expect,host"))
|
||||
.containsExactlyInAnyOrder("connection", "content-length", "upgrade");
|
||||
}
|
||||
|
||||
@Test
|
||||
void disallowedHeadersAreCaseInsensitive() {
|
||||
// AssertJ's contains() uses equals(), so go through Set.contains() instead.
|
||||
assertThat(disallowedHeaders(null).contains("EXPECT")).isTrue();
|
||||
assertThat(disallowedHeaders("Expect").contains("expect")).isFalse();
|
||||
}
|
||||
|
||||
private static Set<String> disallowedHeaders(@Nullable String allowRestrictedHeaders) {
|
||||
String key = "jdk.httpclient.allowRestrictedHeaders";
|
||||
String original = System.getProperty(key);
|
||||
try {
|
||||
if (allowRestrictedHeaders != null) {
|
||||
System.setProperty(key, allowRestrictedHeaders);
|
||||
}
|
||||
else {
|
||||
System.clearProperty(key);
|
||||
}
|
||||
return JdkClientHttpRequest.disallowedHeaders();
|
||||
}
|
||||
finally {
|
||||
if (original != null) {
|
||||
System.setProperty(key, original);
|
||||
}
|
||||
else {
|
||||
System.clearProperty(key);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private JdkClientHttpRequest createRequest(Duration timeout) {
|
||||
return new JdkClientHttpRequest(client, URI.create("https://abc.com"), HttpMethod.GET, executor, timeout, false);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user