diff --git a/codec-http2/src/main/java/io/netty5/handler/codec/http2/HpackDecoder.java b/codec-http2/src/main/java/io/netty5/handler/codec/http2/HpackDecoder.java index 05378101537..16e76410e0b 100644 --- a/codec-http2/src/main/java/io/netty5/handler/codec/http2/HpackDecoder.java +++ b/codec-http2/src/main/java/io/netty5/handler/codec/http2/HpackDecoder.java @@ -33,6 +33,7 @@ import io.netty5.buffer.Buffer; import io.netty5.handler.codec.http.headers.HeaderValidationException; +import io.netty5.handler.codec.http.headers.HttpHeaderValidationUtil; import io.netty5.handler.codec.http2.HpackUtil.IndexType; import io.netty5.handler.codec.http2.headers.Http2Headers; import io.netty5.util.AsciiString; @@ -535,7 +536,7 @@ void appendToHeaderList(AsciiString name, AsciiString value) { try { headers.add(name, value); if (validateHeaders) { - previousType = validateHeader(streamId, name, previousType); + previousType = validateHeader(streamId, name, value, previousType); } } catch (HeaderValidationException ex) { validationException = streamError(streamId, PROTOCOL_ERROR, ex, @@ -545,8 +546,8 @@ void appendToHeaderList(AsciiString name, AsciiString value) { } } - private static HeaderType validateHeader(int streamId, AsciiString name, HeaderType previousHeaderType) - throws Http2Exception { + private static HeaderType validateHeader(int streamId, AsciiString name, AsciiString value, + HeaderType previousHeaderType) throws Http2Exception { if (hasPseudoHeaderFormat(name)) { if (previousHeaderType == HeaderType.REGULAR_HEADER) { throw streamError(streamId, PROTOCOL_ERROR, @@ -560,6 +561,14 @@ private static HeaderType validateHeader(int streamId, AsciiString name, HeaderT } return currentHeaderType; } + if (HttpHeaderValidationUtil.isConnectionHeader(name, true)) { + throw streamError(streamId, PROTOCOL_ERROR, + "Illegal connection-specific header '%s' encountered.", name); + } + if (HttpHeaderValidationUtil.isTeNotTrailers(name, value)) { + throw streamError(streamId, PROTOCOL_ERROR, + "Illegal value specified for the 'TE' header (only 'trailers' is allowed)."); + } return HeaderType.REGULAR_HEADER; } diff --git a/codec-http2/src/main/java/io/netty5/handler/codec/http2/headers/DefaultHttp2Headers.java b/codec-http2/src/main/java/io/netty5/handler/codec/http2/headers/DefaultHttp2Headers.java index 17e5d6380d2..d019ebcc837 100644 --- a/codec-http2/src/main/java/io/netty5/handler/codec/http2/headers/DefaultHttp2Headers.java +++ b/codec-http2/src/main/java/io/netty5/handler/codec/http2/headers/DefaultHttp2Headers.java @@ -18,7 +18,6 @@ import io.netty5.handler.codec.http.headers.DefaultHttpHeaders; import io.netty5.handler.codec.http.headers.HeaderValidationException; import io.netty5.handler.codec.http.headers.HttpCookiePair; -import io.netty5.handler.codec.http.headers.HttpHeaderValidationUtil; import io.netty5.handler.codec.http.headers.HttpHeaders; import io.netty5.handler.codec.http.headers.HttpSetCookie; import io.netty5.handler.codec.http.headers.MultiMap; @@ -93,24 +92,11 @@ protected CharSequence validateKey(@Nullable CharSequence name, boolean forAdd) } } } - if (HttpHeaderValidationUtil.isConnectionHeader(name, true)) { - throw new HeaderValidationException( - "Illegal connection-specific header '" + name + "' encountered."); - } } } return name; } - @Override - protected CharSequence validateValue(CharSequence key, CharSequence value) { - if (validateValues && HttpHeaderValidationUtil.isTeNotTrailers(key, value)) { - throw new HeaderValidationException( - "Illegal value specified for the 'TE' header (only 'trailers' is allowed)."); - } - return super.validateValue(key, value); - } - @Override public Http2Headers copy() { DefaultHttp2Headers copy = new DefaultHttp2Headers( diff --git a/codec-http2/src/test/java/io/netty5/handler/codec/http2/DefaultHttp2HeadersTest.java b/codec-http2/src/test/java/io/netty5/handler/codec/http2/DefaultHttp2HeadersTest.java index d9014089b6f..cab7b3382c7 100644 --- a/codec-http2/src/test/java/io/netty5/handler/codec/http2/DefaultHttp2HeadersTest.java +++ b/codec-http2/src/test/java/io/netty5/handler/codec/http2/DefaultHttp2HeadersTest.java @@ -21,6 +21,8 @@ import io.netty5.util.internal.StringUtil; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.function.Executable; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import java.util.Map.Entry; @@ -191,6 +193,15 @@ void setMustOverwritePseudoHeaders() { assertEquals(of("http"), headers.scheme()); } + @ParameterizedTest(name = "{displayName} [{index}] name={0} value={1}") + @CsvSource(value = {"upgrade,protocol1", "connection,close", "keep-alive,timeout=5", "proxy-connection,close", + "transfer-encoding,chunked", "te,something-else"}) + void possibleToAddConnectionHeaders(String name, String value) { + Http2Headers headers = newHeaders(); + headers.add(name, value); + assertTrue(headers.contains(name, value)); + } + private static void verifyAllPseudoHeadersPresent(Http2Headers headers) { for (PseudoHeaderName pseudoName : PseudoHeaderName.values()) { assertNotNull(headers.get(pseudoName.value()), () -> "did not find pseudo-header " + pseudoName); diff --git a/codec-http2/src/test/java/io/netty5/handler/codec/http2/HpackDecoderTest.java b/codec-http2/src/test/java/io/netty5/handler/codec/http2/HpackDecoderTest.java index fc2f0c3c342..4398fa3636b 100644 --- a/codec-http2/src/test/java/io/netty5/handler/codec/http2/HpackDecoderTest.java +++ b/codec-http2/src/test/java/io/netty5/handler/codec/http2/HpackDecoderTest.java @@ -38,6 +38,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.function.Executable; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import java.util.Iterator; import java.util.List; @@ -46,6 +48,7 @@ import static io.netty5.buffer.DefaultBufferAllocators.onHeapAllocator; import static io.netty5.handler.codec.http2.HpackDecoder.decodeULE128; +import static io.netty5.handler.codec.http2.Http2Error.PROTOCOL_ERROR; import static io.netty5.handler.codec.http2.Http2HeadersEncoder.NEVER_SENSITIVE; import static io.netty5.util.AsciiString.EMPTY_STRING; import static io.netty5.util.AsciiString.of; @@ -672,12 +675,44 @@ public Iterator> iterator() { final Http2Headers decoded = Http2Headers.newHeaders(); - assertThrows(Http2Exception.StreamException.class, new Executable() { + Http2Exception.StreamException e = assertThrows(Http2Exception.StreamException.class, new Executable() { @Override public void execute() throws Throwable { - hpackDecoder.decode(1, in, decoded, true); + hpackDecoder.decode(3, in, decoded, true); + } + }); + assertThat(e.streamId(), is(3)); + assertThat(e.error(), is(PROTOCOL_ERROR)); + } + } + + @ParameterizedTest(name = "{displayName} [{index}] name={0} value={1}") + @CsvSource(value = {"upgrade,protocol1", "connection,close", "keep-alive,timeout=5", "proxy-connection,close", + "transfer-encoding,chunked", "te,something-else"}) + public void receivedConnectionHeader(String name, String value) throws Exception { + try (Buffer in = onHeapAllocator().allocate(200)) { + HpackEncoder hpackEncoder = new HpackEncoder(true); + + Http2Headers toEncode = new DefaultHttp2Headers(2, false, false, false) { + @Override + public Iterator> iterator() { + Entry pseudo = Map.entry(":method", "GET"); + Entry connection = Map.entry(name, value); + return List.of(pseudo, connection).iterator(); + } + }; + hpackEncoder.encodeHeaders(1, in, toEncode, NEVER_SENSITIVE); + + final Http2Headers decoded = Http2Headers.newHeaders(); + + Http2Exception.StreamException e = assertThrows(Http2Exception.StreamException.class, new Executable() { + @Override + public void execute() throws Throwable { + hpackDecoder.decode(3, in, decoded, true); } }); + assertThat(e.streamId(), is(3)); + assertThat(e.error(), is(PROTOCOL_ERROR)); } }