Trailers must not include pseudo-header fields (#3810)

https://www.rfc-editor.org/rfc/rfc9113.html#name-http-message-framing

Signed-off-by: Violeta Georgieva <[email protected]>

Author: violetagg

reactor-netty-http/src/http3Test/java/reactor/netty/http/Http3Tests.java

@@ -964,6 +964,20 @@ void testTrailerHeadersNotSpecifiedUpfront() {
 		doTestTrailerHeaders(createClient(disposableServer.port()), "empty", "testTrailerHeadersNotSpecifiedUpfront");
 	}
 
+	@Test
+	void testTrailerHeadersPseudoHeaderNotAllowed() {
+		disposableServer =
+				createServer()
+				        .handle((req, res) ->
+				            res.header(HttpHeaderNames.TRAILER, ":protocol")
+				               .trailerHeaders(h -> h.set(":protocol", "test"))
+				               .sendString(Flux.just("testTrailerHeaders", "PseudoHeaderNotAllowed")))
+				        .bindNow();
+
+		// Trailers MUST NOT include pseudo-header fields
+		doTestTrailerHeaders(createClient(disposableServer.port()), "empty", "testTrailerHeadersPseudoHeaderNotAllowed");
+	}
+
 	private static void doTestTrailerHeaders(HttpClient client, String expectedHeaderValue, String expectedResponse) {
 		client.get()
 		      .uri("/")

reactor-netty-http/src/main/java/reactor/netty/http/server/HttpServerOperations.java

@@ -74,6 +74,7 @@
 import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder;
 import io.netty.handler.codec.http.websocketx.CloseWebSocketFrame;
 import io.netty.handler.codec.http.websocketx.WebSocketCloseStatus;
+import io.netty.handler.codec.http2.Http2Headers;
 import io.netty.handler.timeout.ReadTimeoutHandler;
 import io.netty.util.AsciiString;
 import io.netty.util.ReferenceCountUtil;
@@ -1000,6 +1001,7 @@ else if (markSentBody()) {
 	}
 
 	@Nullable
+	@SuppressWarnings("ReferenceEquality")
 	private HttpHeaders prepareTrailerHeaders() {
 		HttpHeaders trailerHeaders = null;
 		// https://datatracker.ietf.org/doc/html/rfc7230#section-4.1.2
@@ -1009,7 +1011,8 @@ private HttpHeaders prepareTrailerHeaders() {
 		// message integrity check, digital signature, or post-processing
 		// status.
 		// There is no requirement for chunked message when HTTP/2 and HTTP/3
-		if (trailerHeadersConsumer != null && (version() != HttpVersion.HTTP_1_1 || isTransferEncodingChunked(nettyResponse))) {
+		boolean isNotHttp11 = version() != HttpVersion.HTTP_1_1;
+		if (trailerHeadersConsumer != null && (isNotHttp11 || isTransferEncodingChunked(nettyResponse))) {
 			// https://datatracker.ietf.org/doc/html/rfc7230#section-4.4
 			// When a message includes a message body encoded with the chunked
 			// transfer coding and the sender desires to send metadata in the form
@@ -1018,7 +1021,7 @@ private HttpHeaders prepareTrailerHeaders() {
 			// which fields will be present in the trailers.
 			String declaredHeaderNames = responseHeaders.get(HttpHeaderNames.TRAILER);
 			if (declaredHeaderNames != null) {
-				trailerHeaders = new TrailerHeaders(declaredHeaderNames);
+				trailerHeaders = new TrailerHeaders(declaredHeaderNames, isNotHttp11);
 				try {
 					trailerHeadersConsumer.accept(trailerHeaders);
 				}
@@ -1437,8 +1440,8 @@ static final class TrailerHeaders extends DefaultHttpHeaders {
 			DISALLOWED_TRAILER_HEADER_NAMES.add("warning");
 		}
 
-		TrailerHeaders(String declaredHeaderNames) {
-			super(true, new TrailerNameValidator(filterHeaderNames(declaredHeaderNames)));
+		TrailerHeaders(String declaredHeaderNames, boolean isNotHttp11) {
+			super(true, new TrailerNameValidator(filterHeaderNames(declaredHeaderNames), isNotHttp11));
 		}
 
 		static Set<String> filterHeaderNames(String declaredHeaderNames) {
@@ -1462,9 +1465,11 @@ static final class TrailerNameValidator implements DefaultHeaders.NameValidator<
 			 * Contains the headers names specified with {@link HttpHeaderNames#TRAILER}.
 			 */
 			final Set<String> declaredHeaderNames;
+			final boolean isNotHttp11;
 
-			TrailerNameValidator(Set<String> declaredHeaderNames) {
+			TrailerNameValidator(Set<String> declaredHeaderNames, boolean isNotHttp11) {
 				this.declaredHeaderNames = declaredHeaderNames;
+				this.isNotHttp11 = isNotHttp11;
 			}
 
 			@Override
@@ -1473,6 +1478,12 @@ public void validateName(CharSequence name) {
 					throw new IllegalArgumentException("Trailer header name [" + name +
 							"] not declared with [Trailer] header, or it is not a valid trailer header name");
 				}
+				// https://www.rfc-editor.org/rfc/rfc9113.html#name-http-message-framing
+				// Trailers MUST NOT include pseudo-header fields
+				else if (isNotHttp11 && Http2Headers.PseudoHeaderName.hasPseudoHeaderFormat(name)) {
+					throw new IllegalArgumentException("Pseudo header name [" + name +
+							"] found in trailer headers, trailer headers cannot have pseudo headers");
+				}
 			}
 		}
 	}

reactor-netty-http/src/test/java/reactor/netty/http/Http2Tests.java

@@ -16,6 +16,7 @@
 package reactor.netty.http;
 
 import io.netty.buffer.Unpooled;
+import io.netty.handler.codec.http.HttpHeaderNames;
 import io.netty.handler.codec.http2.Http2Connection;
 import io.netty.handler.codec.http2.Http2FrameCodec;
 import io.netty.handler.ssl.util.InsecureTrustManagerFactory;
@@ -730,6 +731,46 @@ void h2ClientSendsError(HttpProtocol[] serverProtocols, HttpProtocol[] clientPro
 		}
 	}
 
+	@ParameterizedTest
+	@MethodSource("h2CompatibleCombinations")
+	@SuppressWarnings("deprecation")
+	void testTrailerHeadersPseudoHeaderNotAllowedH2(HttpProtocol[] serverProtocols, HttpProtocol[] clientProtocols) {
+		Http2SslContextSpec serverCtx = Http2SslContextSpec.forServer(ssc.certificate(), ssc.privateKey());
+		Http2SslContextSpec clientCtx =
+				Http2SslContextSpec.forClient()
+				                   .configure(builder -> builder.trustManager(InsecureTrustManagerFactory.INSTANCE));
+		testTrailerHeadersPseudoHeaderNotAllowed(
+				createServer().protocol(serverProtocols).secure(spec -> spec.sslContext(serverCtx)),
+				createClient(() -> disposableServer.address()).protocol(clientProtocols).secure(spec -> spec.sslContext(clientCtx)));
+	}
+
+	@ParameterizedTest
+	@MethodSource("h2cCompatibleCombinations")
+	void testTrailerHeadersPseudoHeaderNotAllowedH2C(HttpProtocol[] serverProtocols, HttpProtocol[] clientProtocols) {
+		testTrailerHeadersPseudoHeaderNotAllowed(
+				createServer().protocol(serverProtocols),
+				createClient(() -> disposableServer.address()).protocol(clientProtocols));
+	}
+
+	private void testTrailerHeadersPseudoHeaderNotAllowed(HttpServer server, HttpClient client) {
+		disposableServer =
+				server.handle((req, res) ->
+				          res.header(HttpHeaderNames.TRAILER, ":protocol")
+				             .trailerHeaders(h -> h.set(":protocol", "test"))
+				             .sendString(Flux.just("testTrailerHeaders", "PseudoHeaderNotAllowed")))
+				      .bindNow();
+
+		// Trailers MUST NOT include pseudo-header fields
+		client.get()
+		      .uri("/")
+		      .responseSingle((res, bytes) -> bytes.asString().defaultIfEmpty("empty").zipWith(res.trailerHeaders()))
+		      .as(StepVerifier::create)
+		      .expectNextMatches(t -> "testTrailerHeadersPseudoHeaderNotAllowed".equals(t.getT1()) &&
+		              "empty".equals(t.getT2().get(":protocol", "empty")))
+		      .expectComplete()
+		      .verify(Duration.ofSeconds(5));
+	}
+
 	private void http2ClientSendsError(HttpServer server, HttpClient client) {
 		disposableServer =
 				server.http2Settings(spec -> spec.maxConcurrentStreams(1))

reactor-netty-http/src/test/java/reactor/netty/http/server/TrailerHeadersTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021-2024 VMware, Inc. or its affiliates, All Rights Reserved.
+ * Copyright (c) 2021-2025 VMware, Inc. or its affiliates, 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.
@@ -40,13 +40,14 @@ class TrailerHeadersTests {
 	static final String HEADER_NAME_1 = "foo";
 	static final String HEADER_NAME_2 = "bar";
 	static final String HEADER_VALUE = "test";
+	static final String PSEUDO_HEADER_NAME = ":protocol";
 	static final String SPACE = " ";
 
 	@ParameterizedTest
 	@MethodSource("disallowedTrailerHeaderNames")
 	void testDisallowedTrailerHeaderNames(String declaredHeaderName) {
 		assertThatExceptionOfType(IllegalArgumentException.class)
-				.isThrownBy(() -> new HttpServerOperations.TrailerHeaders(declaredHeaderName).add(declaredHeaderName, HEADER_VALUE))
+				.isThrownBy(() -> new HttpServerOperations.TrailerHeaders(declaredHeaderName, false).add(declaredHeaderName, HEADER_VALUE))
 				.withMessage(String.format(ERROR_MESSAGE, declaredHeaderName));
 	}
 
@@ -59,7 +60,7 @@ void testDisallowedTrailerHeaderNames(String declaredHeaderName) {
 			HEADER_NAME_1 + COMMA + SPACE
 	})
 	void testNameIncludedInTrailerHeader(String declaredHeaderNames) {
-		HttpHeaders headers = new HttpServerOperations.TrailerHeaders(declaredHeaderNames);
+		HttpHeaders headers = new HttpServerOperations.TrailerHeaders(declaredHeaderNames, false);
 		assertThat(headers.isEmpty()).isTrue();
 		headers.add(HEADER_NAME_1, HEADER_VALUE);
 		assertThat(headers.isEmpty()).isFalse();
@@ -69,7 +70,7 @@ void testNameIncludedInTrailerHeader(String declaredHeaderNames) {
 
 	@Test
 	void testNamesIncludedInTrailerHeader() {
-		HttpHeaders headers = new HttpServerOperations.TrailerHeaders(HEADER_NAME_1 + ',' + HEADER_NAME_2);
+		HttpHeaders headers = new HttpServerOperations.TrailerHeaders(HEADER_NAME_1 + ',' + HEADER_NAME_2, false);
 		assertThat(headers.isEmpty()).isTrue();
 		headers.add(HEADER_NAME_1, HEADER_VALUE);
 		headers.add(HEADER_NAME_2, HEADER_VALUE);
@@ -82,18 +83,25 @@ void testNamesIncludedInTrailerHeader() {
 	@Test
 	void testNameNotIncludedInTrailerHeader() {
 		assertThatExceptionOfType(IllegalArgumentException.class)
-				.isThrownBy(() -> new HttpServerOperations.TrailerHeaders(HEADER_NAME_1).add(HEADER_NAME_2, HEADER_VALUE))
+				.isThrownBy(() -> new HttpServerOperations.TrailerHeaders(HEADER_NAME_1, false).add(HEADER_NAME_2, HEADER_VALUE))
 				.withMessage(String.format(ERROR_MESSAGE, HEADER_NAME_2));
 	}
 
 	@ParameterizedTest
 	@ValueSource(strings = {COMMA, EMPTY, SPACE})
 	void testNothingIsIncludedInTrailerHeader(String declaredHeaderNames) {
 		assertThatExceptionOfType(IllegalArgumentException.class)
-				.isThrownBy(() -> new HttpServerOperations.TrailerHeaders(declaredHeaderNames).add(EMPTY, HEADER_VALUE))
+				.isThrownBy(() -> new HttpServerOperations.TrailerHeaders(declaredHeaderNames, false).add(EMPTY, HEADER_VALUE))
 				.withMessage(String.format(ERROR_MESSAGE, EMPTY));
 	}
 
+	@Test
+	void testPseudoHeaderInTrailerHeaderNames() {
+		assertThatExceptionOfType(IllegalArgumentException.class)
+				.isThrownBy(() -> new HttpServerOperations.TrailerHeaders(PSEUDO_HEADER_NAME, true).add(PSEUDO_HEADER_NAME, HEADER_VALUE))
+				.withMessage("Pseudo header name [:protocol] found in trailer headers, trailer headers cannot have pseudo headers");
+	}
+
 	static Set<String> disallowedTrailerHeaderNames() {
 		return HttpServerOperations.TrailerHeaders.DISALLOWED_TRAILER_HEADER_NAMES;
 	}