From 3e6b54bb59d55d4d375c2e97157d34969b7ede0b Mon Sep 17 00:00:00 2001 From: Julien Hoarau Date: Wed, 13 Dec 2017 21:17:57 +1100 Subject: [PATCH] Fix failing h2spec tests 8.1.2.1 related to pseudo-headers validation Motivation: According to the spec: All pseudo-header fields MUST appear in the header block before regular header fields. Any request or response that contains a pseudo-header field that appears in a header block after a regular header field MUST be treated as malformed (Section 8.1.2.6). Pseudo-header fields are only valid in the context in which they are defined. Pseudo-header fields defined for requests MUST NOT appear in responses; pseudo-header fields defined for responses MUST NOT appear in requests. Pseudo-header fields MUST NOT appear in trailers. Endpoints MUST treat a request or response that contains undefined or invalid pseudo-header fields as malformed (Section 8.1.2.6). Clients MUST NOT accept a malformed response. Note that these requirements are intended to protect against several types of common attacks against HTTP; they are deliberately strict because being permissive can expose implementations to these vulnerabilities. Modifications: - Introduce validation in HPackDecoder Result: - Requests with unknown pseudo-field headers are rejected - Requests with containing response specific pseudo-headers are rejected - Requests where pseudo-header appear after regular header are rejected - h2spec 8.1.2.1 pass --- .../codec/http2/DefaultHttp2Headers.java | 13 +- .../http2/DefaultHttp2HeadersDecoder.java | 2 +- .../handler/codec/http2/HpackDecoder.java | 65 +++++++-- .../handler/codec/http2/Http2Headers.java | 54 ++++++-- .../handler/codec/http2/HpackDecoderTest.java | 131 ++++++++++++++++-- .../handler/codec/http2/HpackEncoderTest.java | 4 +- .../handler/codec/http2/HpackTestCase.java | 2 +- .../codec/http2/InOrderHttp2Headers.java | 104 ++++++++++++++ .../testdata/testStaticTableEntries.json | 9 +- .../testStaticTableResponseEntries.json | 23 +++ .../codec/http2/HpackDecoderBenchmark.java | 2 +- testsuite-http2/pom.xml | 3 - 12 files changed, 361 insertions(+), 51 deletions(-) create mode 100644 codec-http2/src/test/java/io/netty/handler/codec/http2/InOrderHttp2Headers.java create mode 100644 codec-http2/src/test/resources/io/netty/handler/codec/http2/testdata/testStaticTableResponseEntries.json diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Headers.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Headers.java index 9c39153d1f..0ab5705224 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Headers.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Headers.java @@ -21,16 +21,19 @@ import io.netty.util.ByteProcessor; import io.netty.util.internal.PlatformDependent; import io.netty.util.internal.UnstableApi; -import static io.netty.handler.codec.http2.Http2Error.*; -import static io.netty.handler.codec.http2.Http2Exception.*; -import static io.netty.util.AsciiString.*; +import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR; +import static io.netty.handler.codec.http2.Http2Exception.connectionError; +import static io.netty.handler.codec.http2.Http2Headers.PseudoHeaderName.hasPseudoHeaderFormat; +import static io.netty.util.AsciiString.CASE_INSENSITIVE_HASHER; +import static io.netty.util.AsciiString.CASE_SENSITIVE_HASHER; +import static io.netty.util.AsciiString.isUpperCase; @UnstableApi public class DefaultHttp2Headers extends DefaultHeaders implements Http2Headers { private static final ByteProcessor HTTP2_NAME_VALIDATOR_PROCESSOR = new ByteProcessor() { @Override - public boolean process(byte value) throws Exception { + public boolean process(byte value) { return !isUpperCase(value); } }; @@ -207,7 +210,7 @@ public class DefaultHttp2Headers this.next = next; // Make sure the pseudo headers fields are first in iteration order - if (key.length() != 0 && key.charAt(0) == ':') { + if (hasPseudoHeaderFormat(key)) { after = firstNonPseudo; before = firstNonPseudo.before(); } else { diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2HeadersDecoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2HeadersDecoder.java index 939d8309a9..5cc064a832 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2HeadersDecoder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2HeadersDecoder.java @@ -115,7 +115,7 @@ public class DefaultHttp2HeadersDecoder implements Http2HeadersDecoder, Http2Hea public Http2Headers decodeHeaders(int streamId, ByteBuf headerBlock) throws Http2Exception { try { final Http2Headers headers = newHeaders(); - hpackDecoder.decode(streamId, headerBlock, headers); + hpackDecoder.decode(streamId, headerBlock, headers, validateHeaders); headerArraySizeAccumulator = HEADERS_COUNT_WEIGHT_NEW * headers.size() + HEADERS_COUNT_WEIGHT_HISTORICAL * headerArraySizeAccumulator; return headers; diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/HpackDecoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/HpackDecoder.java index e69a581a27..67d6aa9944 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/HpackDecoder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/HpackDecoder.java @@ -45,6 +45,8 @@ import static io.netty.handler.codec.http2.Http2Error.COMPRESSION_ERROR; import static io.netty.handler.codec.http2.Http2Error.INTERNAL_ERROR; import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR; import static io.netty.handler.codec.http2.Http2Exception.connectionError; +import static io.netty.handler.codec.http2.Http2Headers.PseudoHeaderName.getPseudoHeader; +import static io.netty.handler.codec.http2.Http2Headers.PseudoHeaderName.hasPseudoHeaderFormat; import static io.netty.util.AsciiString.EMPTY_STRING; import static io.netty.util.internal.ObjectUtil.checkPositive; import static io.netty.util.internal.ThrowableUtil.unknownStackTrace; @@ -119,7 +121,7 @@ final class HpackDecoder { *

* This method assumes the entire header block is contained in {@code in}. */ - public void decode(int streamId, ByteBuf in, Http2Headers headers) throws Http2Exception { + public void decode(int streamId, ByteBuf in, Http2Headers headers, boolean validateHeaders) throws Http2Exception { int index = 0; long headersLength = 0; int nameLength = 0; @@ -127,6 +129,7 @@ final class HpackDecoder { byte state = READ_HEADER_REPRESENTATION; boolean huffmanEncoded = false; CharSequence name = null; + HeaderType headerType = null; IndexType indexType = IndexType.NONE; while (in.isReadable()) { switch (state) { @@ -146,7 +149,10 @@ final class HpackDecoder { state = READ_INDEXED_HEADER; break; default: - headersLength = indexHeader(index, headers, headersLength); + HpackHeaderField indexedHeader = getIndexedHeader(index); + headerType = validate(indexedHeader.name, headerType, validateHeaders); + headersLength = addHeader(headers, indexedHeader.name, indexedHeader.value, + headersLength); } } else if ((b & 0x40) == 0x40) { // Literal Header Field with Incremental Indexing @@ -162,6 +168,7 @@ final class HpackDecoder { default: // Index was stored as the prefix name = readName(index); + headerType = validate(name, headerType, validateHeaders); nameLength = name.length(); state = READ_LITERAL_HEADER_VALUE_LENGTH_PREFIX; } @@ -188,6 +195,7 @@ final class HpackDecoder { default: // Index was stored as the prefix name = readName(index); + headerType = validate(name, headerType, validateHeaders); nameLength = name.length(); state = READ_LITERAL_HEADER_VALUE_LENGTH_PREFIX; } @@ -200,13 +208,16 @@ final class HpackDecoder { break; case READ_INDEXED_HEADER: - headersLength = indexHeader(decodeULE128(in, index), headers, headersLength); + HpackHeaderField indexedHeader = getIndexedHeader(decodeULE128(in, index)); + headerType = validate(indexedHeader.name, headerType, validateHeaders); + headersLength = addHeader(headers, indexedHeader.name, indexedHeader.value, headersLength); state = READ_HEADER_REPRESENTATION; break; case READ_INDEXED_HEADER_NAME: // Header Name matches an entry in the Header Table name = readName(decodeULE128(in, index)); + headerType = validate(name, headerType, validateHeaders); nameLength = name.length(); state = READ_LITERAL_HEADER_VALUE_LENGTH_PREFIX; break; @@ -243,6 +254,7 @@ final class HpackDecoder { } name = readStringLiteral(in, nameLength, huffmanEncoded); + headerType = validate(name, headerType, validateHeaders); state = READ_LITERAL_HEADER_VALUE_LENGTH_PREFIX; break; @@ -256,6 +268,7 @@ final class HpackDecoder { state = READ_LITERAL_HEADER_VALUE_LENGTH; break; case 0: + headerType = validate(name, headerType, validateHeaders); headersLength = insertHeader(headers, name, EMPTY_STRING, indexType, headersLength); state = READ_HEADER_REPRESENTATION; break; @@ -288,6 +301,7 @@ final class HpackDecoder { } CharSequence value = readStringLiteral(in, valueLength, huffmanEncoded); + headerType = validate(name, headerType, validateHeaders); headersLength = insertHeader(headers, name, value, indexType, headersLength); state = READ_HEADER_REPRESENTATION; break; @@ -386,6 +400,34 @@ final class HpackDecoder { hpackDynamicTable.setCapacity(dynamicTableSize); } + private HeaderType validate(CharSequence name, HeaderType previousHeaderType, + final boolean validateHeaders) throws Http2Exception { + if (!validateHeaders) { + return null; + } + + if (hasPseudoHeaderFormat(name)) { + if (previousHeaderType == HeaderType.REGULAR_HEADER) { + throw connectionError(PROTOCOL_ERROR, "Pseudo-header field '%s' found after regular header.", name); + } + + final Http2Headers.PseudoHeaderName pseudoHeader = getPseudoHeader(name); + if (pseudoHeader == null) { + throw connectionError(PROTOCOL_ERROR, "Invalid HTTP/2 pseudo-header '%s' encountered.", name); + } + + final HeaderType currentHeaderType = pseudoHeader.isRequestOnly() ? + HeaderType.REQUEST_PSEUDO_HEADER : HeaderType.RESPONSE_PSEUDO_HEADER; + if (previousHeaderType != null && currentHeaderType != previousHeaderType) { + throw connectionError(PROTOCOL_ERROR, "Mix of request and response pseudo-headers."); + } + + return currentHeaderType; + } + + return HeaderType.REGULAR_HEADER; + } + private CharSequence readName(int index) throws Http2Exception { if (index <= HpackStaticTable.length) { HpackHeaderField hpackHeaderField = HpackStaticTable.getEntry(index); @@ -398,14 +440,12 @@ final class HpackDecoder { throw READ_NAME_ILLEGAL_INDEX_VALUE; } - private long indexHeader(int index, Http2Headers headers, long headersLength) throws Http2Exception { + private HpackHeaderField getIndexedHeader(int index) throws Http2Exception { if (index <= HpackStaticTable.length) { - HpackHeaderField hpackHeaderField = HpackStaticTable.getEntry(index); - return addHeader(headers, hpackHeaderField.name, hpackHeaderField.value, headersLength); + return HpackStaticTable.getEntry(index); } if (index - HpackStaticTable.length <= hpackDynamicTable.length()) { - HpackHeaderField hpackHeaderField = hpackDynamicTable.getEntry(index - HpackStaticTable.length); - return addHeader(headers, hpackHeaderField.name, hpackHeaderField.value, headersLength); + return hpackDynamicTable.getEntry(index - HpackStaticTable.length); } throw INDEX_HEADER_ILLEGAL_INDEX_VALUE; } @@ -504,4 +544,13 @@ final class HpackDecoder { throw DECODE_ULE_128_DECOMPRESSION_EXCEPTION; } + + /** + * HTTP/2 header types. + */ + private enum HeaderType { + REGULAR_HEADER, + REQUEST_PSEUDO_HEADER, + RESPONSE_PSEUDO_HEADER + } } diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Headers.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Headers.java index d2b09610d6..f0999bf644 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Headers.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Headers.java @@ -35,38 +35,44 @@ public interface Http2Headers extends Headers PSEUDO_HEADERS = new CharSequenceMap(); + private final boolean requestOnly; + private static final CharSequenceMap PSEUDO_HEADERS = new CharSequenceMap(); + static { for (PseudoHeaderName pseudoHeader : PseudoHeaderName.values()) { - PSEUDO_HEADERS.add(pseudoHeader.value(), AsciiString.EMPTY_STRING); + PSEUDO_HEADERS.add(pseudoHeader.value(), pseudoHeader); } } - PseudoHeaderName(String value) { + PseudoHeaderName(String value, boolean requestOnly) { this.value = AsciiString.cached(value); + this.requestOnly = requestOnly; } public AsciiString value() { @@ -74,12 +80,44 @@ public interface Http2Headers extends Headers 0 && asciiHeaderName.byteAt(0) == PSEUDO_HEADER_PREFIX_BYTE; + } else { + return headerName.length() > 0 && headerName.charAt(0) == PSEUDO_HEADER_PREFIX; + } + } + /** * Indicates whether the given header name is a valid HTTP/2 pseudo header. */ public static boolean isPseudoHeader(CharSequence header) { return PSEUDO_HEADERS.contains(header); } + + /** + * Returns the {@link PseudoHeaderName} corresponding to the specified header name. + * + * @return corresponding {@link PseudoHeaderName} if any, {@code null} otherwise. + */ + public static PseudoHeaderName getPseudoHeader(CharSequence header) { + return PSEUDO_HEADERS.get(header); + } + + /** + * Indicates whether the pseudo-header is to be used in a request context. + * + * @return {@code true} if the pseudo-header is to be used in a request context + */ + public boolean isRequestOnly() { + return requestOnly; + } } /** diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/HpackDecoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/HpackDecoderTest.java index 16cda47c21..4652a1fceb 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/HpackDecoderTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/HpackDecoderTest.java @@ -71,7 +71,7 @@ public class HpackDecoderTest { byte[] b = StringUtil.decodeHexDump(encoded); ByteBuf in = Unpooled.wrappedBuffer(b); try { - hpackDecoder.decode(0, in, mockHeaders); + hpackDecoder.decode(0, in, mockHeaders, true); } finally { in.release(); } @@ -167,7 +167,7 @@ public class HpackDecoderTest { try { final long expectedHeaderSize = 4026531870L; // based on the input above hpackDecoder.setMaxHeaderTableSize(expectedHeaderSize); - hpackDecoder.decode(0, in, mockHeaders); + hpackDecoder.decode(0, in, mockHeaders, true); assertEquals(expectedHeaderSize, hpackDecoder.getMaxHeaderTableSize()); } finally { in.release(); @@ -180,7 +180,7 @@ public class HpackDecoderTest { ByteBuf in = Unpooled.wrappedBuffer(input); try { hpackDecoder.setMaxHeaderTableSize(4026531870L - 1); // based on the input above ... 1 less than is above. - hpackDecoder.decode(0, in, mockHeaders); + hpackDecoder.decode(0, in, mockHeaders, true); } finally { in.release(); } @@ -191,7 +191,7 @@ public class HpackDecoderTest { byte[] input = {0, (byte) 0x80, 0}; ByteBuf in = Unpooled.wrappedBuffer(input); try { - hpackDecoder.decode(0, in, mockHeaders); + hpackDecoder.decode(0, in, mockHeaders, true); verify(mockHeaders, times(1)).add(EMPTY_STRING, EMPTY_STRING); } finally { in.release(); @@ -203,7 +203,7 @@ public class HpackDecoderTest { byte[] input = {0, (byte) 0x81, -1}; ByteBuf in = Unpooled.wrappedBuffer(input); try { - hpackDecoder.decode(0, in, mockHeaders); + hpackDecoder.decode(0, in, mockHeaders, true); } finally { in.release(); } @@ -214,7 +214,7 @@ public class HpackDecoderTest { byte[] input = {0, (byte) 0x84, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF}; ByteBuf in = Unpooled.wrappedBuffer(input); try { - hpackDecoder.decode(0, in, mockHeaders); + hpackDecoder.decode(0, in, mockHeaders, true); } finally { in.release(); } @@ -225,7 +225,7 @@ public class HpackDecoderTest { byte[] input = {0, (byte) 0x81, 0}; ByteBuf in = Unpooled.wrappedBuffer(input); try { - hpackDecoder.decode(0, in, mockHeaders); + hpackDecoder.decode(0, in, mockHeaders, true); } finally { in.release(); } @@ -236,9 +236,9 @@ public class HpackDecoderTest { byte[] compressed = StringUtil.decodeHexDump("FFF0"); ByteBuf in = Unpooled.wrappedBuffer(compressed); try { - hpackDecoder.decode(0, in, mockHeaders); + hpackDecoder.decode(0, in, mockHeaders, true); assertEquals(1, in.readableBytes()); - hpackDecoder.decode(0, in, mockHeaders); + hpackDecoder.decode(0, in, mockHeaders, true); } finally { in.release(); } @@ -432,7 +432,7 @@ public class HpackDecoderTest { @Test public void testDecodeLargerThanMaxHeaderListSizeButSmallerThanMaxHeaderListSizeUpdatesDynamicTable() - throws Http2Exception { + throws Http2Exception { ByteBuf in = Unpooled.buffer(300); try { hpackDecoder.setMaxHeaderListSize(200, 300); @@ -451,7 +451,7 @@ public class HpackDecoderTest { // the decoded headers object should contain all of the headers Http2Headers decoded = new DefaultHttp2Headers(); try { - hpackDecoder.decode(1, in, decoded); + hpackDecoder.decode(1, in, decoded, true); fail(); } catch (Http2Exception e) { assertTrue(e instanceof Http2Exception.HeaderListSizeException); @@ -479,7 +479,7 @@ public class HpackDecoderTest { hpackEncoder.encodeHeaders(1, in, toEncode, NEVER_SENSITIVE); Http2Headers decoded = new DefaultHttp2Headers(); - hpackDecoder.decode(1, in, decoded); + hpackDecoder.decode(1, in, decoded, true); assertEquals(2, decoded.size()); } finally { in.release(); @@ -507,7 +507,7 @@ public class HpackDecoderTest { // ... but decode should fail because we add some overhead for each header entry expectedException.expect(Http2Exception.HeaderListSizeException.class); - hpackDecoder.decode(1, in, decoded); + hpackDecoder.decode(1, in, decoded, true); } finally { in.release(); } @@ -520,7 +520,110 @@ public class HpackDecoderTest { ByteBuf in = Unpooled.wrappedBuffer(input); try { expectedException.expect(Http2Exception.class); - hpackDecoder.decode(0, in, mockHeaders); + hpackDecoder.decode(0, in, mockHeaders, true); + } finally { + in.release(); + } + } + + @Test + public void unknownPseudoHeader() throws Exception { + ByteBuf in = Unpooled.buffer(200); + try { + HpackEncoder hpackEncoder = new HpackEncoder(true); + + Http2Headers toEncode = new DefaultHttp2Headers(); + toEncode.add(":test", "1"); + hpackEncoder.encodeHeaders(1, in, toEncode, NEVER_SENSITIVE); + + Http2Headers decoded = new DefaultHttp2Headers(); + + expectedException.expect(Http2Exception.class); + hpackDecoder.decode(1, in, decoded, true); + } finally { + in.release(); + } + } + + @Test + public void disableHeaderValidation() throws Exception { + ByteBuf in = Unpooled.buffer(200); + try { + HpackEncoder hpackEncoder = new HpackEncoder(true); + + Http2Headers toEncode = new DefaultHttp2Headers(); + toEncode.add(":test", "1"); + toEncode.add(":status", "200"); + toEncode.add(":method", "GET"); + hpackEncoder.encodeHeaders(1, in, toEncode, NEVER_SENSITIVE); + + Http2Headers decoded = new DefaultHttp2Headers(); + + hpackDecoder.decode(1, in, decoded, false); + + assertThat(decoded.valueIterator(":test").next().toString(), is("1")); + assertThat(decoded.status().toString(), is("200")); + assertThat(decoded.method().toString(), is("GET")); + } finally { + in.release(); + } + } + + @Test + public void requestPseudoHeaderInResponse() throws Exception { + ByteBuf in = Unpooled.buffer(200); + try { + HpackEncoder hpackEncoder = new HpackEncoder(true); + + Http2Headers toEncode = new DefaultHttp2Headers(); + toEncode.add(":status", "200"); + toEncode.add(":method", "GET"); + hpackEncoder.encodeHeaders(1, in, toEncode, NEVER_SENSITIVE); + + Http2Headers decoded = new DefaultHttp2Headers(); + + expectedException.expect(Http2Exception.class); + hpackDecoder.decode(1, in, decoded, true); + } finally { + in.release(); + } + } + + @Test + public void responsePseudoHeaderInRequest() throws Exception { + ByteBuf in = Unpooled.buffer(200); + try { + HpackEncoder hpackEncoder = new HpackEncoder(true); + + Http2Headers toEncode = new DefaultHttp2Headers(); + toEncode.add(":method", "GET"); + toEncode.add(":status", "200"); + hpackEncoder.encodeHeaders(1, in, toEncode, NEVER_SENSITIVE); + + Http2Headers decoded = new DefaultHttp2Headers(); + + expectedException.expect(Http2Exception.class); + hpackDecoder.decode(1, in, decoded, true); + } finally { + in.release(); + } + } + + @Test + public void pseudoHeaderAfterRegularHeader() throws Exception { + ByteBuf in = Unpooled.buffer(200); + try { + HpackEncoder hpackEncoder = new HpackEncoder(true); + + Http2Headers toEncode = new InOrderHttp2Headers(); + toEncode.add("test", "1"); + toEncode.add(":method", "GET"); + hpackEncoder.encodeHeaders(1, in, toEncode, NEVER_SENSITIVE); + + Http2Headers decoded = new DefaultHttp2Headers(); + + expectedException.expect(Http2Exception.class); + hpackDecoder.decode(1, in, decoded, true); } finally { in.release(); } diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/HpackEncoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/HpackEncoderTest.java index 7c6654c367..2996b5db6b 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/HpackEncoderTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/HpackEncoderTest.java @@ -31,7 +31,7 @@ public class HpackEncoderTest { private Http2Headers mockHeaders; @Before - public void setUp() throws Http2Exception { + public void setUp() { hpackEncoder = new HpackEncoder(); hpackDecoder = new HpackDecoder(DEFAULT_HEADER_LIST_SIZE, 32); mockHeaders = mock(Http2Headers.class); @@ -42,7 +42,7 @@ public class HpackEncoderTest { ByteBuf buf = Unpooled.buffer(); hpackEncoder.setMaxHeaderTableSize(buf, MAX_HEADER_TABLE_SIZE); hpackDecoder.setMaxHeaderTableSize(MAX_HEADER_TABLE_SIZE); - hpackDecoder.decode(0, buf, mockHeaders); + hpackDecoder.decode(0, buf, mockHeaders, true); assertEquals(MAX_HEADER_TABLE_SIZE, hpackDecoder.getMaxHeaderTableSize()); buf.release(); } diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/HpackTestCase.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/HpackTestCase.java index e3e2c960e2..33faa50f62 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/HpackTestCase.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/HpackTestCase.java @@ -214,7 +214,7 @@ final class HpackTestCase { try { List headers = new ArrayList(); TestHeaderListener listener = new TestHeaderListener(headers); - hpackDecoder.decode(0, in, listener); + hpackDecoder.decode(0, in, listener, true); return headers; } finally { in.release(); diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/InOrderHttp2Headers.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/InOrderHttp2Headers.java new file mode 100644 index 0000000000..5e523939f3 --- /dev/null +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/InOrderHttp2Headers.java @@ -0,0 +1,104 @@ +/* + * Copyright 2017 The Netty Project + * + * The Netty Project licenses this file to you 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 io.netty.handler.codec.http2; + +import io.netty.handler.codec.CharSequenceValueConverter; +import io.netty.handler.codec.DefaultHeaders; + +import static io.netty.util.AsciiString.CASE_INSENSITIVE_HASHER; +import static io.netty.util.AsciiString.CASE_SENSITIVE_HASHER; + +/** + * Http2Headers implementation that preserves headers insertion order. + */ +public class InOrderHttp2Headers + extends DefaultHeaders implements Http2Headers { + + InOrderHttp2Headers() { + super(CharSequenceValueConverter.INSTANCE); + } + + @Override + public boolean equals(Object o) { + return o instanceof Http2Headers && equals((Http2Headers) o, CASE_SENSITIVE_HASHER); + } + + @Override + public int hashCode() { + return hashCode(CASE_SENSITIVE_HASHER); + } + + @Override + public Http2Headers method(CharSequence value) { + set(PseudoHeaderName.METHOD.value(), value); + return this; + } + + @Override + public Http2Headers scheme(CharSequence value) { + set(PseudoHeaderName.SCHEME.value(), value); + return this; + } + + @Override + public Http2Headers authority(CharSequence value) { + set(PseudoHeaderName.AUTHORITY.value(), value); + return this; + } + + @Override + public Http2Headers path(CharSequence value) { + set(PseudoHeaderName.PATH.value(), value); + return this; + } + + @Override + public Http2Headers status(CharSequence value) { + set(PseudoHeaderName.STATUS.value(), value); + return this; + } + + @Override + public CharSequence method() { + return get(PseudoHeaderName.METHOD.value()); + } + + @Override + public CharSequence scheme() { + return get(PseudoHeaderName.SCHEME.value()); + } + + @Override + public CharSequence authority() { + return get(PseudoHeaderName.AUTHORITY.value()); + } + + @Override + public CharSequence path() { + return get(PseudoHeaderName.PATH.value()); + } + + @Override + public CharSequence status() { + return get(PseudoHeaderName.STATUS.value()); + } + + @Override + public boolean contains(CharSequence name, CharSequence value, boolean caseInsensitive) { + return contains(name, value, caseInsensitive ? CASE_INSENSITIVE_HASHER : CASE_SENSITIVE_HASHER); + } +} diff --git a/codec-http2/src/test/resources/io/netty/handler/codec/http2/testdata/testStaticTableEntries.json b/codec-http2/src/test/resources/io/netty/handler/codec/http2/testdata/testStaticTableEntries.json index fcac28aea9..9f5ccc2631 100644 --- a/codec-http2/src/test/resources/io/netty/handler/codec/http2/testdata/testStaticTableEntries.json +++ b/codec-http2/src/test/resources/io/netty/handler/codec/http2/testdata/testStaticTableEntries.json @@ -10,13 +10,6 @@ { ":path": "/index.html" }, { ":scheme": "http" }, { ":scheme": "https" }, - { ":status": "200" }, - { ":status": "204" }, - { ":status": "206" }, - { ":status": "304" }, - { ":status": "400" }, - { ":status": "404" }, - { ":status": "500" }, { "accept-charset": "" }, { "accept-encoding": "gzip, deflate" }, { "accept-language": "" }, @@ -66,7 +59,7 @@ { "www-authenticate": "" } ], "encoded": [ - "8182 8384 8586 8788 898a 8b8c 8d8e 8f90", + "8182 8384 8586 87 8f90", "9192 9394 9596 9798 999a 9b9c 9d9e 9fa0", "a1a2 a3a4 a5a6 a7a8 a9aa abac adae afb0", "b1b2 b3b4 b5b6 b7b8 b9ba bbbc bd" diff --git a/codec-http2/src/test/resources/io/netty/handler/codec/http2/testdata/testStaticTableResponseEntries.json b/codec-http2/src/test/resources/io/netty/handler/codec/http2/testdata/testStaticTableResponseEntries.json new file mode 100644 index 0000000000..91ec07b8ea --- /dev/null +++ b/codec-http2/src/test/resources/io/netty/handler/codec/http2/testdata/testStaticTableResponseEntries.json @@ -0,0 +1,23 @@ +{ + "header_blocks": + [ + { + "headers": [ + { ":status": "200" }, + { ":status": "204" }, + { ":status": "206" }, + { ":status": "304" }, + { ":status": "400" }, + { ":status": "404" }, + { ":status": "500" } + ], + "encoded": [ + "8889 8a8b 8c8d 8e" + ], + "dynamic_table": [ + ], + "table_size": 0 + } + ] +} + diff --git a/microbench/src/main/java/io/netty/handler/codec/http2/HpackDecoderBenchmark.java b/microbench/src/main/java/io/netty/handler/codec/http2/HpackDecoderBenchmark.java index 067069a7fa..3ecf1bcd87 100644 --- a/microbench/src/main/java/io/netty/handler/codec/http2/HpackDecoderBenchmark.java +++ b/microbench/src/main/java/io/netty/handler/codec/http2/HpackDecoderBenchmark.java @@ -82,7 +82,7 @@ public class HpackDecoderBenchmark extends AbstractMicrobenchmark { return this; } }; - hpackDecoder.decode(0, input.duplicate(), headers); + hpackDecoder.decode(0, input.duplicate(), headers, true); } private byte[] getSerializedHeaders(Http2Headers headers, boolean sensitive) throws Http2Exception { diff --git a/testsuite-http2/pom.xml b/testsuite-http2/pom.xml index 6b4100e0ba..3dc0dc43b3 100644 --- a/testsuite-http2/pom.xml +++ b/testsuite-http2/pom.xml @@ -72,9 +72,6 @@ 5.1 - closed: Sends a HEADERS frame 5.1.1 - Sends stream identifier that is numerically smaller than previous 7 - Sends a GOAWAY frame with unknown error code - 8.1.2.1 - Sends a HEADERS frame that contains a unknown pseudo-header field - 8.1.2.1 - Sends a HEADERS frame that contains the pseudo-header field defined for response - 8.1.2.1 - Sends a HEADERS frame that contains a pseudo-header field that appears in a header block after a regular header field 8.1.2.2 - Sends a HEADERS frame that contains the connection-specific header field 8.1.2.2 - Sends a HEADERS frame that contains the TE header field with any value other than "trailers" 8.1.2.3 - Sends a HEADERS frame with empty ":path" pseudo-header field