Feature/strict validation for header name and value

.github/workflows/maven.yml

@@ -32,9 +32,13 @@ jobs:
       with:
         distribution: 'temurin'
         java-version: ${{ matrix.java_version }}
+    - name: Install Maven
+      run: |
+        curl -o ./apache-maven-3.9.9-bin.tar.gz https://dlcdn.apache.org/maven/maven-3/3.9.9/binaries/apache-maven-3.9.9-bin.tar.gz
+        tar -xf ./apache-maven-3.9.9-bin.tar.gz
     - name: Maven Build
       run: |
-        mvn --show-version \
+        ./apache-maven-3.9.9/bin/mvn --show-version \
             --no-transfer-progress \
             --activate-profiles staging \
             --define skipTests=true \
@@ -57,9 +61,13 @@ jobs:
       with:
         distribution: 'temurin'
         java-version: ${{ matrix.java_version }}
+    - name: Install Maven
+      run: |
+        curl -o ./apache-maven-3.9.9-bin.tar.gz https://dlcdn.apache.org/maven/maven-3/3.9.9/binaries/apache-maven-3.9.9-bin.tar.gz
+        tar -xf ./apache-maven-3.9.9-bin.tar.gz
     - name: Maven Build
       run: |
-        mvn --show-version \
+        ./apache-maven-3.9.9/bin/mvn --show-version \
             --no-transfer-progress \
             --fail-at-end \
             --activate-profiles staging \

modules/comet/src/test/java/org/glassfish/grizzly/comet/BasicCometTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2020 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2009, 2024 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0, which is available at
@@ -105,7 +105,7 @@ public void testClientCloseConnection() throws Exception {
         s.setSoLinger(false, 0);
         s.setSoTimeout(500);
         OutputStream os = s.getOutputStream();
-        String a = "GET " + alias + " HTTP/1.1\n" + "Host: localhost:" + PORT + "\n\n";
+        String a = "GET " + alias + " HTTP/1.1\n" + "Host: localhost:" + PORT + "\r\n\n";
         System.out.println("     " + a);
         os.write(a.getBytes());
         os.flush();
@@ -167,10 +167,10 @@ public void service(Request request, Response response) throws Exception {
         Socket s = new Socket("localhost", PORT);
         s.setSoTimeout(10 * 1000);
         OutputStream os = s.getOutputStream();
-        String cometRequest = "GET " + alias + " HTTP/1.1\nHost: localhost:" + PORT + "\n\n";
-        String staticRequest = "GET /static HTTP/1.1\nHost: localhost:" + PORT + "\n\n";
+        String cometRequest = "GET " + alias + " HTTP/1.1\nHost: localhost:" + PORT + "\r\n\n";
+        String staticRequest = "GET /static HTTP/1.1\nHost: localhost:" + PORT + "\r\n\n";
 
-        String lastCometRequest = "GET " + alias + " HTTP/1.1\n" + "Host: localhost:" + PORT + "\nConnection: close\n\n";
+        String lastCometRequest = "GET " + alias + " HTTP/1.1\n" + "Host: localhost:" + PORT + "\r\nConnection: close\r\n\n";
 
         String pipelinedRequest1 = cometRequest + staticRequest + cometRequest;
         String pipelinedRequest2 = cometRequest + staticRequest + lastCometRequest;
@@ -256,8 +256,8 @@ public void service(Request request, Response response) throws Exception {
         Socket s = new Socket("localhost", PORT);
         s.setSoTimeout(10 * 1000);
         OutputStream os = s.getOutputStream();
-        String cometRequest = "GET " + alias + " HTTP/1.1\nHost: localhost:" + PORT + "\n\n";
-        String staticRequest = "GET /static HTTP/1.1\nHost: localhost:" + PORT + "\n\n";
+        String cometRequest = "GET " + alias + " HTTP/1.1\nHost: localhost:" + PORT + "\r\n\n";
+        String staticRequest = "GET /static HTTP/1.1\nHost: localhost:" + PORT + "\r\n\n";
 
         try {
             os.write(cometRequest.getBytes());

modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2020 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2024 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0, which is available at
@@ -37,6 +37,7 @@
 import org.glassfish.grizzly.http.util.ByteChunk;
 import org.glassfish.grizzly.http.util.CacheableDataChunk;
 import org.glassfish.grizzly.http.util.Constants;
+import org.glassfish.grizzly.http.util.CookieHeaderParser;
 import org.glassfish.grizzly.http.util.DataChunk;
 import org.glassfish.grizzly.http.util.Header;
 import org.glassfish.grizzly.http.util.MimeHeaders;
@@ -108,6 +109,14 @@ public abstract class HttpCodecFilter extends HttpBaseFilter implements Monitori
      * @see #setRemoveHandledContentEncodingHeaders
      */
     private boolean removeHandledContentEncodingHeaders = false;
+
+    public static final String STRICT_HEADER_NAME_VALIDATION_RFC_9110 = "org.glassfish.grizzly.http.STRICT_HEADER_NAME_VALIDATION_RFC_9110";
+
+    public static final String STRICT_HEADER_VALUE_VALIDATION_RFC_9110 = "org.glassfish.grizzly.http.STRICT_HEADER_VALUE_VALIDATION_RFC_9110";
+
+    private final boolean isStrictHeaderNameValidationSet = Boolean.parseBoolean(System.getProperty(STRICT_HEADER_NAME_VALIDATION_RFC_9110));
+
+    private final boolean isStrictHeaderValueValidationSet = Boolean.parseBoolean(System.getProperty(STRICT_HEADER_VALUE_VALIDATION_RFC_9110));
 
     /**
      * File cache probes
@@ -707,8 +716,13 @@ protected boolean parseHeaderFromBytes(final HttpHeader httpHeader, final MimeHe
                 parsingState.subState++;
             }
             case 1: { // parse header name
-                if (!parseHeaderName(httpHeader, mimeHeaders, parsingState, input, end)) {
+                final int result = parseHeaderName(httpHeader, mimeHeaders, parsingState, input, end);
+                if (result == -1) {
                     return false;
+                } else if (result == -2) { // EOL. ignore field-lines
+                    parsingState.subState = 0;
+                    parsingState.start = -1;
+                    return true;
                 }
 
                 parsingState.subState++;
@@ -754,7 +768,7 @@ protected boolean parseHeaderFromBytes(final HttpHeader httpHeader, final MimeHe
         }
     }
 
-    protected boolean parseHeaderName(final HttpHeader httpHeader, final MimeHeaders mimeHeaders, final HeaderParsingState parsingState, final byte[] input,
+    protected int parseHeaderName(final HttpHeader httpHeader, final MimeHeaders mimeHeaders, final HeaderParsingState parsingState, final byte[] input,
             final int end) {
         final int arrayOffs = parsingState.arrayOffset;
 
@@ -770,22 +784,36 @@ protected boolean parseHeaderName(final HttpHeader httpHeader, final MimeHeaders
                 parsingState.offset = offset + 1 - arrayOffs;
                 finalizeKnownHeaderNames(httpHeader, parsingState, input, start, offset);
 
-                return true;
+                return 0;
             } else if (b >= Constants.A && b <= Constants.Z) {
                 if (!preserveHeaderCase) {
                     b -= Constants.LC_OFFSET;
                 }
                 input[offset] = b;
+            } else if (isStrictHeaderNameValidationSet && b == Constants.CR) {
+                parsingState.offset = offset - arrayOffs;
+                final int eol = checkEOL(parsingState, input, end);
+                if (eol == 0) { // EOL
+                    // the offset is already increased in the check
+                    return -2;
+                } else if (eol == -2) { // not enough data
+                    // by keeping the offset unchanged, we will recheck the EOL at the next opportunity.
+                    break;
+                }
             }
 
+            if (isStrictHeaderNameValidationSet && !CookieHeaderParser.isToken(b)) {
+                throw new IllegalStateException(
+                        "An invalid character 0x" + Integer.toHexString(b) + " was found in the header name");
+            }
             offset++;
         }
 
         parsingState.offset = offset - arrayOffs;
-        return false;
+        return -1;
     }
 
-    protected static int parseHeaderValue(final HttpHeader httpHeader, final HeaderParsingState parsingState, final byte[] input, final int end) {
+    protected int parseHeaderValue(final HttpHeader httpHeader, final HeaderParsingState parsingState, final byte[] input, final int end) {
 
         final int arrayOffs = parsingState.arrayOffset;
         final int limit = Math.min(end, arrayOffs + parsingState.packetLimit);
@@ -797,24 +825,41 @@ protected static int parseHeaderValue(final HttpHeader httpHeader, final HeaderP
         while (offset < limit) {
             final byte b = input[offset];
             if (b == Constants.CR) {
-            } else if (b == Constants.LF) {
-                // Check if it's not multi line header
-                if (offset + 1 < limit) {
-                    final byte b2 = input[offset + 1];
-                    if (b2 == Constants.SP || b2 == Constants.HT) {
-                        input[arrayOffs + parsingState.checkpoint++] = b2;
-                        parsingState.offset = offset + 2 - arrayOffs;
-                        return -2;
-                    } else {
-                        parsingState.offset = offset + 1 - arrayOffs;
-                        finalizeKnownHeaderValues(httpHeader, parsingState, input, arrayOffs + parsingState.start, arrayOffs + parsingState.checkpoint2);
-                        parsingState.headerValueStorage.setBytes(input, arrayOffs + parsingState.start, arrayOffs + parsingState.checkpoint2);
+                if (isStrictHeaderValueValidationSet) {
+                    parsingState.offset = offset - arrayOffs;
+                    final int eol = checkEOL(parsingState, input, end);
+                    if (eol == 0) { // EOL
+                        // the offset is already increased in the check
+                        finalizeKnownHeaderValues(httpHeader, parsingState, input, arrayOffs + parsingState.start,
+                                                  arrayOffs + parsingState.checkpoint2);
+                        parsingState.headerValueStorage.setBytes(input, arrayOffs + parsingState.start,
+                                                                 arrayOffs + parsingState.checkpoint2);
                         return 0;
+                    } else if (eol == -2) { // not enough data
+                        // by keeping the offset unchanged, we will recheck the EOL at the next opportunity.
+                        return -1;
                     }
                 }
-
-                parsingState.offset = offset - arrayOffs;
-                return -1;
+            } else if (b == Constants.LF) {
+                if (!isStrictHeaderValueValidationSet) {
+                    // Check if it's not multi line header
+                    if (offset + 1 < limit) {
+                        final byte b2 = input[offset + 1];
+                        if (b2 == Constants.SP || b2 == Constants.HT) {
+                            input[arrayOffs + parsingState.checkpoint++] = b2;
+                            parsingState.offset = offset + 2 - arrayOffs;
+                            return -2;
+                        } else {
+                            parsingState.offset = offset + 1 - arrayOffs;
+                            finalizeKnownHeaderValues(httpHeader, parsingState, input, arrayOffs + parsingState.start, arrayOffs + parsingState.checkpoint2);
+                            parsingState.headerValueStorage.setBytes(input, arrayOffs + parsingState.start, arrayOffs + parsingState.checkpoint2);
+                            return 0;
+                        }
+                    }
+                    // not enough data
+                    parsingState.offset = offset - arrayOffs;
+                    return -1;
+                }
             } else if (b == Constants.SP) {
                 if (hasShift) {
                     input[arrayOffs + parsingState.checkpoint++] = b;
@@ -830,6 +875,10 @@ protected static int parseHeaderValue(final HttpHeader httpHeader, final HeaderP
                 parsingState.checkpoint2 = parsingState.checkpoint;
             }
 
+            if (isStrictHeaderValueValidationSet && !CookieHeaderParser.isText(b)) {
+                throw new IllegalStateException(
+                        "An invalid character 0x" + Integer.toHexString(b) + " was found in the header value");
+            }
             offset++;
         }
         parsingState.offset = offset - arrayOffs;
@@ -963,8 +1012,13 @@ protected boolean parseHeaderFromBuffer(final HttpHeader httpHeader, final MimeH
                 parsingState.subState++;
             }
             case 1: { // parse header name
-                if (!parseHeaderName(httpHeader, mimeHeaders, parsingState, input)) {
+                final int result = parseHeaderName(httpHeader, mimeHeaders, parsingState, input);
+                if (result == -1) {
                     return false;
+                } else if (result == -2) { // EOL. ignore field-lines
+                    parsingState.subState = 0;
+                    parsingState.start = -1;
+                    return true;
                 }
 
                 parsingState.subState++;
@@ -1010,7 +1064,7 @@ protected boolean parseHeaderFromBuffer(final HttpHeader httpHeader, final MimeH
         }
     }
 
-    protected boolean parseHeaderName(final HttpHeader httpHeader, final MimeHeaders mimeHeaders, final HeaderParsingState parsingState, final Buffer input) {
+    protected int parseHeaderName(final HttpHeader httpHeader, final MimeHeaders mimeHeaders, final HeaderParsingState parsingState, final Buffer input) {
         final int limit = Math.min(input.limit(), parsingState.packetLimit);
         final int start = parsingState.start;
         int offset = parsingState.offset;
@@ -1023,22 +1077,36 @@ protected boolean parseHeaderName(final HttpHeader httpHeader, final MimeHeaders
                 parsingState.offset = offset + 1;
                 finalizeKnownHeaderNames(httpHeader, parsingState, input, start, offset);
 
-                return true;
+                return 0;
             } else if (b >= Constants.A && b <= Constants.Z) {
                 if (!preserveHeaderCase) {
                     b -= Constants.LC_OFFSET;
                 }
                 input.put(offset, b);
+            } else if (isStrictHeaderNameValidationSet && b == Constants.CR) {
+                parsingState.offset = offset;
+                final int eol = checkEOL(parsingState, input);
+                if (eol == 0) { // EOL
+                    // the offset is already increased in the check
+                    return -2;
+                } else if (eol == -2) { // not enough data
+                    // by keeping the offset unchanged, we will recheck the EOL at the next opportunity.
+                    break;
+                }
             }
 
+            if (isStrictHeaderNameValidationSet && !CookieHeaderParser.isToken(b)) {
+                throw new IllegalStateException(
+                        "An invalid character 0x" + Integer.toHexString(b) + " was found in the header name");
+            }
             offset++;
         }
 
         parsingState.offset = offset;
-        return false;
+        return -1;
     }
 
-    protected static int parseHeaderValue(final HttpHeader httpHeader, final HeaderParsingState parsingState, final Buffer input) {
+    protected int parseHeaderValue(final HttpHeader httpHeader, final HeaderParsingState parsingState, final Buffer input) {
 
         final int limit = Math.min(input.limit(), parsingState.packetLimit);
 
@@ -1049,24 +1117,40 @@ protected static int parseHeaderValue(final HttpHeader httpHeader, final HeaderP
         while (offset < limit) {
             final byte b = input.get(offset);
             if (b == Constants.CR) {
-            } else if (b == Constants.LF) {
-                // Check if it's not multi line header
-                if (offset + 1 < limit) {
-                    final byte b2 = input.get(offset + 1);
-                    if (b2 == Constants.SP || b2 == Constants.HT) {
-                        input.put(parsingState.checkpoint++, b2);
-                        parsingState.offset = offset + 2;
-                        return -2;
-                    } else {
-                        parsingState.offset = offset + 1;
-                        finalizeKnownHeaderValues(httpHeader, parsingState, input, parsingState.start, parsingState.checkpoint2);
+                if (isStrictHeaderValueValidationSet) {
+                    parsingState.offset = offset;
+                    final int eol = checkEOL(parsingState, input);
+                    if (eol == 0) { // EOL
+                        // the offset is already increased in the check
+                        finalizeKnownHeaderValues(httpHeader, parsingState, input, parsingState.start,
+                                                  parsingState.checkpoint2);
                         parsingState.headerValueStorage.setBuffer(input, parsingState.start, parsingState.checkpoint2);
                         return 0;
+                    } else if (eol == -2) { // not enough data
+                        // by keeping the offset unchanged, we will recheck the EOL at the next opportunity.
+                        return -1;
                     }
                 }
-
-                parsingState.offset = offset;
-                return -1;
+            } else if (b == Constants.LF) {
+                if (!isStrictHeaderValueValidationSet) {
+                    // Check if it's not multi line header
+                    if (offset + 1 < limit) {
+                        final byte b2 = input.get(offset + 1);
+                        if (b2 == Constants.SP || b2 == Constants.HT) {
+                            input.put(parsingState.checkpoint++, b2);
+                            parsingState.offset = offset + 2;
+                            return -2;
+                        } else {
+                            parsingState.offset = offset + 1;
+                            finalizeKnownHeaderValues(httpHeader, parsingState, input, parsingState.start, parsingState.checkpoint2);
+                            parsingState.headerValueStorage.setBuffer(input, parsingState.start, parsingState.checkpoint2);
+                            return 0;
+                        }
+                    }
+                    // not enough data
+                    parsingState.offset = offset;
+                    return -1;
+                }
             } else if (b == Constants.SP) {
                 if (hasShift) {
                     input.put(parsingState.checkpoint++, b);
@@ -1082,6 +1166,10 @@ protected static int parseHeaderValue(final HttpHeader httpHeader, final HeaderP
                 parsingState.checkpoint2 = parsingState.checkpoint;
             }
 
+            if (isStrictHeaderValueValidationSet && !CookieHeaderParser.isText(b)) {
+                throw new IllegalStateException(
+                        "An invalid character 0x" + Integer.toHexString(b) + " was found in the header value");
+            }
             offset++;
         }
         parsingState.offset = offset;

modules/http/src/main/java/org/glassfish/grizzly/http/util/CookieHeaderParser.java

@@ -236,6 +236,10 @@ private static ByteBuffer readToken(ByteBuffer byteBuffer) {
     }
 
     public static boolean isToken(int c) {
+        if (c < 0 || c >= ARRAY_SIZE) {
+            // out of bounds
+            return false;
+        }
         // Fast for correct values, slower for incorrect ones
         try {
             return IS_TOKEN[c];
@@ -244,6 +248,19 @@ public static boolean isToken(int c) {
         }
     }
 
+    public static boolean isText(int c) {
+        if (c < 0 || c >= 256) {
+            // out of bounds
+            return false;
+        }
+        // Fast for correct values, slower for incorrect ones
+        try {
+            return isText[c];
+        } catch (ArrayIndexOutOfBoundsException ex) {
+            return false;
+        }
+    }
+
 
     /**
      * Custom implementation that skips many of the safety checks in