Zero length HTTP header names are not valid (not valid tokens)

apache · May 2, 2023 · ed6d4cc · ed6d4cc
1 parent ca66133
commit ed6d4cc
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 6 deletions.
diff --git a/java/org/apache/coyote/http11/Http11InputBuffer.java b/java/org/apache/coyote/http11/Http11InputBuffer.java
@@ -50,8 +50,8 @@ public class Http11InputBuffer implements InputBuffer, ApplicationBufferHandler
     private static final StringManager sm = StringManager.getManager(Http11InputBuffer.class);
 
 
-    private static final byte[] CLIENT_PREFACE_START = "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n"
-            .getBytes(StandardCharsets.ISO_8859_1);
+    private static final byte[] CLIENT_PREFACE_START =
+            "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n".getBytes(StandardCharsets.ISO_8859_1);
 
     /**
      * Associated Coyote request.
@@ -883,6 +883,11 @@ private HeaderParseStatus parseHeader() throws IOException {
             int pos = byteBuffer.position();
             chr = byteBuffer.get();
             if (chr == Constants.COLON) {
+                if (headerData.start == pos) {
+                    // Zero length header name - not valid.
+                    // skipLine() will handle the error
+                    return skipLine(false);
+                }
                 headerParsePos = HeaderParsePosition.HEADER_VALUE_START;
                 headerData.headerValue = headers.addValue(byteBuffer.array(), headerData.start, pos - headerData.start);
                 pos = byteBuffer.position();
@@ -1064,12 +1069,13 @@ private HeaderParseStatus skipLine(boolean deleteHeader) throws IOException {
             }
         }
         if (rejectThisHeader || log.isDebugEnabled()) {
-            String message = sm.getString("iib.invalidheader", HeaderUtil.toPrintableString(byteBuffer.array(),
-                    headerData.lineStart, headerData.lastSignificantChar - headerData.lineStart + 1));
             if (rejectThisHeader) {
-                throw new IllegalArgumentException(message);
+                throw new IllegalArgumentException(
+                        sm.getString("iib.invalidheader.reject", HeaderUtil.toPrintableString(byteBuffer.array(),
+                                headerData.lineStart, headerData.lastSignificantChar - headerData.lineStart + 1)));
             }
-            log.debug(message);
+            log.debug(sm.getString("iib.invalidheader", HeaderUtil.toPrintableString(byteBuffer.array(),
+                    headerData.lineStart, headerData.lastSignificantChar - headerData.lineStart + 1)));
         }
 
         headerParsePos = HeaderParsePosition.HEADER_START;

diff --git a/java/org/apache/coyote/http11/LocalStrings.properties b/java/org/apache/coyote/http11/LocalStrings.properties
@@ -47,6 +47,7 @@ iib.invalidHttpProtocol=Invalid character found in the HTTP protocol [{0}]
 iib.invalidPhase=Invalid request line parse phase [{0}]
 iib.invalidRequestTarget=Invalid character found in the request target [{0}]. The valid characters are defined in RFC 7230 and RFC 3986
 iib.invalidheader=The HTTP header line [{0}] does not conform to RFC 7230 and has been ignored.
+iib.invalidheader.reject=The HTTP header line [{0}] does not conform to RFC 7230. The request has been rejected.
 iib.invalidmethod=Invalid character found in method name [{0}]. HTTP method names must be tokens
 iib.parseheaders.ise.error=Unexpected state: headers already parsed. Buffer not recycled?
 iib.readtimeout=Timeout attempting to read data from the socket

diff --git a/test/org/apache/coyote/http11/TestHttp11InputBuffer.java b/test/org/apache/coyote/http11/TestHttp11InputBuffer.java
@@ -732,4 +732,48 @@ public boolean isResponseBodyOK() {
             return true;
         }
     }
+
+
+    private static final class Client extends SimpleHttpClient {
+
+        Client(int port) {
+            setPort(port);
+        }
+
+        @Override
+        public boolean isResponseBodyOK() {
+            return getResponseBody().contains("test - data");
+        }
+    }
+
+
+    @Test
+    public void testInvalidHeader02() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        // This setting means the connection will be closed at the end of the
+        // request
+        Assert.assertTrue(tomcat.getConnector().setProperty("maxKeepAliveRequests", "1"));
+
+        // No file system docBase required
+        Context ctx = tomcat.addContext("", null);
+
+        // Add servlet
+        Tomcat.addServlet(ctx, "TesterServlet", new TesterServlet());
+        ctx.addServletMappingDecoded("/foo", "TesterServlet");
+
+        tomcat.start();
+
+        String request = "GET /foo HTTP/1.1" + SimpleHttpClient.CRLF + "Host: localhost" + SimpleHttpClient.CRLF + ":b" +
+                SimpleHttpClient.CRLF + "X-Dummy:b" + SimpleHttpClient.CRLF + SimpleHttpClient.CRLF;
+
+        Client client = new Client(tomcat.getConnector().getLocalPort());
+        client.setRequest(new String[] { request });
+
+        client.connect(10000, 600000);
+        client.processRequest();
+
+        // Expected response is a 400 response.
+        Assert.assertTrue(client.getResponseLine(), client.isResponse400());
+    }
 }
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
@@ -142,6 +142,10 @@
         Add support for a new character set, <code>gb18030-2022</code> -
         introduced in Java 21, to the character set caching mechanism. (markt)
       </add>
+      <fix>
+        Fix an edge case in HTTP header parsing and ensure that HTTP headers
+        without names are treated as invalid. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">