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

Author: Julien Hoarau

codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Headers.java

@@ -21,16 +21,19 @@
 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<CharSequence, CharSequence, Http2Headers> 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 @@ protected Http2HeaderEntry(int hash, CharSequence key, CharSequence value,
             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 {

codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2HeadersDecoder.java

@@ -115,7 +115,7 @@ public Configuration configuration() {
     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;

codec-http2/src/main/java/io/netty/handler/codec/http2/HpackDecoder.java

@@ -45,6 +45,8 @@
 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,14 +121,15 @@ final class HpackDecoder {
      * <p>
      * 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;
         int valueLength = 0;
         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 @@ public void decode(int streamId, ByteBuf in, Http2Headers headers) throws Http2E
                                 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 @@ public void decode(int streamId, ByteBuf in, Http2Headers headers) throws Http2E
                             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 @@ public void decode(int streamId, ByteBuf in, Http2Headers headers) throws Http2E
                             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 @@ public void decode(int streamId, ByteBuf in, Http2Headers headers) throws Http2E
                     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 @@ public void decode(int streamId, ByteBuf in, Http2Headers headers) throws Http2E
                     }
 
                     name = readStringLiteral(in, nameLength, huffmanEncoded);
+                    headerType = validate(name, headerType, validateHeaders);
 
                     state = READ_LITERAL_HEADER_VALUE_LENGTH_PREFIX;
                     break;
@@ -256,6 +268,7 @@ public void decode(int streamId, ByteBuf in, Http2Headers headers) throws Http2E
                             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 @@ public void decode(int streamId, ByteBuf in, Http2Headers headers) throws Http2E
                     }
 
                     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 @@ private void setDynamicTableSize(long dynamicTableSize) throws Http2Exception {
         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 @@ private CharSequence readName(int index) throws Http2Exception {
         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 @@ static long decodeULE128(ByteBuf in, long result) throws Http2Exception {
 
         throw DECODE_ULE_128_DECOMPRESSION_EXCEPTION;
     }
+
+    /**
+     * HTTP/2 header types.
+     */
+    private enum HeaderType {
+        REGULAR_HEADER,
+        REQUEST_PSEUDO_HEADER,
+        RESPONSE_PSEUDO_HEADER
+    }
 }

codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Headers.java

@@ -35,51 +35,89 @@ enum PseudoHeaderName {
         /**
          * {@code :method}.
          */
-        METHOD(":method"),
+        METHOD(":method", true),
 
         /**
          * {@code :scheme}.
          */
-        SCHEME(":scheme"),
+        SCHEME(":scheme", true),
 
         /**
          * {@code :authority}.
          */
-        AUTHORITY(":authority"),
+        AUTHORITY(":authority", true),
 
         /**
          * {@code :path}.
          */
-        PATH(":path"),
+        PATH(":path", true),
 
         /**
          * {@code :status}.
          */
-        STATUS(":status");
+        STATUS(":status", false);
+
+        private static final char PSEUDO_HEADER_PREFIX = ':';
+        private static final byte PSEUDO_HEADER_PREFIX_BYTE = (byte) PSEUDO_HEADER_PREFIX;
 
         private final AsciiString value;
-        private static final CharSequenceMap<AsciiString> PSEUDO_HEADERS = new CharSequenceMap<AsciiString>();
+        private final boolean requestOnly;
+        private static final CharSequenceMap<PseudoHeaderName> PSEUDO_HEADERS = new CharSequenceMap<PseudoHeaderName>();
+
         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() {
             // Return a slice so that the buffer gets its own reader index.
             return value;
         }
 
+        /**
+         * Indicates whether the specified header follows the pseudo-header format (begins with ':' character)
+         *
+         * @return {@code true} if the header follow the pseudo-header format
+         */
+        public static boolean hasPseudoHeaderFormat(CharSequence headerName) {
+            if (headerName instanceof AsciiString) {
+                final AsciiString asciiHeaderName = (AsciiString) headerName;
+                return asciiHeaderName.length() > 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;
+        }
     }
 
     /**