HTTP/2 Header Name Validation

Motivation: The HTTP/2 header name validation was removed, and does not currently exist. Modifications: - Header name validation for HTTP/2 should be restored and set to the default mode of operation. Result: HTTP/2 header names are validated according to https://tools.ietf.org/html/rfc7540
fl4via · Sep 9, 2015 · 4f20400 · 4f20400
1 parent c449cea
commit 4f20400
Show file tree

Hide file tree

Showing 14 changed files with 140 additions and 33 deletions.
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameReader.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameReader.java
@@ -64,8 +64,22 @@ private enum State {
     private HeadersContinuation headersContinuation;
     private int maxFrameSize;
 
+    /**
+     * Create a new instance.
+     * <p>
+     * Header names will be validated.
+     */
     public DefaultHttp2FrameReader() {
-        this(new DefaultHttp2HeadersDecoder());
+        this(true);
+    }
+
+    /**
+     * Create a new instance.
+     * @param validateHeaders {@code true} to validate headers. {@code false} to not validate headers.
+     * @see #DefaultHttp2HeadersDecoder(boolean)
+     */
+    public DefaultHttp2FrameReader(boolean validateHeaders) {
+        this(new DefaultHttp2HeadersDecoder(validateHeaders));
     }
 
     public DefaultHttp2FrameReader(Http2HeadersDecoder headersDecoder) {

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
@@ -17,13 +17,50 @@
 import io.netty.handler.codec.ByteStringValueConverter;
 import io.netty.handler.codec.DefaultHeaders;
 import io.netty.handler.codec.Headers;
+import io.netty.util.ByteProcessor;
 import io.netty.util.ByteString;
+import io.netty.util.internal.PlatformDependent;
 
 public class DefaultHttp2Headers extends DefaultHeaders<ByteString> implements Http2Headers {
+    private static final ByteProcessor HTTP2_NAME_VALIDATOR_PROCESSOR = new ByteProcessor() {
+        @Override
+        public boolean process(byte value) throws Exception {
+            if (value >= 'A' && value <= 'Z') {
+                throw new IllegalArgumentException("name must be all lower case but found: " + (char) value);
+            }
+            return true;
+        }
+    };
+    private static final NameValidator<ByteString> HTTP2_NAME_VALIDATOR = new NameValidator<ByteString>() {
+        @Override
+        public void validateName(ByteString name) {
+            try {
+                name.forEachByte(HTTP2_NAME_VALIDATOR_PROCESSOR);
+            } catch (Exception e) {
+                PlatformDependent.throwException(e);
+            }
+        }
+    };
     private HeaderEntry<ByteString> firstNonPseudo = head;
 
+    /**
+     * Create a new instance.
+     * <p>
+     * Header names will be validated according to
+     * <a href="https://tools.ietf.org/html/rfc7540">rfc7540</a>.
+     */
     public DefaultHttp2Headers() {
-        super(ByteStringValueConverter.INSTANCE);
+        this(true);
+    }
+
+    /**
+     * Create a new instance.
+     * @param validate {@code true} to validate header names according to
+     * <a href="https://tools.ietf.org/html/rfc7540">rfc7540</a>. {@code false} to not validate header names.
+     */
+    @SuppressWarnings("unchecked")
+    public DefaultHttp2Headers(boolean validate) {
+        super(ByteStringValueConverter.INSTANCE, validate ? HTTP2_NAME_VALIDATOR : NameValidator.NOT_NULL);
     }
 
     @Override

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
@@ -36,18 +36,28 @@ public class DefaultHttp2HeadersDecoder implements Http2HeadersDecoder, Http2Hea
     private final int maxHeaderSize;
     private final Decoder decoder;
     private final Http2HeaderTable headerTable;
+    private final boolean validateHeaders;
 
     public DefaultHttp2HeadersDecoder() {
-        this(DEFAULT_MAX_HEADER_SIZE, DEFAULT_HEADER_TABLE_SIZE);
+        this(true);
+    }
+
+    public DefaultHttp2HeadersDecoder(boolean validateHeaders) {
+        this(DEFAULT_MAX_HEADER_SIZE, DEFAULT_HEADER_TABLE_SIZE, validateHeaders);
     }
 
     public DefaultHttp2HeadersDecoder(int maxHeaderSize, int maxHeaderTableSize) {
+        this(maxHeaderSize, maxHeaderTableSize, true);
+    }
+
+    public DefaultHttp2HeadersDecoder(int maxHeaderSize, int maxHeaderTableSize, boolean validateHeaders) {
         if (maxHeaderSize <= 0) {
             throw new IllegalArgumentException("maxHeaderSize must be positive: " + maxHeaderSize);
         }
         decoder = new Decoder(maxHeaderSize, maxHeaderTableSize);
         headerTable = new Http2HeaderTableDecoder();
         this.maxHeaderSize = maxHeaderSize;
+        this.validateHeaders = validateHeaders;
     }
 
     @Override
@@ -77,7 +87,7 @@ protected void maxHeaderSizeExceeded() throws Http2Exception {
     public Http2Headers decodeHeaders(ByteBuf headerBlock) throws Http2Exception {
         InputStream in = new ByteBufInputStream(headerBlock);
         try {
-            final Http2Headers headers = new DefaultHttp2Headers();
+            final Http2Headers headers = new DefaultHttp2Headers(validateHeaders);
             HeaderListener listener = new HeaderListener() {
                 @Override
                 public void addHeader(byte[] key, byte[] value, boolean sensitive) {

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java
@@ -69,11 +69,19 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http
     private long gracefulShutdownTimeoutMillis = DEFAULT_GRACEFUL_SHUTDOWN_TIMEOUT_MILLIS;
 
     public Http2ConnectionHandler(boolean server, Http2FrameListener listener) {
-        this(new DefaultHttp2Connection(server), listener);
+        this(server, listener, true);
+    }
+
+    public Http2ConnectionHandler(boolean server, Http2FrameListener listener, boolean validateHeaders) {
+        this(new DefaultHttp2Connection(server), listener, validateHeaders);
     }
 
     public Http2ConnectionHandler(Http2Connection connection, Http2FrameListener listener) {
-        this(connection, new DefaultHttp2FrameReader(), new DefaultHttp2FrameWriter(), listener);
+        this(connection, listener, true);
+    }
+
+    public Http2ConnectionHandler(Http2Connection connection, Http2FrameListener listener, boolean validateHeaders) {
+        this(connection, new DefaultHttp2FrameReader(validateHeaders), new DefaultHttp2FrameWriter(), listener);
     }
 
     public Http2ConnectionHandler(Http2Connection connection, Http2FrameReader frameReader,

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpConversionUtil.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpConversionUtil.java
@@ -279,8 +279,8 @@ public static void addHttp2ToHttpHeaders(int streamId, Http2Headers sourceHeader
      * </ul>
      * {@link ExtensionHeaderNames#PATH} is ignored and instead extracted from the {@code Request-Line}.
      */
-    public static Http2Headers toHttp2Headers(HttpMessage in) throws Exception {
-        final Http2Headers out = new DefaultHttp2Headers();
+    public static Http2Headers toHttp2Headers(HttpMessage in, boolean validateHeaders) throws Exception {
+        final Http2Headers out = new DefaultHttp2Headers(validateHeaders);
         HttpHeaders inHeaders = in.headers();
         if (in instanceof HttpRequest) {
             HttpRequest request = (HttpRequest) in;
@@ -304,15 +304,15 @@ public static Http2Headers toHttp2Headers(HttpMessage in) throws Exception {
         }
 
         // Add the HTTP headers which have not been consumed above
-        return out.add(toHttp2Headers(inHeaders));
+        return out.add(toHttp2Headers(inHeaders, validateHeaders));
     }
 
-    public static Http2Headers toHttp2Headers(HttpHeaders inHeaders) throws Exception {
+    public static Http2Headers toHttp2Headers(HttpHeaders inHeaders, boolean validateHeaders) throws Exception {
         if (inHeaders.isEmpty()) {
             return EmptyHttp2Headers.INSTANCE;
         }
 
-        final Http2Headers out = new DefaultHttp2Headers();
+        final Http2Headers out = new DefaultHttp2Headers(validateHeaders);
 
         for (Entry<CharSequence, CharSequence> entry : inHeaders) {
             final AsciiString aName = AsciiString.of(entry.getKey()).toLowerCase();

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpToHttp2ConnectionHandler.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpToHttp2ConnectionHandler.java
@@ -33,24 +33,48 @@
  */
 public class HttpToHttp2ConnectionHandler extends Http2ConnectionHandler {
 
+    private final boolean validateHeaders;
     private int currentStreamId;
 
     public HttpToHttp2ConnectionHandler(boolean server, Http2FrameListener listener) {
+        this(server, listener, true);
+    }
+
+    public HttpToHttp2ConnectionHandler(boolean server, Http2FrameListener listener, boolean validateHeaders) {
         super(server, listener);
+        this.validateHeaders = validateHeaders;
     }
 
     public HttpToHttp2ConnectionHandler(Http2Connection connection, Http2FrameListener listener) {
+        this(connection, listener, true);
+    }
+
+    public HttpToHttp2ConnectionHandler(Http2Connection connection, Http2FrameListener listener,
+                                        boolean validateHeaders) {
         super(connection, listener);
+        this.validateHeaders = validateHeaders;
     }
 
     public HttpToHttp2ConnectionHandler(Http2Connection connection, Http2FrameReader frameReader,
             Http2FrameWriter frameWriter, Http2FrameListener listener) {
+        this(connection, frameReader, frameWriter, listener, true);
+    }
+
+    public HttpToHttp2ConnectionHandler(Http2Connection connection, Http2FrameReader frameReader,
+            Http2FrameWriter frameWriter, Http2FrameListener listener, boolean validateHeaders) {
         super(connection, frameReader, frameWriter, listener);
+        this.validateHeaders = validateHeaders;
     }
 
     public HttpToHttp2ConnectionHandler(Http2ConnectionDecoder decoder,
                                         Http2ConnectionEncoder encoder) {
+        this(decoder, encoder, true);
+    }
+
+    public HttpToHttp2ConnectionHandler(Http2ConnectionDecoder decoder,
+            Http2ConnectionEncoder encoder, boolean validateHeaders) {
         super(decoder, encoder);
+        this.validateHeaders = validateHeaders;
     }
 
     /**
@@ -89,7 +113,7 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise)
                 currentStreamId = getStreamId(httpMsg.headers());
 
                 // Convert and write the headers.
-                Http2Headers http2Headers = HttpConversionUtil.toHttp2Headers(httpMsg);
+                Http2Headers http2Headers = HttpConversionUtil.toHttp2Headers(httpMsg, validateHeaders);
                 endStream = msg instanceof FullHttpMessage && !((FullHttpMessage) msg).content().isReadable();
                 encoder.writeHeaders(ctx, currentStreamId, http2Headers, 0, endStream, promiseAggregator.newPromise());
             }
@@ -102,7 +126,7 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise)
 
                     // Convert any trailing headers.
                     final LastHttpContent lastContent = (LastHttpContent) msg;
-                    trailers = HttpConversionUtil.toHttp2Headers(lastContent.trailingHeaders());
+                    trailers = HttpConversionUtil.toHttp2Headers(lastContent.trailingHeaders(), validateHeaders);
                 }
 
                 // Write the data

diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2FrameIOTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2FrameIOTest.java
@@ -121,7 +121,7 @@ public ChannelFuture answer(InvocationOnMock in) throws Throwable {
             }
         }).when(ctx).write(any(), any(ChannelPromise.class));
 
-        reader = new DefaultHttp2FrameReader();
+        reader = new DefaultHttp2FrameReader(false);
         writer = new DefaultHttp2FrameWriter();
     }
 
@@ -357,21 +357,21 @@ private ByteBuf dummyData() {
     }
 
     private static Http2Headers dummyBinaryHeaders() {
-        DefaultHttp2Headers headers = new DefaultHttp2Headers();
+        DefaultHttp2Headers headers = new DefaultHttp2Headers(false);
         for (int ix = 0; ix < 10; ++ix) {
             headers.add(randomString(), randomString());
         }
         return headers;
     }
 
     private static Http2Headers dummyHeaders() {
-        return new DefaultHttp2Headers().method(new AsciiString("GET")).scheme(new AsciiString("https"))
+        return new DefaultHttp2Headers(false).method(new AsciiString("GET")).scheme(new AsciiString("https"))
                 .authority(new AsciiString("example.org")).path(new AsciiString("/some/path"))
                 .add(new AsciiString("accept"), new AsciiString("*/*"));
     }
 
     private static Http2Headers largeHeaders() {
-        DefaultHttp2Headers headers = new DefaultHttp2Headers();
+        DefaultHttp2Headers headers = new DefaultHttp2Headers(false);
         for (int i = 0; i < 100; ++i) {
             String key = "this-is-a-test-header-key-" + i;
             String value = "this-is-a-test-header-value-" + i;
@@ -382,7 +382,7 @@ private static Http2Headers largeHeaders() {
 
     private Http2Headers headersOfSize(final int minSize) {
         final ByteString singleByte = new ByteString(new byte[]{0});
-        DefaultHttp2Headers headers = new DefaultHttp2Headers();
+        DefaultHttp2Headers headers = new DefaultHttp2Headers(false);
         for (int size = 0; size < minSize; size += 2) {
             headers.add(singleByte, singleByte);
         }

diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2HeadersDecoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2HeadersDecoderTest.java
@@ -40,7 +40,7 @@ public class DefaultHttp2HeadersDecoderTest {
 
     @Before
     public void setup() {
-        decoder = new DefaultHttp2HeadersDecoder();
+        decoder = new DefaultHttp2HeadersDecoder(false);
     }
 
     @Test

diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2HeadersTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2HeadersTest.java
@@ -65,6 +65,13 @@ public void pseudoHeadersWithRemovePreservesPseudoIterationOrder() {
         }
     }
 
+    @Test(expected = IllegalArgumentException.class)
+    public void testHeaderNameValidation() {
+        Http2Headers headers = newHeaders();
+
+        headers.add(fromAscii("Foo"), fromAscii("foo"));
+    }
+
     private static void verifyAllPseudoHeadersPresent(Http2Headers headers) {
         for (PseudoHeaderName pseudoName : PseudoHeaderName.values()) {
             assertNotNull(headers.get(pseudoName.value()));

diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionRoundtripTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionRoundtripTest.java
@@ -482,7 +482,7 @@ protected void initChannel(Channel ch) throws Exception {
                 serverFrameCountDown =
                         new FrameCountDown(serverListener, serverSettingsAckLatch,
                                 requestLatch, dataLatch, trailersLatch, goAwayLatch);
-                p.addLast(new Http2ConnectionHandler(true, serverFrameCountDown));
+                p.addLast(new Http2ConnectionHandler(true, serverFrameCountDown, false));
             }
         });
 
@@ -492,7 +492,7 @@ protected void initChannel(Channel ch) throws Exception {
             @Override
             protected void initChannel(Channel ch) throws Exception {
                 ChannelPipeline p = ch.pipeline();
-                p.addLast(new Http2ConnectionHandler(false, clientListener));
+                p.addLast(new Http2ConnectionHandler(false, clientListener, false));
             }
         });
 
@@ -513,7 +513,7 @@ private ChannelPromise newPromise() {
     }
 
     private static Http2Headers dummyHeaders() {
-        return new DefaultHttp2Headers().method(new AsciiString("GET")).scheme(new AsciiString("https"))
+        return new DefaultHttp2Headers(false).method(new AsciiString("GET")).scheme(new AsciiString("https"))
         .authority(new AsciiString("example.org")).path(new AsciiString("/some/path/resource2"))
         .add(randomString(), randomString());
     }

diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameRoundtripTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameRoundtripTest.java
@@ -398,7 +398,7 @@ private ChannelPromise newPromise() {
     }
 
     private static Http2Headers headers() {
-        return new DefaultHttp2Headers().method(new AsciiString("GET")).scheme(new AsciiString("https"))
+        return new DefaultHttp2Headers(false).method(new AsciiString("GET")).scheme(new AsciiString("https"))
                 .authority(new AsciiString("example.org")).path(new AsciiString("/some/path/resource2"))
                 .add(randomString(), randomString());
     }

diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2HeaderBlockIOTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2HeaderBlockIOTest.java
@@ -37,7 +37,7 @@ public class Http2HeaderBlockIOTest {
     @Before
     public void setup() {
         encoder = new DefaultHttp2HeadersEncoder();
-        decoder = new DefaultHttp2HeadersDecoder();
+        decoder = new DefaultHttp2HeadersDecoder(false);
         buffer = Unpooled.buffer();
     }
 
@@ -54,21 +54,18 @@ public void roundtripShouldBeSuccessful() throws Http2Exception {
 
     @Test
     public void successiveCallsShouldSucceed() throws Http2Exception {
-        Http2Headers in =
-                new DefaultHttp2Headers().method(new AsciiString("GET")).scheme(new AsciiString("https"))
+        Http2Headers in = new DefaultHttp2Headers().method(new AsciiString("GET")).scheme(new AsciiString("https"))
                         .authority(new AsciiString("example.org")).path(new AsciiString("/some/path"))
                         .add(new AsciiString("accept"), new AsciiString("*/*"));
         assertRoundtripSuccessful(in);
 
-        in =
-                new DefaultHttp2Headers().method(new AsciiString("GET")).scheme(new AsciiString("https"))
+        in = new DefaultHttp2Headers().method(new AsciiString("GET")).scheme(new AsciiString("https"))
                         .authority(new AsciiString("example.org")).path(new AsciiString("/some/path/resource1"))
                         .add(new AsciiString("accept"), new AsciiString("image/jpeg"))
                         .add(new AsciiString("cache-control"), new AsciiString("no-cache"));
         assertRoundtripSuccessful(in);
 
-        in =
-                new DefaultHttp2Headers().method(new AsciiString("GET")).scheme(new AsciiString("https"))
+        in = new DefaultHttp2Headers().method(new AsciiString("GET")).scheme(new AsciiString("https"))
                         .authority(new AsciiString("example.org")).path(new AsciiString("/some/path/resource2"))
                         .add(new AsciiString("accept"), new AsciiString("image/png"))
                         .add(new AsciiString("cache-control"), new AsciiString("no-cache"));
@@ -91,7 +88,7 @@ private void assertRoundtripSuccessful(Http2Headers in) throws Http2Exception {
     }
 
     private static Http2Headers headers() {
-        return new DefaultHttp2Headers().method(new AsciiString("GET")).scheme(new AsciiString("https"))
+        return new DefaultHttp2Headers(false).method(new AsciiString("GET")).scheme(new AsciiString("https"))
         .authority(new AsciiString("example.org")).path(new AsciiString("/some/path/resource2"))
                 .add(new AsciiString("accept"), new AsciiString("image/png"))
                 .add(new AsciiString("cache-control"), new AsciiString("no-cache"))

diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2TestUtil.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2TestUtil.java
@@ -89,7 +89,7 @@ static class FrameAdapter extends ByteToMessageDecoder {
         }
 
         FrameAdapter(Http2Connection connection, Http2FrameListener listener, CountDownLatch latch) {
-            this(connection, new DefaultHttp2FrameReader(), listener, latch);
+            this(connection, new DefaultHttp2FrameReader(false), listener, latch);
         }
 
         FrameAdapter(Http2Connection connection, DefaultHttp2FrameReader reader, Http2FrameListener listener,
-Original file line number
+Diff line change
@@ Expand Up / @@ -40,7 +40,7 @@ public class DefaultHttp2HeadersDecoderTest { @@
         @Before
         public void setup() {
-            decoder = new DefaultHttp2HeadersDecoder();
+            decoder = new DefaultHttp2HeadersDecoder(false);
         }
         @Test
@@ Expand Down @@