From ff218ccd6698cb3e86a128f7939aa4926c213feb Mon Sep 17 00:00:00 2001 From: Idel Pivnitskiy Date: Mon, 17 Jun 2024 15:30:32 -0700 Subject: [PATCH] Multi-address client: take advantage of lowercase scheme (#2975) Motivation: In #2938 we made it guaranteed that `HttpRequestMetaData#scheme()` result is always in lowercase. We can use that to simplify scheme checks inside `DefaultMultiAddressUrlHttpClientBuilder`. Modifications: - Replace all scheme checks from `equalsIgnoreCase` to `equals`; - Extract exact same instance of `https` string from `Uri3986` to speed up `equals` check; Result: No need to worry about cases for scheme check. --- .../DefaultMultiAddressUrlHttpClientBuilder.java | 16 ++++++++++++---- .../http/netty/SslAndNonSslConnectionsTest.java | 4 ++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/DefaultMultiAddressUrlHttpClientBuilder.java b/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/DefaultMultiAddressUrlHttpClientBuilder.java index d647f966f0..a4a24024a8 100644 --- a/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/DefaultMultiAddressUrlHttpClientBuilder.java +++ b/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/DefaultMultiAddressUrlHttpClientBuilder.java @@ -27,6 +27,7 @@ import io.servicetalk.context.api.ContextMap; import io.servicetalk.http.api.DefaultHttpHeadersFactory; import io.servicetalk.http.api.DefaultStreamingHttpRequestResponseFactory; +import io.servicetalk.http.api.EmptyHttpHeaders; import io.servicetalk.http.api.FilterableReservedStreamingHttpConnection; import io.servicetalk.http.api.FilterableStreamingHttpClient; import io.servicetalk.http.api.HttpContextKeys; @@ -58,6 +59,7 @@ import java.net.InetSocketAddress; import java.net.MalformedURLException; +import java.util.Locale; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.function.Function; @@ -76,6 +78,8 @@ import static io.servicetalk.http.api.HttpHeaderNames.HOST; import static io.servicetalk.http.api.HttpProtocolVersion.HTTP_1_0; import static io.servicetalk.http.api.HttpProtocolVersion.HTTP_1_1; +import static io.servicetalk.http.api.HttpRequestMetaDataFactory.newRequestMetaData; +import static io.servicetalk.http.api.HttpRequestMethod.GET; import static io.servicetalk.http.netty.DefaultSingleAddressHttpClientBuilder.setExecutionContext; import static java.util.Objects.requireNonNull; @@ -96,7 +100,11 @@ final class DefaultMultiAddressUrlHttpClientBuilder private static final Logger LOGGER = LoggerFactory.getLogger(DefaultMultiAddressUrlHttpClientBuilder.class); - private static final String HTTPS_SCHEME = HTTPS.toString(); + // Use HttpRequestMetaData to access "https" constant used by Uri3986 class to optimize "equals" check to be a + // trivial reference check. + @SuppressWarnings("DataFlowIssue") + private static final String HTTPS_SCHEME = newRequestMetaData(HTTP_1_1, GET, "https://invalid./", + EmptyHttpHeaders.INSTANCE).scheme(); private final Function> builderFactory; private final HttpExecutionContextBuilder executionContextBuilder = new HttpExecutionContextBuilder(); @@ -159,10 +167,11 @@ private static final class CachingKeyFactory implements AsyncCloseable { UrlKey get(final HttpRequestMetaData metaData) throws MalformedURLException { final String scheme = ensureUrlComponentNonNull(metaData.scheme(), "scheme"); + assert scheme.equals(scheme.toLowerCase(Locale.ENGLISH)) : "scheme must be in lowercase"; final String host = ensureUrlComponentNonNull(metaData.host(), "host"); final int parsedPort = metaData.port(); final int port = parsedPort >= 0 ? parsedPort : - (HTTPS_SCHEME.equalsIgnoreCase(scheme) ? defaultHttpsPort : defaultHttpPort); + (HTTPS_SCHEME.equals(scheme) ? defaultHttpsPort : defaultHttpPort); setHostHeader(metaData); metaData.requestTarget(absoluteToRelativeFormRequestTarget(metaData.requestTarget(), scheme, host)); @@ -260,7 +269,7 @@ public StreamingHttpClient apply(final UrlKey urlKey) { requireNonNull(builderFactory.apply(urlKey.hostAndPort)); setExecutionContext(builder, executionContext); - if (HTTPS_SCHEME.equalsIgnoreCase(urlKey.scheme)) { + if (HTTPS_SCHEME.equals(urlKey.scheme)) { builder.sslConfig(DEFAULT_CLIENT_SSL_CONFIG); } @@ -318,7 +327,6 @@ protected Single request( final StreamingHttpRequester delegate, final StreamingHttpRequest request) { return defer(() -> { singleClientStrategyUpdate(request.context(), client.executionContext().executionStrategy()); - return delegate.request(request); }); } diff --git a/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/SslAndNonSslConnectionsTest.java b/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/SslAndNonSslConnectionsTest.java index 0ea603401c..ed8365e427 100644 --- a/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/SslAndNonSslConnectionsTest.java +++ b/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/SslAndNonSslConnectionsTest.java @@ -236,7 +236,7 @@ void multiAddressClientWithSslToSecureServer() throws Exception { void multiAddressClientToSecureServerThenToNonSecureServer() throws Exception { try (BlockingHttpClient client = HttpClients.forMultiAddressUrl(getClass().getSimpleName()) .initializer((scheme, address, builder) -> { - if ("https".equalsIgnoreCase(scheme)) { + if ("https".equals(scheme)) { builder.sslConfig(new ClientSslConfigBuilder(DefaultTestCerts::loadServerCAPem) .peerHost(serverPemHostname()).build()); } @@ -251,7 +251,7 @@ void multiAddressClientToSecureServerThenToNonSecureServer() throws Exception { void multiAddressClientToNonSecureServerThenToSecureServer() throws Exception { try (BlockingHttpClient client = HttpClients.forMultiAddressUrl(getClass().getSimpleName()) .initializer((scheme, address, builder) -> { - if ("https".equalsIgnoreCase(scheme)) { + if ("https".equals(scheme)) { builder.sslConfig(new ClientSslConfigBuilder(DefaultTestCerts::loadServerCAPem) .peerHost(serverPemHostname()).build()); }