Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allows JdkClientHttpRequest.DISALLOWED_HEADERS to be customized. #30788

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,17 @@
import java.net.http.HttpResponse;
import java.nio.ByteBuffer;
import java.time.Duration;
import java.util.List;
import java.util.Collections;
import java.util.Set;
import java.util.TreeSet;
import java.util.concurrent.Executor;
import java.util.concurrent.Flow;

import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.lang.Nullable;
import org.springframework.util.StreamUtils;
import org.springframework.util.StringUtils;

/**
* {@link ClientHttpRequest} implementation based the Java {@link HttpClient}.
Expand All @@ -48,8 +51,19 @@ 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 List<String> DISALLOWED_HEADERS =
List.of("connection", "content-length", "expect", "host", "upgrade");
protected static final Set<String> DISALLOWED_HEADERS = getDisallowedHeaders();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

protected was changed to test without much setup.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AbstractMockWebServerTests dispatch for /echo checks that Host is localhost:port, so unsure how to test otherwise.


private static Set<String> getDisallowedHeaders() {
TreeSet<String> headers = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
headers.addAll(Set.of("connection", "content-length", "expect", "host", "upgrade"));

String headersToAllow = System.getProperty("jdk.httpclient.allowRestrictedHeaders");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe there is a setter somewhere and we set the property rather than trying to read it?

if (headersToAllow != null) {
Set<String> toAllow = StringUtils.commaDelimitedListToSet(headersToAllow);
headers.removeAll(toAllow);
}
return Collections.unmodifiableSet(headers);
}

private final HttpClient httpClient;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,17 @@

package org.springframework.http.client;

import java.net.URI;
import java.net.http.HttpClient;
import java.time.Duration;
import java.util.concurrent.Executor;

import org.junit.jupiter.api.Test;

import org.springframework.http.HttpMethod;

import static org.assertj.core.api.Assertions.assertThat;

/**
* @author Marten Deinum
*/
Expand All @@ -37,4 +44,26 @@ public void httpMethods() throws Exception {
assertHttpMethod("patch", HttpMethod.PATCH);
}

@Test
public void customizeDisallowedHeaders() {
String original = System.getProperty("jdk.httpclient.allowRestrictedHeaders");
System.setProperty("jdk.httpclient.allowRestrictedHeaders", "host");

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);
}
}

}