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

Initial investigation into supporting HTTP/2 #44295

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
class JdkHttpClient implements HttpClient {
private static final ClientLogger LOGGER = new ClientLogger(JdkHttpClient.class);

private final java.net.http.HttpClient jdkHttpClient;
final java.net.http.HttpClient jdkHttpClient;

private final Set<String> restrictedHeaders;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package com.azure.core.http.jdk.httpclient;

import com.azure.core.http.HttpClient;
import com.azure.core.http.HttpProtocolVersion;
import com.azure.core.http.ProxyOptions;
import com.azure.core.http.jdk.httpclient.implementation.JdkHttpClientProxySelector;
import com.azure.core.util.Configuration;
Expand All @@ -19,6 +20,7 @@
import java.nio.file.Paths;
import java.time.Duration;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashSet;
import java.util.Locale;
import java.util.Properties;
Expand Down Expand Up @@ -63,6 +65,7 @@ public class JdkHttpClientBuilder {
private Duration writeTimeout;
private Duration responseTimeout;
private Duration readTimeout;
private EnumSet<HttpProtocolVersion> protocolVersions;

/**
* Creates JdkHttpClientBuilder.
Expand Down Expand Up @@ -227,6 +230,26 @@ public JdkHttpClientBuilder configuration(Configuration configuration) {
return this;
}

/**
* Sets the {@link HttpProtocolVersion protocol versions} that the {@link HttpClient} can use.
* <p>
* {@link HttpProtocolVersion#HTTP_1_1} must be included in the set of versions.
* <p>
* If the value is not set, only {@link HttpProtocolVersion#HTTP_1_1} can be used.
*
* @param protocolVersions The {@link HttpProtocolVersion protocol versions} that the {@link HttpClient} can use.
* @return The updated {@link JdkHttpClientBuilder} object.
*/
public JdkHttpClientBuilder setProtocolVersions(EnumSet<HttpProtocolVersion> protocolVersions) {
if (protocolVersions != null && !protocolVersions.contains(HttpProtocolVersion.HTTP_1_1)) {
throw LOGGER.logExceptionAsError(
new IllegalArgumentException("'protocolVersions' must contain HTTP_1_1 (HTTP/1.1)."));
}

this.protocolVersions = (protocolVersions == null) ? null : EnumSet.copyOf(protocolVersions);
return this;
}

/**
* Build a HttpClient with current configurations.
*
Expand All @@ -236,9 +259,6 @@ public HttpClient build() {
java.net.http.HttpClient.Builder httpClientBuilder
= this.httpClientBuilder == null ? java.net.http.HttpClient.newBuilder() : this.httpClientBuilder;

// Azure JDK http client supports HTTP 1.1 by default.
httpClientBuilder.version(java.net.http.HttpClient.Version.HTTP_1_1);

httpClientBuilder = httpClientBuilder.connectTimeout(getTimeout(connectionTimeout, getDefaultConnectTimeout()));

Duration writeTimeout = getTimeout(this.writeTimeout, getDefaultWriteTimeout());
Expand Down Expand Up @@ -269,6 +289,18 @@ public HttpClient build() {
new ProxyAuthenticator(buildProxyOptions.getUsername(), buildProxyOptions.getPassword()));
}
}

// The JDK HttpClientBuilder only allows configuring a single choice.
// If HTTP/2 is chosen it will attempt to use it, if not available it will fall back to HTTP/1.1.
// If HTTP/1.1 is chosen it will not attempt to use HTTP/2.
// The default if version is not set is HTTP/2, which we don't want. So, if HTTP/2 is not in the set we
// set the version explicitly to HTTP/1.1.
if (protocolVersions != null && protocolVersions.contains(HttpProtocolVersion.HTTP_2)) {
httpClientBuilder.version(java.net.http.HttpClient.Version.HTTP_2);
} else {
httpClientBuilder.version(java.net.http.HttpClient.Version.HTTP_1_1);
}

return new JdkHttpClient(httpClientBuilder.build(), Collections.unmodifiableSet(getRestrictedHeaders()),
writeTimeout, responseTimeout, readTimeout);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ public HttpClient createInstance(HttpClientOptions clientOptions) {
.connectionTimeout(clientOptions.getConnectTimeout())
.writeTimeout(clientOptions.getWriteTimeout())
.responseTimeout(clientOptions.getResponseTimeout())
.readTimeout(clientOptions.getReadTimeout());
.readTimeout(clientOptions.getReadTimeout())
.setProtocolVersions(clientOptions.getProtocolVersions());

return builder.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import com.azure.core.http.HttpClient;
import com.azure.core.http.HttpMethod;
import com.azure.core.http.HttpProtocolVersion;
import com.azure.core.http.HttpRequest;
import com.azure.core.http.ProxyOptions;
import com.azure.core.util.Configuration;
Expand All @@ -24,6 +25,7 @@
import java.net.InetSocketAddress;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.EnumSet;
import java.util.List;
import java.util.Properties;
import java.util.Set;
Expand All @@ -39,6 +41,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

/**
Expand Down Expand Up @@ -289,6 +292,32 @@ void testCaseInsensitivity() {
assertFalse(restrictedHeaders.contains("content-length"), "content-length not removed");
}

@Test
public void noHttpProtocolResultsInJustHttp11() {
JdkHttpClient client = (JdkHttpClient) new JdkHttpClientBuilder().build();

java.net.http.HttpClient.Version version = client.jdkHttpClient.version();
assertEquals(java.net.http.HttpClient.Version.HTTP_1_1, version);
}

@Test
public void missingHttp11InHttpProtocolsThrows() {
assertThrows(IllegalArgumentException.class,
() -> new JdkHttpClientBuilder().setProtocolVersions(EnumSet.noneOf(HttpProtocolVersion.class)));
assertThrows(IllegalArgumentException.class,
() -> new JdkHttpClientBuilder().setProtocolVersions(EnumSet.of(HttpProtocolVersion.HTTP_2)));
}

@Test
public void settingHttp2InHttpProtocolsAllowsHttp2() {
JdkHttpClient client = (JdkHttpClient) new JdkHttpClientBuilder()
.setProtocolVersions(EnumSet.of(HttpProtocolVersion.HTTP_2, HttpProtocolVersion.HTTP_1_1))
.build();

java.net.http.HttpClient.Version version = client.jdkHttpClient.version();
assertEquals(java.net.http.HttpClient.Version.HTTP_2, version);
}

private static void configurationProxyTest(Configuration configuration) {
HttpClient httpClient
= new JdkHttpClientBuilder(java.net.http.HttpClient.newBuilder()).configuration(configuration).build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

package com.azure.core.http.netty;

import com.azure.core.http.HttpProtocolVersion;
import com.azure.core.http.HttpRequest;
import com.azure.core.http.ProxyOptions;
import com.azure.core.http.netty.implementation.AzureNettyHttpClientContext;
Expand All @@ -24,6 +25,7 @@
import io.netty.resolver.NoopAddressResolverGroup;
import reactor.netty.Connection;
import reactor.netty.NettyPipeline;
import reactor.netty.http.HttpProtocol;
import reactor.netty.http.client.HttpClient;
import reactor.netty.http.client.HttpClientRequest;
import reactor.netty.http.client.HttpResponseDecoderSpec;
Expand All @@ -35,6 +37,7 @@
import java.net.SocketAddress;
import java.nio.ByteBuffer;
import java.time.Duration;
import java.util.EnumSet;
import java.util.Objects;
import java.util.concurrent.atomic.AtomicReference;
import java.util.regex.Pattern;
Expand Down Expand Up @@ -130,6 +133,7 @@ public class NettyAsyncHttpClientBuilder {
private Duration writeTimeout;
private Duration responseTimeout;
private Duration readTimeout;
private EnumSet<HttpProtocolVersion> protocolVersions;

/**
* Creates a new builder instance, where a builder is capable of generating multiple instances of {@link
Expand Down Expand Up @@ -273,9 +277,39 @@ public com.azure.core.http.HttpClient build() {
}
}

if (protocolVersions != null) {
nettyHttpClient = nettyHttpClient.protocol(toNettyProtocol(protocolVersions));
} else {
nettyHttpClient = nettyHttpClient.protocol(HttpProtocol.HTTP11);
}

return new NettyAsyncHttpClient(nettyHttpClient, disableBufferCopy, addProxyHandler);
}

private static HttpProtocol[] toNettyProtocol(EnumSet<HttpProtocolVersion> protocolVersions) {
HttpProtocol[] httpProtocols = new HttpProtocol[protocolVersions.size()];
int i = 0;
for (HttpProtocolVersion version : protocolVersions) {
switch (version) {
case HTTP_1_1:
httpProtocols[i] = HttpProtocol.HTTP11;
break;

case HTTP_2:
httpProtocols[i] = HttpProtocol.H2;
break;

default:
throw LOGGER
.logExceptionAsError(new IllegalArgumentException("Unsupported protocol version: " + version));
}

i++;
}

return httpProtocols;
}

/**
* Sets the connection provider.
*
Expand Down Expand Up @@ -523,6 +557,26 @@ public NettyAsyncHttpClientBuilder readTimeout(Duration readTimeout) {
return this;
}

/**
* Sets the {@link HttpProtocolVersion protocol versions} that the {@link HttpClient} can use.
* <p>
* {@link HttpProtocolVersion#HTTP_1_1} must be included in the set of versions.
* <p>
* If the value is not set, only {@link HttpProtocolVersion#HTTP_1_1} can be used.
*
* @param protocolVersions The {@link HttpProtocolVersion protocol versions} that the {@link HttpClient} can use.
* @return The updated {@link NettyAsyncHttpClientBuilder} object.
*/
public NettyAsyncHttpClientBuilder setProtocolVersions(EnumSet<HttpProtocolVersion> protocolVersions) {
if (protocolVersions != null && !protocolVersions.contains(HttpProtocolVersion.HTTP_1_1)) {
throw LOGGER.logExceptionAsError(
new IllegalArgumentException("'protocolVersions' must contain HTTP_1_1 (HTTP/1.1)."));
}

this.protocolVersions = (protocolVersions == null) ? null : EnumSet.copyOf(protocolVersions);
return this;
}

private static boolean shouldUseCustomProxyHandler(ProxyOptions options) {
return options != null && options.getUsername() != null && options.getType() == ProxyOptions.Type.HTTP;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ public HttpClient createInstance(HttpClientOptions clientOptions) {
.connectTimeout(clientOptions.getConnectTimeout())
.writeTimeout(clientOptions.getWriteTimeout())
.responseTimeout(clientOptions.getResponseTimeout())
.readTimeout(clientOptions.getReadTimeout());
.readTimeout(clientOptions.getReadTimeout())
.setProtocolVersions(clientOptions.getProtocolVersions());

ConnectionProvider.Builder connectionProviderBuilder = ConnectionProvider.builder("azure-sdk");
connectionProviderBuilder.maxIdleTime(clientOptions.getConnectionIdleTimeout());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@
package com.azure.core.http.netty;

import com.azure.core.http.HttpMethod;
import com.azure.core.http.HttpProtocolVersion;
import com.azure.core.http.HttpRequest;
import com.azure.core.http.ProxyOptions;
import com.azure.core.http.netty.implementation.NettyHttpClientLocalTestServer;
import com.azure.core.validation.http.models.TestConfigurationSource;
import com.azure.core.util.Configuration;
import com.azure.core.util.ConfigurationBuilder;
import com.azure.core.util.ConfigurationSource;
import com.azure.core.validation.http.models.TestConfigurationSource;
import io.netty.channel.ChannelDuplexHandler;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelPromise;
Expand All @@ -28,6 +29,7 @@
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import reactor.netty.http.HttpProtocol;
import reactor.netty.http.client.HttpClient;
import reactor.netty.http.client.HttpResponseDecoderSpec;
import reactor.netty.resources.ConnectionProvider;
Expand All @@ -39,6 +41,7 @@
import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.EnumSet;
import java.util.List;
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;
Expand Down Expand Up @@ -571,4 +574,33 @@ public void preconfiguredHttpResponseDecoderIsMaintained() {
assertFalse(spec.validateHeaders());
assertEquals(64 * 1024, spec.maxChunkSize());
}

@Test
public void noHttpProtocolResultsInJustHttp11() {
NettyAsyncHttpClient client = (NettyAsyncHttpClient) new NettyAsyncHttpClientBuilder().build();

HttpProtocol[] protocols = client.nettyClient.configuration().protocols();
assertEquals(1, protocols.length);
assertEquals(HttpProtocol.HTTP11, protocols[0]);
}

@Test
public void missingHttp11InHttpProtocolsThrows() {
assertThrows(IllegalArgumentException.class,
() -> new NettyAsyncHttpClientBuilder().setProtocolVersions(EnumSet.noneOf(HttpProtocolVersion.class)));
assertThrows(IllegalArgumentException.class,
() -> new NettyAsyncHttpClientBuilder().setProtocolVersions(EnumSet.of(HttpProtocolVersion.HTTP_2)));
}

@Test
public void settingHttp2InHttpProtocolsAllowsHttp2() {
NettyAsyncHttpClient client = (NettyAsyncHttpClient) new NettyAsyncHttpClientBuilder()
.setProtocolVersions(EnumSet.of(HttpProtocolVersion.HTTP_2, HttpProtocolVersion.HTTP_1_1))
.build();

List<HttpProtocol> protocols = Arrays.asList(client.nettyClient.configuration().protocols());
assertEquals(2, protocols.size());
assertTrue(protocols.contains(HttpProtocol.H2), "Expected HTTP/2 to be in the protocols");
assertTrue(protocols.contains(HttpProtocol.HTTP11), "Expected HTTP/1.1 to be in the protocols");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ public HttpClient createInstance(HttpClientOptions clientOptions) {
.configuration(clientOptions.getConfiguration())
.connectionTimeout(clientOptions.getConnectTimeout())
.writeTimeout(clientOptions.getWriteTimeout())
.readTimeout(clientOptions.getReadTimeout());
.readTimeout(clientOptions.getReadTimeout())
.setProtocolVersions(clientOptions.getProtocolVersions());

Integer poolSize = clientOptions.getMaximumConnectionPoolSize();
int maximumConnectionPoolSize = (poolSize != null && poolSize > 0) ? poolSize : 5; // By default, OkHttp uses a
Expand Down
Loading
Loading