Skip to content

Commit

Permalink
Optimize header handling in JDK HttpClient (#38285)
Browse files Browse the repository at this point in the history
Optimize header handling in JDK HttpClient
  • Loading branch information
alzimmermsft authored Jan 25, 2024
1 parent 12205a1 commit 48843d7
Show file tree
Hide file tree
Showing 8 changed files with 252 additions and 104 deletions.
21 changes: 0 additions & 21 deletions sdk/core/azure-core-http-jdk-httpclient/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -126,27 +126,6 @@
<version>5.9.3</version> <!-- {x-version-update;org.junit.jupiter:junit-jupiter-params;external_dependency} -->
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<version>4.11.0</version><!-- {x-version-update;org.mockito:mockito-core;external_dependency} -->
<scope>test</scope>
</dependency>
<!-- bytebuddy dependencies are required for mockito 4.11.0 to work with Java 21. Mockito 4.11.0 is the last release -->
<!-- of Mockito supporting Java 8 as a baseline. -->
<dependency>
<groupId>net.bytebuddy</groupId>
<artifactId>byte-buddy</artifactId>
<version>1.14.8</version> <!-- {x-version-update;testdep_net.bytebuddy:byte-buddy;external_dependency} -->
<scope>test</scope>
</dependency>
<dependency>
<groupId>net.bytebuddy</groupId>
<artifactId>byte-buddy-agent</artifactId>
<version>1.14.8</version> <!-- {x-version-update;testdep_net.bytebuddy:byte-buddy-agent;external_dependency} -->
<scope>test</scope>
</dependency>
</dependencies>

<profiles>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,22 @@
package com.azure.core.http.jdk.httpclient;

import com.azure.core.http.HttpClient;
import com.azure.core.http.HttpHeader;
import com.azure.core.http.HttpHeaders;
import com.azure.core.http.HttpRequest;
import com.azure.core.http.HttpResponse;
import com.azure.core.http.jdk.httpclient.implementation.BodyPublisherUtils;
import com.azure.core.http.jdk.httpclient.implementation.AzureJdkHttpRequest;
import com.azure.core.http.jdk.httpclient.implementation.JdkHttpResponseAsync;
import com.azure.core.http.jdk.httpclient.implementation.JdkHttpResponseSync;
import com.azure.core.util.Context;
import com.azure.core.util.Contexts;
import com.azure.core.util.CoreUtils;
import com.azure.core.util.ProgressReporter;
import com.azure.core.util.logging.ClientLogger;
import reactor.core.Exceptions;
import reactor.core.publisher.Mono;

import java.io.IOException;
import java.io.UncheckedIOException;
import java.net.URISyntaxException;
import java.util.Set;

import static com.azure.core.http.jdk.httpclient.implementation.JdkHttpUtils.fromJdkHttpHeaders;
import static java.net.http.HttpRequest.BodyPublishers.noBody;
import static java.net.http.HttpResponse.BodyHandlers.ofByteArray;
import static java.net.http.HttpResponse.BodyHandlers.ofInputStream;
import static java.net.http.HttpResponse.BodyHandlers.ofPublisher;
Expand Down Expand Up @@ -119,38 +113,7 @@ public HttpResponse sendSync(HttpRequest request, Context context) {
* @return the HttpRequest
*/
private java.net.http.HttpRequest toJdkHttpRequest(HttpRequest request, Context context) {
ProgressReporter progressReporter = Contexts.with(context).getHttpRequestProgressReporter();

final java.net.http.HttpRequest.Builder builder = java.net.http.HttpRequest.newBuilder();
try {
builder.uri(request.getUrl().toURI());
} catch (URISyntaxException e) {
throw LOGGER.logExceptionAsError(Exceptions.propagate(e));
}
final HttpHeaders headers = request.getHeaders();
if (headers != null) {
for (HttpHeader header : headers) {
final String headerName = header.getName();
if (!restrictedHeaders.contains(headerName)) {
header.getValuesList().forEach(headerValue -> builder.header(headerName, headerValue));
} else {
LOGGER.warning("The header '" + headerName + "' is restricted by default in JDK HttpClient 12 "
+ "and above. This header can be added to allow list in JAVA_HOME/conf/net.properties "
+ "or in System.setProperty() or in Configuration. Use the key 'jdk.httpclient"
+ ".allowRestrictedHeaders' and a comma separated list of header names.");
}
}
}
switch (request.getHttpMethod()) {
case GET:
return builder.GET().build();
case HEAD:
return builder.method("HEAD", noBody()).build();
default:
java.net.http.HttpRequest.BodyPublisher bodyPublisher = BodyPublisherUtils.toBodyPublisher(request,
progressReporter);
return builder.method(request.getHttpMethod().toString(), bodyPublisher).build();
}
return new AzureJdkHttpRequest(request, context, restrictedHeaders, LOGGER);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@
import java.nio.file.Paths;
import java.time.Duration;
import java.util.Collections;
import java.util.HashSet;
import java.util.Locale;
import java.util.Objects;
import java.util.Properties;
import java.util.Set;
import java.util.TreeSet;
import java.util.concurrent.Executor;

/**
Expand All @@ -44,15 +45,7 @@ public class JdkHttpClientBuilder {
static final Set<String> DEFAULT_RESTRICTED_HEADERS;

static {
TreeSet<String> treeSet = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
treeSet.addAll(Set.of(
"connection",
"content-length",
"expect",
"host",
"upgrade"
));
DEFAULT_RESTRICTED_HEADERS = Collections.unmodifiableSet(treeSet);
DEFAULT_RESTRICTED_HEADERS = Set.of("connection", "content-length", "expect", "host", "upgrade");
}

private static final ClientLogger LOGGER = new ClientLogger(JdkHttpClientBuilder.class);
Expand Down Expand Up @@ -204,37 +197,29 @@ public HttpClient build() {

Set<String> getRestrictedHeaders() {
// Compute the effective restricted headers by removing the allowed headers from default restricted headers
Set<String> allowRestrictedHeaders = getAllowRestrictedHeaders();
Set<String> restrictedHeaders = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
restrictedHeaders.addAll(DEFAULT_RESTRICTED_HEADERS);
restrictedHeaders.removeAll(allowRestrictedHeaders);
Set<String> restrictedHeaders = new HashSet<>(DEFAULT_RESTRICTED_HEADERS);
removeAllowedHeaders(restrictedHeaders);
return restrictedHeaders;
}

private Set<String> getAllowRestrictedHeaders() {
private void removeAllowedHeaders(Set<String> restrictedHeaders) {
Properties properties = getNetworkProperties();
String[] allowRestrictedHeadersNetProperties =
properties.getProperty(JDK_HTTPCLIENT_ALLOW_RESTRICTED_HEADERS, "").split(",");

// Read all allowed restricted headers from configuration
Configuration config = (this.configuration == null)
? Configuration.getGlobalConfiguration()
: configuration;
Configuration config = (this.configuration == null) ? Configuration.getGlobalConfiguration() : configuration;
String[] allowRestrictedHeadersSystemProperties = config.get(JDK_HTTPCLIENT_ALLOW_RESTRICTED_HEADERS, "")
.split(",");

Set<String> allowRestrictedHeaders = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);

// Combine the set of all allowed restricted headers from both sources
for (String header : allowRestrictedHeadersSystemProperties) {
allowRestrictedHeaders.add(header.trim());
restrictedHeaders.remove(header.trim().toLowerCase(Locale.ROOT));
}

for (String header : allowRestrictedHeadersNetProperties) {
allowRestrictedHeaders.add(header.trim());
restrictedHeaders.remove(header.trim().toLowerCase(Locale.ROOT));
}

return allowRestrictedHeaders;
}

Properties getNetworkProperties() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
package com.azure.core.http.jdk.httpclient.implementation;

import com.azure.core.http.HttpMethod;
import com.azure.core.implementation.util.HttpHeadersAccessHelper;
import com.azure.core.util.Context;
import com.azure.core.util.Contexts;
import com.azure.core.util.ProgressReporter;
import com.azure.core.util.logging.ClientLogger;
import reactor.core.Exceptions;

import java.net.URI;
import java.net.URISyntaxException;
import java.net.http.HttpClient;
import java.net.http.HttpHeaders;
import java.net.http.HttpRequest;
import java.time.Duration;
import java.util.Optional;
import java.util.Set;
import java.util.TreeMap;

import static java.net.http.HttpRequest.BodyPublishers.noBody;

/**
* Implementation of the JDK {@link HttpRequest}.
* <p>
* Using this instead of {@link HttpRequest#newBuilder()} allows us to optimize some cases now allowed by the builder.
* For example, setting headers requires each key-value for the same header to be set individually. This class allows
* us to set all headers at once. And given that the headers are backed by a {@link TreeMap} it reduces the number of
* String comparisons performed.
*/
public final class AzureJdkHttpRequest extends HttpRequest {
private final BodyPublisher bodyPublisher;
private final String method;
private final URI uri;
private final HttpHeaders headers;

/**
* Creates a new instance of the JDK HttpRequest.
*
* @param azureCoreRequest The Azure Core request to create the JDK HttpRequest from.
* @param context The context of the request.
* @param restrictedHeaders The set of restricted headers.
* @param logger The logger to log warnings to.
*/
public AzureJdkHttpRequest(com.azure.core.http.HttpRequest azureCoreRequest, Context context,
Set<String> restrictedHeaders, ClientLogger logger) {
HttpMethod method = azureCoreRequest.getHttpMethod();
ProgressReporter progressReporter = Contexts.with(context).getHttpRequestProgressReporter();

this.method = method.toString();
this.bodyPublisher = (method == HttpMethod.GET || method == HttpMethod.HEAD)
? noBody() : BodyPublisherUtils.toBodyPublisher(azureCoreRequest, progressReporter);

try {
uri = azureCoreRequest.getUrl().toURI();
} catch (URISyntaxException e) {
throw logger.logExceptionAsError(Exceptions.propagate(e));
}

this.headers = HttpHeaders.of(new HeaderFilteringMap(
HttpHeadersAccessHelper.getRawHeaderMap(azureCoreRequest.getHeaders()), restrictedHeaders, logger),
(ignored1, ignored2) -> true);
}

@Override
public Optional<BodyPublisher> bodyPublisher() {
return Optional.ofNullable(bodyPublisher);
}

@Override
public String method() {
return method;
}

@Override
public Optional<Duration> timeout() {
return Optional.empty();
}

@Override
public boolean expectContinue() {
return false;
}

@Override
public URI uri() {
return uri;
}

@Override
public Optional<HttpClient.Version> version() {
return Optional.empty();
}

@Override
public HttpHeaders headers() {
return headers;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
package com.azure.core.http.jdk.httpclient.implementation;

import com.azure.core.http.HttpHeader;
import com.azure.core.util.logging.ClientLogger;

import java.util.AbstractMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.BiConsumer;
import java.util.function.BiPredicate;

/**
* This class is a {@link Map} implementation that filters out headers that are not allowed to be set on an HTTP
* request.
* <p>
* Based on logic used in {@link java.net.http.HttpHeaders#of(Map, BiPredicate)} it's known that the headers will be
* accessed using {@link Map#forEach(BiConsumer)}. Give that, this class can be optimized to use the forEach method
* on the raw HttpHeaders map used in azure-core which can filter out the headers that are not allowed to be set using
* the known lowercase header names, then return the cased header name and values.
*/
final class HeaderFilteringMap extends AbstractMap<String, List<String>> {
private final Map<String, HttpHeader> rawHeaders;
private final Set<String> restrictedHeaders;
private final ClientLogger logger;

/**
* Creates a new HeaderFilteringMap.
*
* @param rawHeaders The raw headers map.
* @param restrictedHeaders The header filter.
* @param logger The logger to log any errors.
*/
HeaderFilteringMap(Map<String, HttpHeader> rawHeaders, Set<String> restrictedHeaders, ClientLogger logger) {
this.rawHeaders = rawHeaders;
this.restrictedHeaders = restrictedHeaders;
this.logger = logger;
}

@Override
public Set<Entry<String, List<String>>> entrySet() {
throw logger.logExceptionAsError(
new UnsupportedOperationException("The only operation permitted by this Map is forEach."));
}

@Override
public void forEach(BiConsumer<? super String, ? super List<String>> action) {
rawHeaders.forEach((headerName, header) -> {
if (restrictedHeaders.contains(headerName)) {
logger.atWarning()
.addKeyValue("headerName", headerName)
.log("The header is restricted by 'java.net.http.HttpClient' and will be ignored. To allow this "
+ "header to be set on the request, configure 'jdk.httpclient.allowRestrictedHeaders' with the "
+ "header added in the comma-separated list.");
} else {
action.accept(header.getName(), header.getValuesList());
}
});
}
}
Loading

0 comments on commit 48843d7

Please sign in to comment.