Disallow space after header name and colon (#901)

Motivation:
https://tools.ietf.org/html/rfc7230#section-3.2.4 specifies that spaces are
prohibited between the end of the header-name and the colon. Our header parsing
is currently liberal and allows a space. This may leave us vulnerable to HTTP
request smuggling.

Modifications:
- HttpObjectDecoder should prevent white space before the colon when paring
  header-name field

Result:
HTTP/1.x header parsing follows
https://tools.ietf.org/html/rfc7230#section-3.2.4

servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/HttpObjectDecoder.java

@@ -545,6 +545,12 @@ private void parseHeaderLine(HttpHeaders headers, ByteBuf buffer, final int lfIn
         //      obs-fold       = CRLF 1*( SP / HTAB )
         //                     ; obsolete line folding
         //                     ; see Section 3.2.4
+        //      OWS            = *( SP / HTAB )
+        //                     ; optional whitespace
+        // https://tools.ietf.org/html/rfc7230#section-3.2.4
+        // No whitespace is allowed between the header field-name and colon.  In
+        //    the past, differences in the handling of such whitespace have led to
+        //    security vulnerabilities in request routing and response handling.
         final int nonControlIndex = lfIndex - 2;
         int headerStart = buffer.forEachByte(buffer.readerIndex(), nonControlIndex - buffer.readerIndex(),
                 FIND_NON_LINEAR_WHITESPACE);
@@ -558,15 +564,13 @@ private void parseHeaderLine(HttpHeaders headers, ByteBuf buffer, final int lfIn
             throw new IllegalArgumentException("unable to find end of header name");
         }
 
+        if (buffer.getByte(headerEnd) != COLON_BYTE) {
+            throw new IllegalArgumentException("No whitespace is allowed between the header field-name and colon.");
+        }
+
         int valueStart = headerEnd + 1;
         // We assume the allocator will not leak memory, and so we retain + slice to avoid copying data.
         CharSequence name = newAsciiString(newBufferFrom(buffer.retainedSlice(headerStart, headerEnd - headerStart)));
-        if (buffer.getByte(headerEnd) != COLON_BYTE) {
-            valueStart = buffer.forEachByte(headerEnd + 1, nonControlIndex - headerEnd, FIND_COLON) + 1;
-            if (valueStart < 0) {
-                throw new IllegalArgumentException("unable to find colon");
-            }
-        }
         if (nonControlIndex < valueStart) {
             headers.add(name, emptyAsciiString());
         } else {

servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/HttpRequestDecoderTest.java

@@ -101,9 +101,9 @@ public void contentLengthNoTrailersHeaderWhiteSpace() {
         byte[] content = new byte[128];
         ThreadLocalRandom.current().nextBytes(content);
         byte[] beforeContentBytes = ("GET /some/path?foo=bar&baz=yyy HTTP/1.1" + "\r\n" +
-                " Connection :  keep-alive " + "\r\n" +
-                "  User-Agent  :        unit-test        " + "\r\n" +
-                "   Content-Length  : " + content.length + "\r\n" + "\r\n").getBytes(US_ASCII);
+                " Connection:  keep-alive " + "\r\n" +
+                "  User-Agent:        unit-test        " + "\r\n" +
+                "   Content-Length: " + content.length + "\r\n" + "\r\n").getBytes(US_ASCII);
         assertTrue(channel.writeInbound(wrappedBuffer(beforeContentBytes)));
         assertTrue(channel.writeInbound(wrappedBuffer(content)));
 
@@ -134,8 +134,8 @@ public void contentLengthNoTrailersHeaderMixedWhiteSpace() {
         byte[] content = new byte[128];
         ThreadLocalRandom.current().nextBytes(content);
         byte[] beforeContentBytes = ("GET /some/path?foo=bar&baz=yyy HTTP/1.1" + "\r\n" +
-                "Connection   :keep-alive" + "\r\n" +
-                "   User-Agent   :unit-test" + "\r\n" +
+                "Connection:keep-alive" + "\r\n" +
+                "   User-Agent:unit-test" + "\r\n" +
                 "Empty:" + "\r\n" +
                 "EmptyWhitespace:   " + "\r\n" +
                 "SingleCharacterNoWhiteSpace: a" + "\r\n" +
@@ -329,6 +329,21 @@ public void variableNoTrailersWithInvalidContent() {
         channel.writeInbound(wrappedBuffer(content));
     }
 
+    @Test(expected = DecoderException.class)
+    public void testWhitespaceNotAllowedBetweenHeaderFieldNameAndColon() {
+        EmbeddedChannel channel = newEmbeddedChannel();
+        try {
+            byte[] beforeContentBytes = ("GET /some/path HTTP/1.1\r\n" +
+                    "Transfer-Encoding : chunked\r\n" +
+                    "Host: servicetalk.io\r\n\r\n").getBytes(US_ASCII);
+
+            assertTrue(channel.writeInbound(wrappedBuffer(beforeContentBytes)));
+            channel.readInbound();
+        } finally {
+            channel.finishAndReleaseAll();
+        }
+    }
+
     @Test(expected = DecoderException.class)
     public void variableWithTrailers() {
         EmbeddedChannel channel = newEmbeddedChannel();

servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/HttpResponseDecoderTest.java

@@ -108,9 +108,9 @@ public void contentLengthNoTrailersHeaderWhiteSpace() {
         byte[] content = new byte[128];
         ThreadLocalRandom.current().nextBytes(content);
         byte[] beforeContentBytes = ("HTTP/1.1 200 OK" + "\r\n" +
-                " Connection :  keep-alive " + "\r\n" +
-                "  Server  :        unit-test        " + "\r\n" +
-                "   Content-Length  : " + content.length + "\r\n" + "\r\n").getBytes(US_ASCII);
+                " Connection:  keep-alive " + "\r\n" +
+                "  Server:        unit-test        " + "\r\n" +
+                "   Content-Length: " + content.length + "\r\n" + "\r\n").getBytes(US_ASCII);
         assertTrue(channel.writeInbound(wrappedBuffer(beforeContentBytes)));
         assertTrue(channel.writeInbound(wrappedBuffer(content)));
 
@@ -141,8 +141,8 @@ public void contentLengthNoTrailersHeaderMixedWhiteSpace() {
         byte[] content = new byte[128];
         ThreadLocalRandom.current().nextBytes(content);
         byte[] beforeContentBytes = ("HTTP/1.1 200 OK" + "\r\n" +
-                "Connection   :keep-alive" + "\r\n" +
-                "   Server   :unit-test" + "\r\n" +
+                "Connection:keep-alive" + "\r\n" +
+                "   Server:unit-test" + "\r\n" +
                 "Empty:" + "\r\n" +
                 "EmptyWhitespace:   " + "\r\n" +
                 "SingleCharacterNoWhiteSpace: a" + "\r\n" +
@@ -353,6 +353,21 @@ public void variableNoTrailersNoContent() {
         assertFalse(channel.finishAndReleaseAll());
     }
 
+    @Test(expected = DecoderException.class)
+    public void testWhitespaceNotAllowedBetweenHeaderFieldNameAndColon() {
+        EmbeddedChannel channel = newEmbeddedChannel();
+        try {
+            byte[] beforeContentBytes = ("HTTP/1.1 200 OK\r\n" +
+                    "Transfer-Encoding : chunked\r\n" +
+                    "Host: servicetalk.io\r\n\r\n").getBytes(US_ASCII);
+
+            assertTrue(channel.writeInbound(wrappedBuffer(beforeContentBytes)));
+            channel.readInbound();
+        } finally {
+            channel.finishAndReleaseAll();
+        }
+    }
+
     @Test
     public void variableWithTrailers() {
         EmbeddedChannel channel = newEmbeddedChannel();