From 4eb6a6e671eb18278d0f66a5b0a9009838fa4a1f Mon Sep 17 00:00:00 2001 From: Pavel Ptashyts <49400901+pavel-ptashyts@users.noreply.github.com> Date: Mon, 20 Jul 2026 19:25:21 +0200 Subject: [PATCH 1/3] Avoid redundant HTTP/2 name validation HTTP/2 decoding already validates response header names. Rechecking them while converting to HttpHeaders repeats per-character work for every response and trailer. Use a factory that disables only name validation while preserving value validation, including CR/LF rejection. Cover both properties with focused tests. Codex on behalf of Pavel Ptashyts Co-Authored-By: Codex --- .../netty/handler/Http2Handler.java | 35 +++++++------- .../netty/handler/Http2HandlerTest.java | 48 +++++++++++++++++++ 2 files changed, 67 insertions(+), 16 deletions(-) create mode 100644 client/src/test/java/org/asynchttpclient/netty/handler/Http2HandlerTest.java diff --git a/client/src/main/java/org/asynchttpclient/netty/handler/Http2Handler.java b/client/src/main/java/org/asynchttpclient/netty/handler/Http2Handler.java index b83634b68..9a743159c 100644 --- a/client/src/main/java/org/asynchttpclient/netty/handler/Http2Handler.java +++ b/client/src/main/java/org/asynchttpclient/netty/handler/Http2Handler.java @@ -19,9 +19,10 @@ import io.netty.channel.Channel; import io.netty.channel.ChannelHandler.Sharable; import io.netty.channel.ChannelHandlerContext; -import io.netty.handler.codec.http.DefaultHttpHeaders; +import io.netty.handler.codec.http.DefaultHttpHeadersFactory; import io.netty.handler.codec.http.DefaultHttpResponse; import io.netty.handler.codec.http.HttpHeaders; +import io.netty.handler.codec.http.HttpHeadersFactory; import io.netty.handler.codec.http.HttpResponse; import io.netty.handler.codec.http.HttpResponseStatus; import io.netty.handler.codec.http.HttpVersion; @@ -56,6 +57,8 @@ public final class Http2Handler extends AsyncHttpClientHandler { private static final HttpVersion HTTP_2 = new HttpVersion("HTTP", 2, 0, true); + private static final HttpHeadersFactory RESPONSE_HEADERS_FACTORY = + DefaultHttpHeadersFactory.headersFactory().withNameValidation(false); public Http2Handler(AsyncHttpClientConfig config, ChannelManager channelManager, NettyRequestSender requestSender) { super(config, channelManager, requestSender); @@ -146,14 +149,7 @@ private void handleHttp2HeadersFrame(Http2HeadersFrame headersFrame, Channel cha } HttpResponseStatus nettyStatus = HttpResponseStatus.valueOf(statusCode); - // Build HTTP/1.1-style headers, skipping HTTP/2 pseudo-headers (start with ':') - HttpHeaders responseHeaders = new DefaultHttpHeaders(); - h2Headers.forEach(entry -> { - CharSequence name = entry.getKey(); - if (name.length() > 0 && name.charAt(0) != ':') { - responseHeaders.add(name, entry.getValue()); - } - }); + HttpHeaders responseHeaders = copyHttp2Headers(h2Headers); // Build a synthetic HttpResponse so the existing interceptor chain can be reused unchanged HttpResponse syntheticResponse = new DefaultHttpResponse(HTTP_2, nettyStatus, responseHeaders); @@ -222,13 +218,7 @@ private void handleHttp2TrailingHeadersFrame(Http2HeadersFrame headersFrame, Cha NettyResponseFuture future, AsyncHandler handler) throws Exception { Http2Headers h2Headers = headersFrame.headers(); - HttpHeaders trailingHeaders = new DefaultHttpHeaders(); - h2Headers.forEach(entry -> { - CharSequence name = entry.getKey(); - if (name.length() > 0 && name.charAt(0) != ':') { - trailingHeaders.add(name, entry.getValue()); - } - }); + HttpHeaders trailingHeaders = copyHttp2Headers(h2Headers); boolean abort = false; if (!trailingHeaders.isEmpty()) { @@ -240,6 +230,19 @@ private void handleHttp2TrailingHeadersFrame(Http2HeadersFrame headersFrame, Cha } } + static HttpHeaders copyHttp2Headers(Http2Headers h2Headers) { + // The HTTP/2 decoder validates names but not values by default. Avoid repeating the name scan while + // retaining the HTTP/1 value validator that rejects CR/LF and other prohibited characters. + HttpHeaders headers = RESPONSE_HEADERS_FACTORY.newHeaders(); + h2Headers.forEach(entry -> { + CharSequence name = entry.getKey(); + if (name.length() > 0 && name.charAt(0) != ':') { + headers.add(name, entry.getValue()); + } + }); + return headers; + } + /** * Processes an HTTP/2 RST_STREAM frame, which indicates the server aborted the stream. */ diff --git a/client/src/test/java/org/asynchttpclient/netty/handler/Http2HandlerTest.java b/client/src/test/java/org/asynchttpclient/netty/handler/Http2HandlerTest.java new file mode 100644 index 000000000..9743d209a --- /dev/null +++ b/client/src/test/java/org/asynchttpclient/netty/handler/Http2HandlerTest.java @@ -0,0 +1,48 @@ +/* + * 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.netty.handler; + +import io.netty.handler.codec.http.HttpHeaders; +import io.netty.handler.codec.http2.DefaultHttp2Headers; +import io.netty.handler.codec.http2.Http2Headers; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class Http2HandlerTest { + + @Test + void copiesDecoderValidatedHeaderNamesWithoutRevalidation() { + Http2Headers source = new DefaultHttp2Headers(false) + .status("200") + .add("invalid name", "value"); + + HttpHeaders copy = Http2Handler.copyHttp2Headers(source); + + assertEquals("value", copy.get("invalid name")); + assertFalse(copy.contains(":status")); + } + + @Test + void stillValidatesHeaderValues() { + Http2Headers source = new DefaultHttp2Headers(false) + .add("x-test", "value\r\ninjected"); + + assertThrows(IllegalArgumentException.class, () -> Http2Handler.copyHttp2Headers(source)); + } +} From 0411239fe748fcf99de22b64438f9abcfbac419b Mon Sep 17 00:00:00 2001 From: Pavel Ptashyts <49400901+pavel-ptashyts@users.noreply.github.com> Date: Sat, 25 Jul 2026 13:12:04 +0200 Subject: [PATCH 2/3] Validate converted HTTP/2 headers Restore complete HTTP token and value validation when converting decoded HTTP/2 headers. Use Netty's trailer-specific factory so prohibited trailing fields cannot reach handlers. Replace capturing copy lambdas with enhanced loops and cover malformed names, values, empty names, and prohibited trailers. Codex on behalf of Pavel Ptashyts Co-Authored-By: Codex --- .../netty/handler/Http2Handler.java | 24 ++++++++---- .../netty/handler/Http2HandlerTest.java | 39 +++++++++++++++++-- 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/client/src/main/java/org/asynchttpclient/netty/handler/Http2Handler.java b/client/src/main/java/org/asynchttpclient/netty/handler/Http2Handler.java index 9a743159c..8886ae0ae 100644 --- a/client/src/main/java/org/asynchttpclient/netty/handler/Http2Handler.java +++ b/client/src/main/java/org/asynchttpclient/netty/handler/Http2Handler.java @@ -42,6 +42,7 @@ import org.asynchttpclient.netty.request.NettyRequestSender; import java.io.IOException; +import java.util.Map; /** * HTTP/2 channel handler for stream child channels created by {@link io.netty.handler.codec.http2.Http2MultiplexHandler}. @@ -57,8 +58,8 @@ public final class Http2Handler extends AsyncHttpClientHandler { private static final HttpVersion HTTP_2 = new HttpVersion("HTTP", 2, 0, true); - private static final HttpHeadersFactory RESPONSE_HEADERS_FACTORY = - DefaultHttpHeadersFactory.headersFactory().withNameValidation(false); + private static final HttpHeadersFactory RESPONSE_HEADERS_FACTORY = DefaultHttpHeadersFactory.headersFactory(); + private static final HttpHeadersFactory TRAILING_HEADERS_FACTORY = DefaultHttpHeadersFactory.trailersFactory(); public Http2Handler(AsyncHttpClientConfig config, ChannelManager channelManager, NettyRequestSender requestSender) { super(config, channelManager, requestSender); @@ -218,7 +219,7 @@ private void handleHttp2TrailingHeadersFrame(Http2HeadersFrame headersFrame, Cha NettyResponseFuture future, AsyncHandler handler) throws Exception { Http2Headers h2Headers = headersFrame.headers(); - HttpHeaders trailingHeaders = copyHttp2Headers(h2Headers); + HttpHeaders trailingHeaders = copyHttp2Trailers(h2Headers); boolean abort = false; if (!trailingHeaders.isEmpty()) { @@ -231,15 +232,22 @@ private void handleHttp2TrailingHeadersFrame(Http2HeadersFrame headersFrame, Cha } static HttpHeaders copyHttp2Headers(Http2Headers h2Headers) { - // The HTTP/2 decoder validates names but not values by default. Avoid repeating the name scan while - // retaining the HTTP/1 value validator that rejects CR/LF and other prohibited characters. - HttpHeaders headers = RESPONSE_HEADERS_FACTORY.newHeaders(); - h2Headers.forEach(entry -> { + return copyHttp2Headers(h2Headers, RESPONSE_HEADERS_FACTORY); + } + + static HttpHeaders copyHttp2Trailers(Http2Headers h2Headers) { + return copyHttp2Headers(h2Headers, TRAILING_HEADERS_FACTORY); + } + + private static HttpHeaders copyHttp2Headers(Http2Headers h2Headers, HttpHeadersFactory headersFactory) { + // HTTP/2 validation does not enforce HTTP token syntax or header values, so validate both while converting. + HttpHeaders headers = headersFactory.newHeaders(); + for (Map.Entry entry : h2Headers) { CharSequence name = entry.getKey(); if (name.length() > 0 && name.charAt(0) != ':') { headers.add(name, entry.getValue()); } - }); + } return headers; } diff --git a/client/src/test/java/org/asynchttpclient/netty/handler/Http2HandlerTest.java b/client/src/test/java/org/asynchttpclient/netty/handler/Http2HandlerTest.java index 9743d209a..9b4324e64 100644 --- a/client/src/test/java/org/asynchttpclient/netty/handler/Http2HandlerTest.java +++ b/client/src/test/java/org/asynchttpclient/netty/handler/Http2HandlerTest.java @@ -20,29 +20,60 @@ import io.netty.handler.codec.http2.Http2Headers; import org.junit.jupiter.api.Test; +import java.util.Arrays; + +import static io.netty.handler.codec.http.HttpHeaderNames.CONTENT_LENGTH; +import static io.netty.handler.codec.http.HttpHeaderNames.TRAILER; +import static io.netty.handler.codec.http.HttpHeaderNames.TRANSFER_ENCODING; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; class Http2HandlerTest { @Test - void copiesDecoderValidatedHeaderNamesWithoutRevalidation() { + void copiesRegularHeadersAndSkipsPseudoHeaders() { Http2Headers source = new DefaultHttp2Headers(false) .status("200") - .add("invalid name", "value"); + .add("x-test", "value"); HttpHeaders copy = Http2Handler.copyHttp2Headers(source); - assertEquals("value", copy.get("invalid name")); + assertEquals("value", copy.get("x-test")); assertFalse(copy.contains(":status")); } @Test - void stillValidatesHeaderValues() { + void rejectsInvalidHeaderNames() { + Http2Headers source = new DefaultHttp2Headers(false) + .add("invalid\r\nname", "value"); + + assertThrows(IllegalArgumentException.class, () -> Http2Handler.copyHttp2Headers(source)); + } + + @Test + void skipsEmptyHeaderNames() { + Http2Headers source = new DefaultHttp2Headers(false) + .add("", "value"); + + assertTrue(Http2Handler.copyHttp2Headers(source).isEmpty()); + } + + @Test + void rejectsInvalidHeaderValues() { Http2Headers source = new DefaultHttp2Headers(false) .add("x-test", "value\r\ninjected"); assertThrows(IllegalArgumentException.class, () -> Http2Handler.copyHttp2Headers(source)); } + + @Test + void rejectsProhibitedTrailingHeaders() { + for (CharSequence name : Arrays.asList(CONTENT_LENGTH, TRANSFER_ENCODING, TRAILER)) { + Http2Headers source = new DefaultHttp2Headers(false).add(name, "value"); + + assertThrows(IllegalArgumentException.class, () -> Http2Handler.copyHttp2Trailers(source), name.toString()); + } + } } From 505470acfa772bcdbe5a07ae39e33ace705490e4 Mon Sep 17 00:00:00 2001 From: Pavel Ptashyts <49400901+pavel-ptashyts@users.noreply.github.com> Date: Sat, 25 Jul 2026 20:10:36 +0200 Subject: [PATCH 3/3] Drop prohibited HTTP/2 trailers Preserve valid trailing headers when a peer sends fields that are prohibited in trailers. Failing the stream at that point discards the trailer section after the response body has already been delivered. Keep full header name and value validation for both response headers and trailers, and cover the distinct conversion rules. Codex on behalf of Pavel Ptashyts Co-Authored-By: Codex --- .../netty/handler/Http2Handler.java | 27 +++++++++----- .../netty/handler/Http2HandlerTest.java | 36 +++++++++++++------ 2 files changed, 45 insertions(+), 18 deletions(-) diff --git a/client/src/main/java/org/asynchttpclient/netty/handler/Http2Handler.java b/client/src/main/java/org/asynchttpclient/netty/handler/Http2Handler.java index 8886ae0ae..7c581acbd 100644 --- a/client/src/main/java/org/asynchttpclient/netty/handler/Http2Handler.java +++ b/client/src/main/java/org/asynchttpclient/netty/handler/Http2Handler.java @@ -21,6 +21,7 @@ import io.netty.channel.ChannelHandlerContext; import io.netty.handler.codec.http.DefaultHttpHeadersFactory; import io.netty.handler.codec.http.DefaultHttpResponse; +import io.netty.handler.codec.http.HttpHeaderNames; import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpHeadersFactory; import io.netty.handler.codec.http.HttpResponse; @@ -58,8 +59,7 @@ public final class Http2Handler extends AsyncHttpClientHandler { private static final HttpVersion HTTP_2 = new HttpVersion("HTTP", 2, 0, true); - private static final HttpHeadersFactory RESPONSE_HEADERS_FACTORY = DefaultHttpHeadersFactory.headersFactory(); - private static final HttpHeadersFactory TRAILING_HEADERS_FACTORY = DefaultHttpHeadersFactory.trailersFactory(); + private static final HttpHeadersFactory HEADERS_FACTORY = DefaultHttpHeadersFactory.headersFactory(); public Http2Handler(AsyncHttpClientConfig config, ChannelManager channelManager, NettyRequestSender requestSender) { super(config, channelManager, requestSender); @@ -232,25 +232,36 @@ private void handleHttp2TrailingHeadersFrame(Http2HeadersFrame headersFrame, Cha } static HttpHeaders copyHttp2Headers(Http2Headers h2Headers) { - return copyHttp2Headers(h2Headers, RESPONSE_HEADERS_FACTORY); + return copyHttp2Headers(h2Headers, false); } static HttpHeaders copyHttp2Trailers(Http2Headers h2Headers) { - return copyHttp2Headers(h2Headers, TRAILING_HEADERS_FACTORY); + return copyHttp2Headers(h2Headers, true); } - private static HttpHeaders copyHttp2Headers(Http2Headers h2Headers, HttpHeadersFactory headersFactory) { + private static HttpHeaders copyHttp2Headers(Http2Headers h2Headers, boolean trailers) { // HTTP/2 validation does not enforce HTTP token syntax or header values, so validate both while converting. - HttpHeaders headers = headersFactory.newHeaders(); + HttpHeaders headers = HEADERS_FACTORY.newHeaders(); for (Map.Entry entry : h2Headers) { CharSequence name = entry.getKey(); - if (name.length() > 0 && name.charAt(0) != ':') { - headers.add(name, entry.getValue()); + if (name.length() == 0 || name.charAt(0) == ':') { + continue; } + // The restriction is on the sender; drop prohibited trailers instead of failing after delivering the body. + if (trailers && !isPermittedTrailingHeader(name)) { + continue; + } + headers.add(name, entry.getValue()); } return headers; } + private static boolean isPermittedTrailingHeader(CharSequence name) { + return !HttpHeaderNames.CONTENT_LENGTH.contentEqualsIgnoreCase(name) + && !HttpHeaderNames.TRANSFER_ENCODING.contentEqualsIgnoreCase(name) + && !HttpHeaderNames.TRAILER.contentEqualsIgnoreCase(name); + } + /** * Processes an HTTP/2 RST_STREAM frame, which indicates the server aborted the stream. */ diff --git a/client/src/test/java/org/asynchttpclient/netty/handler/Http2HandlerTest.java b/client/src/test/java/org/asynchttpclient/netty/handler/Http2HandlerTest.java index 9b4324e64..45c04fbe9 100644 --- a/client/src/test/java/org/asynchttpclient/netty/handler/Http2HandlerTest.java +++ b/client/src/test/java/org/asynchttpclient/netty/handler/Http2HandlerTest.java @@ -20,11 +20,8 @@ import io.netty.handler.codec.http2.Http2Headers; import org.junit.jupiter.api.Test; -import java.util.Arrays; - import static io.netty.handler.codec.http.HttpHeaderNames.CONTENT_LENGTH; import static io.netty.handler.codec.http.HttpHeaderNames.TRAILER; -import static io.netty.handler.codec.http.HttpHeaderNames.TRANSFER_ENCODING; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -36,20 +33,31 @@ class Http2HandlerTest { void copiesRegularHeadersAndSkipsPseudoHeaders() { Http2Headers source = new DefaultHttp2Headers(false) .status("200") + .add(CONTENT_LENGTH, "5") .add("x-test", "value"); HttpHeaders copy = Http2Handler.copyHttp2Headers(source); + assertEquals("5", copy.get(CONTENT_LENGTH)); assertEquals("value", copy.get("x-test")); assertFalse(copy.contains(":status")); } @Test void rejectsInvalidHeaderNames() { - Http2Headers source = new DefaultHttp2Headers(false) + Http2Headers crlf = new DefaultHttp2Headers(false) .add("invalid\r\nname", "value"); + Http2Headers space = new DefaultHttp2Headers(false) + .add("invalid name", "value"); + Http2Headers parenthesis = new DefaultHttp2Headers(false) + .add("invalid(name", "value"); - assertThrows(IllegalArgumentException.class, () -> Http2Handler.copyHttp2Headers(source)); + assertThrows(IllegalArgumentException.class, () -> Http2Handler.copyHttp2Headers(crlf)); + assertThrows(IllegalArgumentException.class, () -> Http2Handler.copyHttp2Trailers(crlf)); + assertThrows(IllegalArgumentException.class, () -> Http2Handler.copyHttp2Headers(space)); + assertThrows(IllegalArgumentException.class, () -> Http2Handler.copyHttp2Trailers(space)); + assertThrows(IllegalArgumentException.class, () -> Http2Handler.copyHttp2Headers(parenthesis)); + assertThrows(IllegalArgumentException.class, () -> Http2Handler.copyHttp2Trailers(parenthesis)); } @Test @@ -66,14 +74,22 @@ void rejectsInvalidHeaderValues() { .add("x-test", "value\r\ninjected"); assertThrows(IllegalArgumentException.class, () -> Http2Handler.copyHttp2Headers(source)); + assertThrows(IllegalArgumentException.class, () -> Http2Handler.copyHttp2Trailers(source)); } @Test - void rejectsProhibitedTrailingHeaders() { - for (CharSequence name : Arrays.asList(CONTENT_LENGTH, TRANSFER_ENCODING, TRAILER)) { - Http2Headers source = new DefaultHttp2Headers(false).add(name, "value"); + void dropsProhibitedTrailingHeaders() { + Http2Headers source = new DefaultHttp2Headers(false) + .status("200") + .add("grpc-status", "0") + .add(CONTENT_LENGTH, "5") + .add(TRAILER, "x-checksum"); + + HttpHeaders copy = Http2Handler.copyHttp2Trailers(source); - assertThrows(IllegalArgumentException.class, () -> Http2Handler.copyHttp2Trailers(source), name.toString()); - } + assertEquals("0", copy.get("grpc-status")); + assertFalse(copy.contains(CONTENT_LENGTH)); + assertFalse(copy.contains(TRAILER)); + assertFalse(copy.contains(":status")); } }