diff --git a/client/src/jmh/java/org/asynchttpclient/bench/Http2HeaderConversionBenchmark.java b/client/src/jmh/java/org/asynchttpclient/bench/Http2HeaderConversionBenchmark.java new file mode 100644 index 0000000000..c8b121f933 --- /dev/null +++ b/client/src/jmh/java/org/asynchttpclient/bench/Http2HeaderConversionBenchmark.java @@ -0,0 +1,126 @@ +/* + * Copyright (c) 2026 AsyncHttpClient Project. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.asynchttpclient.bench; + +import io.netty.handler.codec.http.DefaultHttpHeaders; +import io.netty.handler.codec.http.HttpHeaderNames; +import io.netty.handler.codec.http.HttpHeaders; +import io.netty.handler.codec.http2.DefaultHttp2Headers; +import io.netty.handler.codec.http2.Http2Headers; +import io.netty.util.AsciiString; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Iterator; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.TimeUnit; + +/** + * Measures the per-request HTTP/2 header copy in {@code NettyRequestSender.sendHttp2Frames}: the + * baseline (String-typed {@code forEach} + {@code toLowerCase()} + {@code HashSet} skip-set) versus + * {@link HttpHeaders#iteratorCharSequence()} + {@link AsciiString#contentEqualsIgnoreCase}. Both still + * lowercase forwarded names (Netty's validating {@link DefaultHttp2Headers} rejects uppercase), so the + * header set includes mixed-case names to exercise that path. + */ +@State(Scope.Thread) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +public class Http2HeaderConversionBenchmark { + + private static final Set EXCLUDED_STRING = new HashSet<>(Arrays.asList( + "connection", "keep-alive", "proxy-connection", "transfer-encoding", "upgrade", "host")); + + private HttpHeaders headers; + + @Setup + public void setup() { + // Representative request header set built the way NettyRequestFactory builds it: + // names are AsciiString constants from HttpHeaderNames. + headers = new DefaultHttpHeaders(false); + headers.set(HttpHeaderNames.HOST, "www.example.com"); + headers.set(HttpHeaderNames.USER_AGENT, "AHC/3.0"); + headers.set(HttpHeaderNames.ACCEPT, "*/*"); + headers.set(HttpHeaderNames.ACCEPT_ENCODING, "gzip, deflate"); + headers.set(HttpHeaderNames.CONTENT_TYPE, "application/json; charset=utf-8"); + headers.set(HttpHeaderNames.CONTENT_LENGTH, "256"); + headers.set(HttpHeaderNames.AUTHORIZATION, "Bearer abcdef0123456789"); + headers.set(HttpHeaderNames.COOKIE, "session=deadbeef; theme=dark"); + headers.set(HttpHeaderNames.CONNECTION, "keep-alive"); + headers.set(HttpHeaderNames.CACHE_CONTROL, "no-cache"); + // Mixed-case user-supplied names (stored as String, original casing preserved) — these are the + // names the proposed path must lowercase, since validating DefaultHttp2Headers rejects uppercase. + headers.add("X-Request-ID", "abc-123-def-456"); + headers.add("X-Custom-Header", "some-custom-value"); + } + + /** Exact reproduction of production NettyRequestSender.sendHttp2Frames header loop. */ + @Benchmark + public Http2Headers baseline_forEach_toLowerCase() { + Http2Headers h2 = new DefaultHttp2Headers() + .method("GET").path("/path?q=1").scheme("https").authority("www.example.com"); + for (Map.Entry entry : headers) { + String name = entry.getKey().toLowerCase(); + if (!EXCLUDED_STRING.contains(name)) { + h2.add(name, entry.getValue()); + } + } + return h2; + } + + /** Proposed: iteratorCharSequence + AsciiString-keyed case-insensitive skip set. */ + @Benchmark + public Http2Headers proposed_charSequence_ascii() { + Http2Headers h2 = new DefaultHttp2Headers() + .method("GET").path("/path?q=1").scheme("https").authority("www.example.com"); + Iterator> it = headers.iteratorCharSequence(); + while (it.hasNext()) { + Map.Entry entry = it.next(); + CharSequence name = entry.getKey(); + if (!containsIgnoreCase(name)) { + h2.add(toLowerCaseName(name), entry.getValue()); + } + } + return h2; + } + + /** Mirrors production NettyRequestSender.toLowerCaseHeaderName — allocation-free when already lowercase. */ + private static CharSequence toLowerCaseName(CharSequence name) { + if (name instanceof AsciiString) { + return ((AsciiString) name).toLowerCase(); + } + return name.toString().toLowerCase(Locale.ROOT); + } + + private static boolean containsIgnoreCase(CharSequence name) { + // Direct constant comparison: HTTP/2 forbids exactly these 6 connection-specific names. + // AsciiString.contentEqualsIgnoreCase short-circuits on length mismatch (cheap). + return HttpHeaderNames.CONNECTION.contentEqualsIgnoreCase(name) + || HttpHeaderNames.HOST.contentEqualsIgnoreCase(name) + || HttpHeaderNames.TRANSFER_ENCODING.contentEqualsIgnoreCase(name) + || HttpHeaderNames.UPGRADE.contentEqualsIgnoreCase(name) + || HttpHeaderNames.KEEP_ALIVE.contentEqualsIgnoreCase(name) + || HttpHeaderNames.PROXY_CONNECTION.contentEqualsIgnoreCase(name); + } +} diff --git a/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java b/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java index 73679fd17e..05e33d0646 100755 --- a/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java +++ b/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java @@ -24,6 +24,7 @@ import io.netty.channel.ChannelPromise; import io.netty.handler.codec.http.DefaultFullHttpRequest; import io.netty.handler.codec.http.DefaultHttpHeaders; +import io.netty.handler.codec.http.HttpHeaderNames; import io.netty.handler.codec.http.HttpHeaderValues; import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpMethod; @@ -36,6 +37,7 @@ import io.netty.handler.codec.http2.Http2StreamChannelBootstrap; import io.netty.resolver.AddressResolver; import io.netty.resolver.AddressResolverGroup; +import io.netty.util.AsciiString; import io.netty.util.ReferenceCountUtil; import io.netty.util.Timer; import io.netty.util.concurrent.Future; @@ -81,13 +83,14 @@ import java.io.IOException; import java.net.InetSocketAddress; import java.net.SocketAddress; +import java.util.Iterator; import java.util.List; -import java.util.Set; +import java.util.Locale; +import java.util.Map; import static io.netty.handler.codec.http.HttpHeaderNames.EXPECT; import static java.util.Collections.singletonList; import static java.util.Objects.requireNonNull; -import static java.util.Set.of; import static org.asynchttpclient.util.AuthenticatorUtils.perConnectionAuthorizationHeader; import static org.asynchttpclient.util.AuthenticatorUtils.perConnectionProxyAuthorizationHeader; import static org.asynchttpclient.util.HttpConstants.Methods.CONNECT; @@ -420,12 +423,33 @@ private NettyResponseFuture newNettyResponseFuture(Request request, Async } /** - * HTTP/2 connection-specific headers that must NOT be forwarded as per RFC 7540 §8.1.2.2. - * These are HTTP/1.1 connection-specific headers that have no meaning in HTTP/2. + * Whether {@code name} is a connection-specific header forbidden in HTTP/2 (RFC 7540 §8.1.2.2). + * Matched case-insensitively against the {@link HttpHeaderNames} {@link AsciiString} constants, so the + * per-request header copy needs no {@link String}/{@code toLowerCase} allocation to run this check. */ - private static final Set HTTP2_EXCLUDED_HEADERS = of( - "connection", "keep-alive", "proxy-connection", "transfer-encoding", "upgrade", "host" - ); + private static boolean isHttp2ExcludedHeader(CharSequence name) { + return HttpHeaderNames.CONNECTION.contentEqualsIgnoreCase(name) + || HttpHeaderNames.HOST.contentEqualsIgnoreCase(name) + || HttpHeaderNames.TRANSFER_ENCODING.contentEqualsIgnoreCase(name) + || HttpHeaderNames.UPGRADE.contentEqualsIgnoreCase(name) + || HttpHeaderNames.KEEP_ALIVE.contentEqualsIgnoreCase(name) + || HttpHeaderNames.PROXY_CONNECTION.contentEqualsIgnoreCase(name); + } + + /** + * Lower-cases an HTTP/1.1 header name for HTTP/2, allocating nothing when it is already lowercase. + * Netty's validating {@link DefaultHttp2Headers} throws {@link io.netty.handler.codec.http2.Http2Exception} + * on a name with any uppercase ASCII letter (it does not normalise), so mixed-case user names must be + * lowercased before they are added. {@link AsciiString#toLowerCase()} and {@link String#toLowerCase(Locale)} + * both return the same instance when nothing changes, so already-lowercase names — AHC's own + * {@link HttpHeaderNames} constants — allocate nothing. + */ + private static CharSequence toLowerCaseHeaderName(CharSequence name) { + if (name instanceof AsciiString) { + return ((AsciiString) name).toLowerCase(); + } + return name.toString().toLowerCase(Locale.ROOT); + } public void writeRequest(NettyResponseFuture future, Channel channel) { // if the channel is dead because it was pooled and the remote server decided to close it, @@ -575,21 +599,25 @@ private void sendHttp2Frames(NettyResponseFuture future, Http2StreamChann Uri uri = future.getUri(); try { - // Build HTTP/2 pseudo-headers + regular headers + // Build HTTP/2 pseudo-headers + regular headers. :path reuses Uri.toRelativeUrl() (pooled + // StringBuilder) instead of re-concatenating path + "?" + query on every request. Http2Headers h2Headers = new DefaultHttp2Headers() .method(httpRequest.method().name()) - .path(uri.getNonEmptyPath() + (uri.getQuery() != null ? "?" + uri.getQuery() : "")) + .path(uri.toRelativeUrl()) .scheme(uri.getScheme()) .authority(hostHeader(uri)); - // Copy HTTP/1.1 headers, skipping connection-specific ones that are forbidden in HTTP/2. - // RFC 7540 §8.1.2 requires all header field names to be lowercase in HTTP/2. - httpRequest.headers().forEach(entry -> { - String name = entry.getKey().toLowerCase(); - if (!HTTP2_EXCLUDED_HEADERS.contains(name)) { - h2Headers.add(name, entry.getValue()); + // Copy the HTTP/1.1 headers, dropping connection-specific names forbidden in HTTP/2 (RFC 7540 + // §8.1.2.2). iteratorCharSequence() avoids the per-name String the String-typed iterator forces; + // see isHttp2ExcludedHeader and toLowerCaseHeaderName for the skip-check and lowercasing rules. + Iterator> it = httpRequest.headers().iteratorCharSequence(); + while (it.hasNext()) { + Map.Entry entry = it.next(); + CharSequence name = entry.getKey(); + if (!isHttp2ExcludedHeader(name)) { + h2Headers.add(toLowerCaseHeaderName(name), entry.getValue()); } - }); + } // Determine if we have a body to write. // Support both DefaultFullHttpRequest (inline content) and NettyDirectBody (byte array/buffer bodies). diff --git a/client/src/test/java/org/asynchttpclient/BasicHttp2Test.java b/client/src/test/java/org/asynchttpclient/BasicHttp2Test.java index c6fa880b1f..aeb5044f45 100644 --- a/client/src/test/java/org/asynchttpclient/BasicHttp2Test.java +++ b/client/src/test/java/org/asynchttpclient/BasicHttp2Test.java @@ -262,14 +262,21 @@ private void sendEchoResponse(ChannelHandlerContext ctx, ByteBuf body, String fu responseHeaders.add("x-querystring", queryString); } - // Echo request headers as X-{name} + // Echo request headers as X-{name}, and also report the exact (case-preserving) set of + // received non-pseudo header names in a single value so wire casing / exclusion is testable. + StringBuilder receivedNames = new StringBuilder(); for (Map.Entry entry : requestHeaders) { String name = entry.getKey().toString(); // Skip pseudo-headers if (!name.startsWith(":")) { responseHeaders.add("x-" + name, entry.getValue()); + if (receivedNames.length() > 0) { + receivedNames.append(','); + } + receivedNames.append(name); } } + responseHeaders.add("x-received-names", receivedNames.toString()); // Handle OPTIONS if ("OPTIONS".equalsIgnoreCase(method)) { @@ -768,6 +775,51 @@ public void getWithHeadersOverHttp2() throws Exception { } } + /** + * Regression guard for the HTTP/2 header copy: a user-supplied mixed-case header name must be + * lowercased before it reaches the validating {@link DefaultHttp2Headers}, which otherwise throws + * {@code Http2Exception: invalid header name}. Asserts (a) the request succeeds, (b) the name is + * lowercase on the wire, and (c) connection-specific names are not forwarded. + */ + @Test + public void mixedCaseHeaderIsLowercasedAndConnectionHeadersExcludedOverHttp2() throws Exception { + try (AsyncHttpClient client = http2Client()) { + Response response = client.prepareGet(httpsUrl("/echo")) + .addHeader("X-Mixed-Case", "v1") + .addHeader("Another-Custom-Header", "v2") + .addHeader("connection", "keep-alive") + .addHeader("Keep-Alive", "timeout=5") + .addHeader("Upgrade", "h2c") + .execute() + .get(30, SECONDS); + + assertEquals(200, response.getStatusCode()); + + // x-received-names reports the exact, case-preserving names the server decoded off the wire. + String received = response.getHeader("x-received-names"); + assertNotNull(received, "server should report received header names"); + List names = Arrays.asList(received.split(",")); + + // (b) mixed-case user headers arrive lowercase on the wire (case-sensitive membership check) + assertTrue(names.contains("x-mixed-case"), + "mixed-case header should be lowercase on the wire, got: " + received); + assertTrue(names.contains("another-custom-header"), + "mixed-case header should be lowercase on the wire, got: " + received); + assertFalse(names.contains("X-Mixed-Case"), + "uppercase header name must not appear on the wire, got: " + received); + + // (c) connection-specific names (any casing) are dropped, not forwarded as regular headers + for (String forbidden : new String[]{"connection", "keep-alive", "upgrade", "host"}) { + assertFalse(names.contains(forbidden), + "connection-specific header '" + forbidden + "' must be excluded, got: " + received); + } + + // and the values still round-trip for the forwarded headers + assertEquals("v1", response.getHeader("X-x-mixed-case")); + assertEquals("v2", response.getHeader("X-another-custom-header")); + } + } + @Test public void postWithHeadersAndFormParamsOverHttp2() throws Exception { try (AsyncHttpClient client = http2Client()) { diff --git a/client/src/test/java/org/asynchttpclient/uri/UriTest.java b/client/src/test/java/org/asynchttpclient/uri/UriTest.java index f766854e13..288594f69b 100644 --- a/client/src/test/java/org/asynchttpclient/uri/UriTest.java +++ b/client/src/test/java/org/asynchttpclient/uri/UriTest.java @@ -352,4 +352,26 @@ public void testGetPathWhenPathIsEmpty() { Uri uri = Uri.create("http://stackoverflow.com"); assertEquals("/", uri.getNonEmptyPath(), "Incorrect path returned from getNonEmptyPath"); } + + /** + * The HTTP/2 writer builds the {@code :path} pseudo-header via {@link Uri#toRelativeUrl()} instead of + * the older {@code getNonEmptyPath() + (query != null ? "?" + query : "")} concatenation; this locks in + * that the two are byte-identical for representative origin-form request targets (no wire change). + */ + @RepeatedIfExceptionsTest(repeats = 5) + public void testToRelativeUrlMatchesLegacyPathConcat() { + for (String url : new String[]{ + "http://example.com", // empty path + "http://example.com/", // root path + "http://example.com/a/b", // path, no query + "http://example.com/a/b?x=1&y=2", // path + query + "http://example.com/?q=1", // root path + query + "http://example.com/search?q=a%20b&n=1" // encoded query + }) { + Uri uri = Uri.create(url); + String legacy = uri.getNonEmptyPath() + (uri.getQuery() != null ? "?" + uri.getQuery() : ""); + assertEquals(legacy, uri.toRelativeUrl(), + "toRelativeUrl() must equal the legacy :path concatenation for " + url); + } + } }