From ed2eba1352b223279b0d9a62f7df8175c68899ab Mon Sep 17 00:00:00 2001 From: Bongjae Chang Date: Sat, 14 Sep 2024 21:33:03 +0900 Subject: [PATCH 1/7] Issue #2212 Enhances validation of HTTP header names --- .../grizzly/http/HttpCodecFilter.java | 57 ++++++++++++++++--- .../grizzly/http/HttpRequestParseTest.java | 55 +++++++++++++++++- 2 files changed, 100 insertions(+), 12 deletions(-) diff --git a/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java b/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java index 49bdd6be29..bc5061c4af 100644 --- a/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java +++ b/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; @@ -707,8 +708,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 +760,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,19 +776,33 @@ 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 (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 (!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) { @@ -963,8 +983,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 +1035,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,19 +1048,33 @@ 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 (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 (!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) { diff --git a/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java b/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java index 79c10e622a..6d81a74e97 100644 --- a/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java +++ b/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.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 @@ -81,6 +81,42 @@ public void testSimpleHeadersPreserveCase() throws Exception { doHttpRequestTest("POST", "/index.html", "HTTP/1.1", headers, "\r\n", true); } + public void testDisallowedHeaders() { + final StringBuilder sb = new StringBuilder("GET / HTTP/1.1\r\n"); + sb.append("Host: localhost\r\n"); + sb.append(new char[]{0x00, 0x01, 0x02, '\t', '\n', '\r', ' ', '\"', '(', ')', '/', ';', '<', '=', '>', '?', '@', + '[', 0x5c, ']', '{', '}'}).append(": some-value\r\n"); + sb.append("\r\n"); + try { + doTestDecoder(sb.toString(), 128); + fail("Bad HTTP headers exception had to be thrown"); + } catch (IllegalStateException e) { + // expected + } + try { + doTestDecoder("GET /index.html HTTP/1.1\nHost: localhost\nContent -Length: 1234\n\n", 128); + fail("Bad HTTP headers exception had to be thrown"); + } catch (IllegalStateException e) { + // expected + } + try { + doTestDecoder("GET /index.html HTTP/1.1\nHost: localhost\nContent-\rLength: 1234\n\n", 128); + fail("Bad HTTP headers exception had to be thrown"); + } catch (IllegalStateException e) { + // expected + } + } + + public void testIgnoredHeaders() throws Exception { + final Map> headers = new HashMap<>(); + headers.put("Host", new Pair<>("localhost", "localhost")); + headers.put("Ignore\r\nContent-length", new Pair<>("2345", "2345")); + final Map> expectedHeaders = new HashMap<>(); + expectedHeaders.put("Host", new Pair<>("localhost", "localhost")); + expectedHeaders.put("Content-length", new Pair<>("2345", "2345")); + doHttpRequestTest("POST", "/index.html", "HTTP/1.1", headers, expectedHeaders, "\r\n"); + } + public void testMultiLineHeaders() throws Exception { Map> headers = new HashMap<>(); headers.put("Host", new Pair<>("localhost", "localhost")); @@ -204,16 +240,29 @@ private void doHttpRequestTest(String method, String requestURI, String protocol new Pair<>(protocol, protocol), headers, eol, false); } + private void doHttpRequestTest(String method, String requestURI, String protocol, + Map> headers, + Map> expectedHeaders, String eol) throws Exception { + doHttpRequestTest(new Pair<>(method, method), new Pair<>(requestURI, requestURI), + new Pair<>(protocol, protocol), headers, expectedHeaders, eol, false); + } + private void doHttpRequestTest(String method, String requestURI, String protocol, Map> headers, String eol, boolean preserveCase) throws Exception { doHttpRequestTest(new Pair<>(method, method), new Pair<>(requestURI, requestURI), new Pair<>(protocol, protocol), headers, eol, preserveCase); } - @SuppressWarnings("unchecked") private void doHttpRequestTest(Pair method, Pair requestURI, Pair protocol, Map> headers, String eol, boolean preserveHeaderCase) throws Exception { + doHttpRequestTest(method, requestURI, protocol, headers, headers, eol, preserveHeaderCase); + } + @SuppressWarnings("unchecked") + private void doHttpRequestTest(Pair method, Pair requestURI, + Pair protocol, Map> headers, + Map> expectedHeaders, String eol, + boolean preserveHeaderCase) throws Exception { final FutureImpl parseResult = SafeFutureImpl.create(); Connection connection = null; @@ -222,7 +271,7 @@ private void doHttpRequestTest(Pair method, Pair serverFilter.setPreserveHeaderCase(preserveHeaderCase); FilterChainBuilder filterChainBuilder = FilterChainBuilder.stateless().add(new TransportFilter()).add(new ChunkingFilter(2)).add(serverFilter) - .add(new HTTPRequestCheckFilter(parseResult, method, requestURI, protocol, headers, preserveHeaderCase)); + .add(new HTTPRequestCheckFilter(parseResult, method, requestURI, protocol, expectedHeaders == null ? headers : expectedHeaders, preserveHeaderCase)); TCPNIOTransport transport = TCPNIOTransportBuilder.newInstance().build(); transport.setProcessor(filterChainBuilder.build()); From 2b6f103a1060081116ba8b9e2615f1e3bf899471 Mon Sep 17 00:00:00 2001 From: Alfonso Altamirano Date: Mon, 11 Nov 2024 15:39:53 -0600 Subject: [PATCH 2/7] FISH-9690: adding properties to enable Header Name and Header Value validation against invalid characters RFC-9110 --- .../grizzly/http/HttpCodecFilter.java | 47 +++++++++++++++++-- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java b/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java index bc5061c4af..0336c6a10f 100644 --- a/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java +++ b/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java @@ -22,11 +22,14 @@ import static org.glassfish.grizzly.utils.Charsets.ASCII_CHARSET; import java.io.IOException; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.regex.Pattern; import org.glassfish.grizzly.Buffer; import org.glassfish.grizzly.Connection; import org.glassfish.grizzly.Grizzly; @@ -109,6 +112,16 @@ public abstract class HttpCodecFilter extends HttpBaseFilter implements Monitori * @see #setRemoveHandledContentEncodingHeaders */ private boolean removeHandledContentEncodingHeaders = false; + + private static final String STRICT_HEADER_NAME_VALIDATION_RFC_9110 = "org.glassfish.grizzly.http.STRICT_HEADER_NAME_VALIDATION_RFC_9110"; + + private static final String STRICT_HEADER_VALUE_VALIDATION_RFC_9110 = "org.glassfish.grizzly.http.STRICT_HEADER_VALUE_VALIDATION_RFC_9110"; + + private static final boolean isStrictHeaderNameValidationSet = Boolean.parseBoolean(System.getProperty(STRICT_HEADER_NAME_VALIDATION_RFC_9110)); + + private static final boolean isStrictHeaderValueValidationSet = Boolean.parseBoolean(System.getProperty(STRICT_HEADER_VALUE_VALIDATION_RFC_9110)); + + private static final String REGEX_RFC_9110_INVALID_CHARACTERS = "(\\\\n)|(\\\\0)|(\\\\r)|(\\\\x00)|(\\\\x0A)|(\\\\x0D)"; /** * File cache probes @@ -782,7 +795,7 @@ protected int parseHeaderName(final HttpHeader httpHeader, final MimeHeaders mim b -= Constants.LC_OFFSET; } input[offset] = b; - } else if (b == Constants.CR) { + } else if (isStrictHeaderNameValidationSet && b == Constants.CR) { parsingState.offset = offset - arrayOffs; final int eol = checkEOL(parsingState, input, end); if (eol == 0) { // EOL @@ -794,7 +807,7 @@ protected int parseHeaderName(final HttpHeader httpHeader, final MimeHeaders mim } } - if (!CookieHeaderParser.isToken(b)) { + if (isStrictHeaderNameValidationSet && !CookieHeaderParser.isToken(b)) { throw new IllegalStateException( "An invalid character 0x" + Integer.toHexString(b) + " was found in the header name"); } @@ -829,6 +842,10 @@ protected static int parseHeaderValue(final HttpHeader httpHeader, final HeaderP 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) { + //make validation with regex mode + validateRFC9110Characters(input, arrayOffs + parsingState.start, arrayOffs + parsingState.checkpoint2); + } return 0; } } @@ -849,13 +866,37 @@ protected static int parseHeaderValue(final HttpHeader httpHeader, final HeaderP } parsingState.checkpoint2 = parsingState.checkpoint; } - + offset++; } parsingState.offset = offset - arrayOffs; return -1; } + private static void validateRFC9110Characters(final byte[] headerValueContent, int start, int end) { + if (headerValueContent != null) { + if (isInvalidCharacterAvailable(start, end, headerValueContent)) { + throw new IllegalStateException( + "An invalid character NUL, LF or CR found in the header value: " + headerValueContent.toString()); + } + } + } + + /** + * This method evaluates the String from the bytes that contains Header Value and validates if contains literal value + * of \n, \r or \0 , in case any of those characters are available return true + * @param start index of the starting point to extract characters from the byte array + * @param end index of the end point to extract characters from the byte array + * @param bytesFromByteChunk represents the bytes from the request message to be processed + * @return Boolean true if any of those characters are available + */ + private static boolean isInvalidCharacterAvailable(int start, int end, byte[] bytesFromByteChunk) { + byte[] bytesFromHeaderValue = Arrays.copyOfRange(bytesFromByteChunk, start, end); + String uft8String = new String(bytesFromHeaderValue, StandardCharsets.UTF_8); + Pattern pattern = Pattern.compile(REGEX_RFC_9110_INVALID_CHARACTERS); + return pattern.matcher(uft8String).find(); + } + private static void finalizeKnownHeaderNames(final HttpHeader httpHeader, final HeaderParsingState parsingState, final byte[] input, final int start, final int end) { From d581202ec54afe75cbbe9e55e571d1e7dc295571 Mon Sep 17 00:00:00 2001 From: Alfonso Altamirano Date: Mon, 11 Nov 2024 21:53:09 -0600 Subject: [PATCH 3/7] FISH-9690: adding unit test for header value validation --- .../grizzly/http/HttpCodecFilter.java | 4 +- .../grizzly/http/HttpRequestParseTest.java | 39 ++++++++++++++++++- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java b/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java index 0336c6a10f..b29c1309cd 100644 --- a/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java +++ b/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java @@ -113,9 +113,9 @@ public abstract class HttpCodecFilter extends HttpBaseFilter implements Monitori */ private boolean removeHandledContentEncodingHeaders = false; - private 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_NAME_VALIDATION_RFC_9110 = "org.glassfish.grizzly.http.STRICT_HEADER_NAME_VALIDATION_RFC_9110"; - private static final String STRICT_HEADER_VALUE_VALIDATION_RFC_9110 = "org.glassfish.grizzly.http.STRICT_HEADER_VALUE_VALIDATION_RFC_9110"; + public static final String STRICT_HEADER_VALUE_VALIDATION_RFC_9110 = "org.glassfish.grizzly.http.STRICT_HEADER_VALUE_VALIDATION_RFC_9110"; private static final boolean isStrictHeaderNameValidationSet = Boolean.parseBoolean(System.getProperty(STRICT_HEADER_NAME_VALIDATION_RFC_9110)); diff --git a/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java b/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java index 6d81a74e97..47e1b54189 100644 --- a/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java +++ b/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java @@ -20,7 +20,6 @@ import java.net.InetSocketAddress; import java.net.SocketAddress; import java.util.Collections; -import java.util.HashMap; import java.util.Map; import java.util.Map.Entry; import java.util.concurrent.Future; @@ -50,6 +49,9 @@ import junit.framework.TestCase; +import static org.glassfish.grizzly.http.HttpCodecFilter.STRICT_HEADER_NAME_VALIDATION_RFC_9110; +import static org.glassfish.grizzly.http.HttpCodecFilter.STRICT_HEADER_VALUE_VALIDATION_RFC_9110; + /** * Testing HTTP request parsing * @@ -59,6 +61,20 @@ public class HttpRequestParseTest extends TestCase { public static final int PORT = 19000; + @Override + protected void setUp() throws Exception { + super.setUp(); + System.setProperty(STRICT_HEADER_NAME_VALIDATION_RFC_9110, String.valueOf(Boolean.TRUE)); + System.setProperty(STRICT_HEADER_VALUE_VALIDATION_RFC_9110, String.valueOf(Boolean.TRUE)); + } + + @Override + protected void tearDown() throws Exception { + super.tearDown(); + System.setProperty(STRICT_HEADER_NAME_VALIDATION_RFC_9110, String.valueOf(Boolean.FALSE)); + System.setProperty(STRICT_HEADER_VALUE_VALIDATION_RFC_9110, String.valueOf(Boolean.FALSE)); + } + public void testCustomMethod() throws Exception { doHttpRequestTest("TAKE", "/index.html", "HTTP/1.0", Collections.>emptyMap(), "\r\n"); } @@ -107,6 +123,27 @@ public void testDisallowedHeaders() { } } + public void testDisallowedCharactersForHeaderContentValues() { + try { + doTestDecoder("GET /index.html HTTP/1.1\nHost: loca\\rlhost\nContent -Length: 1234\n\n", 128); + fail("Bad HTTP headers exception had to be thrown"); + } catch (IllegalStateException e) { + // expected + } + try { + doTestDecoder("GET /index.html HTTP/1.1\nHost: loca\\nlhost\nContent-Length: 1234\n\n", 128); + fail("Bad HTTP headers exception had to be thrown"); + } catch (IllegalStateException e) { + // expected + } + try { + doTestDecoder("GET /index.html HTTP/1.1\nHost: loca\\0lhost\nContent-Length: 1234\n\n", 128); + fail("Bad HTTP headers exception had to be thrown"); + } catch (IllegalStateException e) { + // expected + } + } + public void testIgnoredHeaders() throws Exception { final Map> headers = new HashMap<>(); headers.put("Host", new Pair<>("localhost", "localhost")); From 59216d7bf6bea058e6a59522742084ade562fde4 Mon Sep 17 00:00:00 2001 From: Alfonso Altamirano Date: Mon, 11 Nov 2024 22:14:43 -0600 Subject: [PATCH 4/7] FISH-9690: adding missed import --- .../java/org/glassfish/grizzly/http/HttpRequestParseTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java b/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java index 47e1b54189..af909c529c 100644 --- a/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java +++ b/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java @@ -24,7 +24,7 @@ import java.util.Map.Entry; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; - +import java.util.HashMap; import org.glassfish.grizzly.Buffer; import org.glassfish.grizzly.Connection; import org.glassfish.grizzly.SocketConnectorHandler; From 6bb7eab869a6e5b550a4ee81d59fff19b71d76a0 Mon Sep 17 00:00:00 2001 From: Bongjae Chang Date: Mon, 18 Nov 2024 14:09:20 +0900 Subject: [PATCH 5/7] Issue #2212 Enhances validation of HTTP header names + Added the validation of HTTP header values + Name and value's validation are provided as options. (Improved based on https://github.com/carryel/grizzly/pull/1 @breakponchito) --- .../grizzly/http/HttpCodecFilter.java | 75 ++++++++++--------- .../grizzly/http/util/CookieHeaderParser.java | 9 +++ .../grizzly/http/HttpRequestParseTest.java | 19 +++++ 3 files changed, 67 insertions(+), 36 deletions(-) diff --git a/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java b/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java index b29c1309cd..bc01380bda 100644 --- a/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java +++ b/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java @@ -22,14 +22,11 @@ import static org.glassfish.grizzly.utils.Charsets.ASCII_CHARSET; import java.io.IOException; -import java.nio.charset.Charset; -import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.regex.Pattern; import org.glassfish.grizzly.Buffer; import org.glassfish.grizzly.Connection; import org.glassfish.grizzly.Grizzly; @@ -120,8 +117,6 @@ public abstract class HttpCodecFilter extends HttpBaseFilter implements Monitori private static final boolean isStrictHeaderNameValidationSet = Boolean.parseBoolean(System.getProperty(STRICT_HEADER_NAME_VALIDATION_RFC_9110)); private static final boolean isStrictHeaderValueValidationSet = Boolean.parseBoolean(System.getProperty(STRICT_HEADER_VALUE_VALIDATION_RFC_9110)); - - private static final String REGEX_RFC_9110_INVALID_CHARACTERS = "(\\\\n)|(\\\\0)|(\\\\r)|(\\\\x00)|(\\\\x0A)|(\\\\x0D)"; /** * File cache probes @@ -830,6 +825,20 @@ protected static int parseHeaderValue(final HttpHeader httpHeader, final HeaderP while (offset < limit) { final byte b = input[offset]; if (b == Constants.CR) { + if (isStrictHeaderValueValidationSet) { + if (offset + 1 < limit) { + final byte b2 = input[offset + 1]; + if (b2 == Constants.LF) { + // Continue for next parsing without the validation + offset++; + continue; + } + } else { + // not enough data + parsingState.offset = offset - arrayOffs; + return -1; + } + } } else if (b == Constants.LF) { // Check if it's not multi line header if (offset + 1 < limit) { @@ -842,10 +851,6 @@ protected static int parseHeaderValue(final HttpHeader httpHeader, final HeaderP 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) { - //make validation with regex mode - validateRFC9110Characters(input, arrayOffs + parsingState.start, arrayOffs + parsingState.checkpoint2); - } return 0; } } @@ -866,35 +871,15 @@ protected static int parseHeaderValue(final HttpHeader httpHeader, final HeaderP } parsingState.checkpoint2 = parsingState.checkpoint; } - - offset++; - } - parsingState.offset = offset - arrayOffs; - return -1; - } - private static void validateRFC9110Characters(final byte[] headerValueContent, int start, int end) { - if (headerValueContent != null) { - if (isInvalidCharacterAvailable(start, end, headerValueContent)) { + if (isStrictHeaderValueValidationSet && !CookieHeaderParser.isText(b)) { throw new IllegalStateException( - "An invalid character NUL, LF or CR found in the header value: " + headerValueContent.toString()); + "An invalid character 0x" + Integer.toHexString(b) + " was found in the header value"); } + offset++; } - } - - /** - * This method evaluates the String from the bytes that contains Header Value and validates if contains literal value - * of \n, \r or \0 , in case any of those characters are available return true - * @param start index of the starting point to extract characters from the byte array - * @param end index of the end point to extract characters from the byte array - * @param bytesFromByteChunk represents the bytes from the request message to be processed - * @return Boolean true if any of those characters are available - */ - private static boolean isInvalidCharacterAvailable(int start, int end, byte[] bytesFromByteChunk) { - byte[] bytesFromHeaderValue = Arrays.copyOfRange(bytesFromByteChunk, start, end); - String uft8String = new String(bytesFromHeaderValue, StandardCharsets.UTF_8); - Pattern pattern = Pattern.compile(REGEX_RFC_9110_INVALID_CHARACTERS); - return pattern.matcher(uft8String).find(); + parsingState.offset = offset - arrayOffs; + return -1; } private static void finalizeKnownHeaderNames(final HttpHeader httpHeader, final HeaderParsingState parsingState, final byte[] input, final int start, @@ -1095,7 +1080,7 @@ protected int parseHeaderName(final HttpHeader httpHeader, final MimeHeaders mim b -= Constants.LC_OFFSET; } input.put(offset, b); - } else if (b == Constants.CR) { + } else if (isStrictHeaderNameValidationSet && b == Constants.CR) { parsingState.offset = offset; final int eol = checkEOL(parsingState, input); if (eol == 0) { // EOL @@ -1107,7 +1092,7 @@ protected int parseHeaderName(final HttpHeader httpHeader, final MimeHeaders mim } } - if (!CookieHeaderParser.isToken(b)) { + if (isStrictHeaderNameValidationSet && !CookieHeaderParser.isToken(b)) { throw new IllegalStateException( "An invalid character 0x" + Integer.toHexString(b) + " was found in the header name"); } @@ -1129,6 +1114,20 @@ protected static int parseHeaderValue(final HttpHeader httpHeader, final HeaderP while (offset < limit) { final byte b = input.get(offset); if (b == Constants.CR) { + if (isStrictHeaderValueValidationSet) { + if (offset + 1 < limit) { + final byte b2 = input.get(offset + 1); + if (b2 == Constants.LF) { + // Continue for next parsing without the validation + offset++; + continue; + } + } else { + // not enough data + parsingState.offset = offset; + return -1; + } + } } else if (b == Constants.LF) { // Check if it's not multi line header if (offset + 1 < limit) { @@ -1162,6 +1161,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; diff --git a/modules/http/src/main/java/org/glassfish/grizzly/http/util/CookieHeaderParser.java b/modules/http/src/main/java/org/glassfish/grizzly/http/util/CookieHeaderParser.java index b6ea633b18..1275a06af9 100644 --- a/modules/http/src/main/java/org/glassfish/grizzly/http/util/CookieHeaderParser.java +++ b/modules/http/src/main/java/org/glassfish/grizzly/http/util/CookieHeaderParser.java @@ -248,6 +248,15 @@ public static boolean isToken(int c) { } } + public static boolean isText(int c) { + // 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 diff --git a/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java b/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java index af909c529c..a0fe1ee60d 100644 --- a/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java +++ b/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java @@ -124,6 +124,15 @@ public void testDisallowedHeaders() { } public void testDisallowedCharactersForHeaderContentValues() { + final StringBuilder sb = new StringBuilder("GET / HTTP/1.1\r\n"); + sb.append("Host: localhost\r\n"); + sb.append("Some-Header: some-"); + // valid header values + sb.append(new char[]{'\t', ' ', '\"', '(', ')', '/', ';', '<', '=', '>', '?', '@', '[', 0x5c, ']', '{', '}'}) + .append("\r\n"); + sb.append("\r\n"); + doTestDecoder(sb.toString(), 128); + try { doTestDecoder("GET /index.html HTTP/1.1\nHost: loca\\rlhost\nContent -Length: 1234\n\n", 128); fail("Bad HTTP headers exception had to be thrown"); @@ -142,6 +151,16 @@ public void testDisallowedCharactersForHeaderContentValues() { } catch (IllegalStateException e) { // expected } + + final char[] invalidChars = new char[]{0x00, 0x01, 0x02, '\r'}; + for (final char ch : invalidChars) { + try { + doTestDecoder("GET /index.html HTTP/1.1\nHost: localhost\nSome-Header: some-" + ch + "value\n\n", 128); + fail("Bad HTTP headers exception had to be thrown"); + } catch (IllegalStateException e) { + // expected + } + } } public void testIgnoredHeaders() throws Exception { From 9b09e7607acd0b2061e8d656284daa4244a264d4 Mon Sep 17 00:00:00 2001 From: Bongjae Chang Date: Tue, 19 Nov 2024 13:49:40 +0900 Subject: [PATCH 6/7] Issue #2212 Enhances validation of HTTP header names + trivial) updated license and re-trigger status checks(https://github.com/eclipse-ee4j/grizzly/pull/2213). --- .../org/glassfish/grizzly/http/util/CookieHeaderParser.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/http/src/main/java/org/glassfish/grizzly/http/util/CookieHeaderParser.java b/modules/http/src/main/java/org/glassfish/grizzly/http/util/CookieHeaderParser.java index 1275a06af9..12e1ce4b7d 100644 --- a/modules/http/src/main/java/org/glassfish/grizzly/http/util/CookieHeaderParser.java +++ b/modules/http/src/main/java/org/glassfish/grizzly/http/util/CookieHeaderParser.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, 2022 Contributors to the Eclipse Foundation + * Copyright (c) 2022, 2024 Contributors to the Eclipse Foundation * Copyright 2004, 2022 The Apache Software Foundation * * Licensed under the Apache License, Version 2.0 (the "License"); From d258c8beda6629ebe95f1e7d5485593ddc7bc348 Mon Sep 17 00:00:00 2001 From: Bongjae Chang Date: Sat, 14 Dec 2024 22:32:26 +0900 Subject: [PATCH 7/7] Issue #2212 Enhances validation of HTTP header names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit + Field values ​​cannot have a single LF as per RFC-9110(https://www.rfc-editor.org/rfc/rfc9110.html#section-5.5). + Additionally, this patch automatically removes support for multiple lines of http headers as mentioned in RFC-2616(https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2). Note) The features related to this issue #2212 only work when the STRICT_HEADER_NAME_VALIDATION_RFC_9110 and STRICT_HEADER_VALUE_VALIDATION_RFC_9110 options are enabled, and the existing code base behavior is maintained when options are not enabled. + Instead of throwing ArrayIndexOutOfBoundsException when judging Token and Text char, it ensures checking within the array range. + Since several existing http testcases do not respect CRLF in header values, I modified them to comply with the spec where it was not intentional. This patch passed all grizzly existing testcases locally, depending on the presence of options. > mvn clean test All passed. > mvn clean test -Dorg.glassfish.grizzly.http.STRICT_HEADER_NAME_VALIDATION_RFC_9110=true -Dorg.glassfish.grizzly.http.STRICT_HEADER_VALUE_VALIDATION_RFC_9110=true All passed. --- .../grizzly/comet/BasicCometTest.java | 14 +- .../grizzly/http/HttpCodecFilter.java | 113 +++++++-------- .../grizzly/http/util/CookieHeaderParser.java | 8 ++ .../http/ChunkedTransferEncodingTest.java | 12 +- .../grizzly/http/HttpRequestParseTest.java | 130 ++++++++++++++---- .../grizzly/http/HttpResponseParseTest.java | 81 ++++++++++- .../grizzly/http/HttpSemanticsTest.java | 4 +- 7 files changed, 259 insertions(+), 103 deletions(-) diff --git a/modules/comet/src/test/java/org/glassfish/grizzly/comet/BasicCometTest.java b/modules/comet/src/test/java/org/glassfish/grizzly/comet/BasicCometTest.java index 0eb262e277..812be25754 100644 --- a/modules/comet/src/test/java/org/glassfish/grizzly/comet/BasicCometTest.java +++ b/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()); diff --git a/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java b/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java index bc01380bda..caaf9ed847 100644 --- a/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java +++ b/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java @@ -114,9 +114,9 @@ public abstract class HttpCodecFilter extends HttpBaseFilter implements Monitori public static final String STRICT_HEADER_VALUE_VALIDATION_RFC_9110 = "org.glassfish.grizzly.http.STRICT_HEADER_VALUE_VALIDATION_RFC_9110"; - private static final boolean isStrictHeaderNameValidationSet = Boolean.parseBoolean(System.getProperty(STRICT_HEADER_NAME_VALIDATION_RFC_9110)); + private final boolean isStrictHeaderNameValidationSet = Boolean.parseBoolean(System.getProperty(STRICT_HEADER_NAME_VALIDATION_RFC_9110)); - private static final boolean isStrictHeaderValueValidationSet = Boolean.parseBoolean(System.getProperty(STRICT_HEADER_VALUE_VALIDATION_RFC_9110)); + private final boolean isStrictHeaderValueValidationSet = Boolean.parseBoolean(System.getProperty(STRICT_HEADER_VALUE_VALIDATION_RFC_9110)); /** * File cache probes @@ -813,7 +813,7 @@ protected int parseHeaderName(final HttpHeader httpHeader, final MimeHeaders mim 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); @@ -826,37 +826,40 @@ protected static int parseHeaderValue(final HttpHeader httpHeader, final HeaderP final byte b = input[offset]; if (b == Constants.CR) { if (isStrictHeaderValueValidationSet) { - if (offset + 1 < limit) { - final byte b2 = input[offset + 1]; - if (b2 == Constants.LF) { - // Continue for next parsing without the validation - offset++; - continue; - } - } else { - // not enough data - parsingState.offset = offset - arrayOffs; + 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; } } } 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); - return 0; + 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; } - - parsingState.offset = offset - arrayOffs; - return -1; } else if (b == Constants.SP) { if (hasShift) { input[arrayOffs + parsingState.checkpoint++] = b; @@ -1103,7 +1106,7 @@ protected int parseHeaderName(final HttpHeader httpHeader, final MimeHeaders mim 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); @@ -1115,37 +1118,39 @@ protected static int parseHeaderValue(final HttpHeader httpHeader, final HeaderP final byte b = input.get(offset); if (b == Constants.CR) { if (isStrictHeaderValueValidationSet) { - if (offset + 1 < limit) { - final byte b2 = input.get(offset + 1); - if (b2 == Constants.LF) { - // Continue for next parsing without the validation - offset++; - continue; - } - } else { - // not enough data - parsingState.offset = offset; + 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; } } } 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); - parsingState.headerValueStorage.setBuffer(input, parsingState.start, parsingState.checkpoint2); - return 0; + 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; } - - parsingState.offset = offset; - return -1; } else if (b == Constants.SP) { if (hasShift) { input.put(parsingState.checkpoint++, b); diff --git a/modules/http/src/main/java/org/glassfish/grizzly/http/util/CookieHeaderParser.java b/modules/http/src/main/java/org/glassfish/grizzly/http/util/CookieHeaderParser.java index 12e1ce4b7d..595e9b6191 100644 --- a/modules/http/src/main/java/org/glassfish/grizzly/http/util/CookieHeaderParser.java +++ b/modules/http/src/main/java/org/glassfish/grizzly/http/util/CookieHeaderParser.java @@ -240,6 +240,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]; @@ -249,6 +253,10 @@ 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]; diff --git a/modules/http/src/test/java/org/glassfish/grizzly/http/ChunkedTransferEncodingTest.java b/modules/http/src/test/java/org/glassfish/grizzly/http/ChunkedTransferEncodingTest.java index 844e44e1bd..2dc46537ba 100644 --- a/modules/http/src/test/java/org/glassfish/grizzly/http/ChunkedTransferEncodingTest.java +++ b/modules/http/src/test/java/org/glassfish/grizzly/http/ChunkedTransferEncodingTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2020 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 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 @@ -307,13 +307,13 @@ private void doHttpRequestTest(int packetsNum, boolean hasContent, Map> entry : trailerHeaders.entrySet()) { final String value = entry.getValue().getFirst(); if (value != null) { - sb.append(entry.getKey()).append(": ").append(value).append(eol); + sb.append(entry.getKey()).append(": ").append(value).append("\r\n"); } } diff --git a/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java b/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java index a0fe1ee60d..5315206955 100644 --- a/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java +++ b/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.net.InetSocketAddress; import java.net.SocketAddress; +import java.util.Collection; import java.util.Collections; import java.util.Map; import java.util.Map.Entry; @@ -47,42 +48,83 @@ import org.glassfish.grizzly.utils.ChunkingFilter; import org.glassfish.grizzly.utils.Pair; -import junit.framework.TestCase; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import static java.lang.Boolean.FALSE; +import static java.lang.Boolean.TRUE; +import static java.util.Arrays.asList; import static org.glassfish.grizzly.http.HttpCodecFilter.STRICT_HEADER_NAME_VALIDATION_RFC_9110; import static org.glassfish.grizzly.http.HttpCodecFilter.STRICT_HEADER_VALUE_VALIDATION_RFC_9110; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * Testing HTTP request parsing * * @author Alexey Stashok */ -public class HttpRequestParseTest extends TestCase { +@RunWith(Parameterized.class) +public class HttpRequestParseTest { public static final int PORT = 19000; - @Override - protected void setUp() throws Exception { - super.setUp(); - System.setProperty(STRICT_HEADER_NAME_VALIDATION_RFC_9110, String.valueOf(Boolean.TRUE)); - System.setProperty(STRICT_HEADER_VALUE_VALIDATION_RFC_9110, String.valueOf(Boolean.TRUE)); + private final boolean isStrictHeaderNameValidationSet; + private final boolean isStrictHeaderValueValidationSet; + private final String isStrictHeaderNameValidationSetBefore; + private final String isStrictHeaderValueValidationSetBefore; + + @Parameterized.Parameters + public static Collection getMode() { + return asList(new Object[][] { { FALSE, FALSE }, { FALSE, TRUE }, { TRUE, FALSE }, { TRUE, TRUE } }); + } + + @Before + public void before() throws Exception { + if (isStrictHeaderNameValidationSet) { + System.setProperty(STRICT_HEADER_NAME_VALIDATION_RFC_9110, String.valueOf(Boolean.TRUE)); + } else { + System.setProperty(STRICT_HEADER_NAME_VALIDATION_RFC_9110, String.valueOf(Boolean.FALSE)); + } + if (isStrictHeaderValueValidationSet) { + System.setProperty(STRICT_HEADER_VALUE_VALIDATION_RFC_9110, String.valueOf(Boolean.TRUE)); + } else { + System.setProperty(STRICT_HEADER_VALUE_VALIDATION_RFC_9110, String.valueOf(Boolean.FALSE)); + } } - @Override - protected void tearDown() throws Exception { - super.tearDown(); - System.setProperty(STRICT_HEADER_NAME_VALIDATION_RFC_9110, String.valueOf(Boolean.FALSE)); - System.setProperty(STRICT_HEADER_VALUE_VALIDATION_RFC_9110, String.valueOf(Boolean.FALSE)); + @After + public void after() throws Exception { + System.setProperty(STRICT_HEADER_NAME_VALIDATION_RFC_9110, + isStrictHeaderNameValidationSetBefore != null ? isStrictHeaderNameValidationSetBefore : + String.valueOf(Boolean.FALSE)); + System.setProperty(STRICT_HEADER_VALUE_VALIDATION_RFC_9110, + isStrictHeaderValueValidationSetBefore != null ? isStrictHeaderValueValidationSetBefore : + String.valueOf(Boolean.FALSE)); } + public HttpRequestParseTest(boolean isStrictHeaderNameValidationSet, boolean isStrictHeaderValueValidationSet) { + this.isStrictHeaderNameValidationSet = isStrictHeaderNameValidationSet; + this.isStrictHeaderValueValidationSet = isStrictHeaderValueValidationSet; + this.isStrictHeaderNameValidationSetBefore = System.getProperty(STRICT_HEADER_NAME_VALIDATION_RFC_9110); + this.isStrictHeaderValueValidationSetBefore = System.getProperty(STRICT_HEADER_VALUE_VALIDATION_RFC_9110); + } + + @Test public void testCustomMethod() throws Exception { doHttpRequestTest("TAKE", "/index.html", "HTTP/1.0", Collections.>emptyMap(), "\r\n"); } + @Test public void testHeaderlessRequestLine() throws Exception { doHttpRequestTest("GET", "/index.html", "HTTP/1.0", Collections.>emptyMap(), "\r\n"); } + @Test public void testSimpleHeaders() throws Exception { Map> headers = new HashMap<>(); headers.put("Host", new Pair<>("localhost", "localhost")); @@ -90,6 +132,7 @@ public void testSimpleHeaders() throws Exception { doHttpRequestTest("POST", "/index.html", "HTTP/1.1", headers, "\r\n"); } + @Test public void testSimpleHeadersPreserveCase() throws Exception { Map> headers = new HashMap<>(); headers.put("Host", new Pair<>("localhost", "localhost")); @@ -97,7 +140,11 @@ public void testSimpleHeadersPreserveCase() throws Exception { doHttpRequestTest("POST", "/index.html", "HTTP/1.1", headers, "\r\n", true); } - public void testDisallowedHeaders() { + @Test + public void testDisallowedCharactersForHeaderNames() { + if (!isStrictHeaderNameValidationSet) { + return; + } final StringBuilder sb = new StringBuilder("GET / HTTP/1.1\r\n"); sb.append("Host: localhost\r\n"); sb.append(new char[]{0x00, 0x01, 0x02, '\t', '\n', '\r', ' ', '\"', '(', ')', '/', ';', '<', '=', '>', '?', '@', @@ -110,20 +157,24 @@ public void testDisallowedHeaders() { // expected } try { - doTestDecoder("GET /index.html HTTP/1.1\nHost: localhost\nContent -Length: 1234\n\n", 128); + doTestDecoder("GET /index.html HTTP/1.1\nHost: localhost\r\nContent -Length: 1234\r\n\n", 128); fail("Bad HTTP headers exception had to be thrown"); } catch (IllegalStateException e) { // expected } try { - doTestDecoder("GET /index.html HTTP/1.1\nHost: localhost\nContent-\rLength: 1234\n\n", 128); + doTestDecoder("GET /index.html HTTP/1.1\nHost: localhost\r\nContent-\rLength: 1234\r\n\n", 128); fail("Bad HTTP headers exception had to be thrown"); } catch (IllegalStateException e) { // expected } } + @Test public void testDisallowedCharactersForHeaderContentValues() { + if (!isStrictHeaderValueValidationSet) { + return; + } final StringBuilder sb = new StringBuilder("GET / HTTP/1.1\r\n"); sb.append("Host: localhost\r\n"); sb.append("Some-Header: some-"); @@ -134,28 +185,22 @@ public void testDisallowedCharactersForHeaderContentValues() { doTestDecoder(sb.toString(), 128); try { - doTestDecoder("GET /index.html HTTP/1.1\nHost: loca\\rlhost\nContent -Length: 1234\n\n", 128); + doTestDecoder("GET /index.html HTTP/1.1\nHost: loca\rlhost\r\nContent-Length: 1234\r\n\n", 128); fail("Bad HTTP headers exception had to be thrown"); } catch (IllegalStateException e) { // expected } try { - doTestDecoder("GET /index.html HTTP/1.1\nHost: loca\\nlhost\nContent-Length: 1234\n\n", 128); - fail("Bad HTTP headers exception had to be thrown"); - } catch (IllegalStateException e) { - // expected - } - try { - doTestDecoder("GET /index.html HTTP/1.1\nHost: loca\\0lhost\nContent-Length: 1234\n\n", 128); + doTestDecoder("GET /index.html HTTP/1.1\nHost: loca\nlhost\r\nContent-Length: 1234\r\n\n", 128); fail("Bad HTTP headers exception had to be thrown"); } catch (IllegalStateException e) { // expected } - final char[] invalidChars = new char[]{0x00, 0x01, 0x02, '\r'}; + final char[] invalidChars = new char[]{0x00, 0x01, 0x02, '\r', '\n'}; for (final char ch : invalidChars) { try { - doTestDecoder("GET /index.html HTTP/1.1\nHost: localhost\nSome-Header: some-" + ch + "value\n\n", 128); + doTestDecoder("GET /index.html HTTP/1.1\nHost: localhost\r\nSome-Header: some-" + ch + "value\r\n\n", 128); fail("Bad HTTP headers exception had to be thrown"); } catch (IllegalStateException e) { // expected @@ -163,7 +208,11 @@ public void testDisallowedCharactersForHeaderContentValues() { } } + @Test public void testIgnoredHeaders() throws Exception { + if (!isStrictHeaderNameValidationSet) { + return; + } final Map> headers = new HashMap<>(); headers.put("Host", new Pair<>("localhost", "localhost")); headers.put("Ignore\r\nContent-length", new Pair<>("2345", "2345")); @@ -173,7 +222,12 @@ public void testIgnoredHeaders() throws Exception { doHttpRequestTest("POST", "/index.html", "HTTP/1.1", headers, expectedHeaders, "\r\n"); } + @Test public void testMultiLineHeaders() throws Exception { + if (isStrictHeaderValueValidationSet) { + // Multiline headers should not be supported + return; + } Map> headers = new HashMap<>(); headers.put("Host", new Pair<>("localhost", "localhost")); headers.put("Multi-line", new Pair<>("first\r\n second\r\n third", "first second third")); @@ -181,7 +235,12 @@ public void testMultiLineHeaders() throws Exception { doHttpRequestTest("POST", "/index.html", "HTTP/1.1", headers, "\r\n"); } + @Test public void testHeadersN() throws Exception { + if (isStrictHeaderValueValidationSet) { + // Multiline headers should not be supported + return; + } Map> headers = new HashMap<>(); headers.put("Host", new Pair<>("localhost", "localhost")); headers.put("Multi-line", new Pair<>("first\r\n second\n third", "first second third")); @@ -189,26 +248,30 @@ public void testHeadersN() throws Exception { doHttpRequestTest("POST", "/index.html", "HTTP/1.1", headers, "\n"); } + @Test public void testCompleteURI() throws Exception { Map> headers = new HashMap<>(); headers.put("Host", new Pair(null, "localhost:8180")); headers.put("Content-length", new Pair<>("2345", "2345")); doHttpRequestTest(new Pair<>("POST", "POST"), new Pair<>("http://localhost:8180/index.html", "/index.html"), - new Pair<>("HTTP/1.1", "HTTP/1.1"), headers, "\n", false); + new Pair<>("HTTP/1.1", "HTTP/1.1"), headers, "\r\n", false); } + @Test public void testCompleteEmptyURI() throws Exception { Map> headers = new HashMap<>(); headers.put("Host", new Pair(null, "localhost:8180")); headers.put("Content-length", new Pair<>("2345", "2345")); doHttpRequestTest(new Pair<>("POST", "POST"), new Pair<>("http://localhost:8180", "/"), - new Pair<>("HTTP/1.1", "HTTP/1.1"), headers, "\n", false); + new Pair<>("HTTP/1.1", "HTTP/1.1"), headers, "\r\n", false); } + @Test public void testDecoderOK() { doTestDecoder("GET /index.html HTTP/1.0\n\n", 4096); } + @Test public void testDecoderOverflowMethod() { try { doTestDecoder("GET /index.html HTTP/1.0\n\n", 2); @@ -218,6 +281,7 @@ public void testDecoderOverflowMethod() { } } + @Test public void testDecoderOverflowURI() { try { doTestDecoder("GET /index.html HTTP/1.0\n\n", 8); @@ -227,6 +291,7 @@ public void testDecoderOverflowURI() { } } + @Test public void testDecoderOverflowProtocol() { try { doTestDecoder("GET /index.html HTTP/1.0\n\n", 19); @@ -236,19 +301,22 @@ public void testDecoderOverflowProtocol() { } } + @Test public void testDecoderOverflowHeader1() { try { - doTestDecoder("GET /index.html HTTP/1.0\nHost: localhost\n\n", 41); + doTestDecoder("GET /index.html HTTP/1.0\nHost: localhost\r\n\n", 42); fail("Overflow exception had to be thrown"); } catch (IllegalStateException e) { // expected } } + @Test public void testDecoderOverflowHeader2() { - doTestDecoder("GET /index.html HTTP/1.0\nHost: localhost\n\n", 42); + doTestDecoder("GET /index.html HTTP/1.0\nHost: localhost\r\n\n", 43); } + @Test public void testDecoderOverflowHeader3() { try { doTestDecoder("GET /index.html HTTP/1.0\nHost: localhost\r\n\r\n", 43); @@ -258,12 +326,14 @@ public void testDecoderOverflowHeader3() { } } + @Test public void testDecoderOverflowHeader4() { doTestDecoder("GET /index.html HTTP/1.0\nHost: localhost\r\n\r\n", 44); } + @Test public void testChunkedTransferEncodingCaseInsensitive() { - HttpPacket packet = doTestDecoder("POST /index.html HTTP/1.1\nHost: localhost\nTransfer-Encoding: CHUNked\r\n\r\n0\r\n\r\n", 4096); + HttpPacket packet = doTestDecoder("POST /index.html HTTP/1.1\nHost: localhost\r\nTransfer-Encoding: CHUNked\r\n\r\n0\r\n\r\n", 4096); assertTrue(packet.getHttpHeader().isChunked()); } diff --git a/modules/http/src/test/java/org/glassfish/grizzly/http/HttpResponseParseTest.java b/modules/http/src/test/java/org/glassfish/grizzly/http/HttpResponseParseTest.java index 250d3836a7..e8af25b6c1 100644 --- a/modules/http/src/test/java/org/glassfish/grizzly/http/HttpResponseParseTest.java +++ b/modules/http/src/test/java/org/glassfish/grizzly/http/HttpResponseParseTest.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 @@ -18,6 +18,7 @@ import java.io.IOException; import java.net.SocketAddress; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -48,22 +49,78 @@ import org.glassfish.grizzly.utils.ChunkingFilter; import org.glassfish.grizzly.utils.Pair; -import junit.framework.TestCase; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import static java.lang.Boolean.FALSE; +import static java.lang.Boolean.TRUE; +import static java.util.Arrays.asList; +import static org.glassfish.grizzly.http.HttpCodecFilter.STRICT_HEADER_NAME_VALIDATION_RFC_9110; +import static org.glassfish.grizzly.http.HttpCodecFilter.STRICT_HEADER_VALUE_VALIDATION_RFC_9110; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; /** * Testing HTTP response parsing * * @author Alexey Stashok */ -public class HttpResponseParseTest extends TestCase { +@RunWith(Parameterized.class) +public class HttpResponseParseTest { private static final Logger logger = Grizzly.logger(HttpResponseParseTest.class); public static final int PORT = 19021; + private final boolean isStrictHeaderNameValidationSet; + private final boolean isStrictHeaderValueValidationSet; + private final String isStrictHeaderNameValidationSetBefore; + private final String isStrictHeaderValueValidationSetBefore; + + @Parameterized.Parameters + public static Collection getMode() { + return asList(new Object[][] { { FALSE, FALSE }, { FALSE, TRUE }, { TRUE, FALSE }, { TRUE, TRUE } }); + } + + @Before + public void before() throws Exception { + if (isStrictHeaderNameValidationSet) { + System.setProperty(STRICT_HEADER_NAME_VALIDATION_RFC_9110, String.valueOf(Boolean.TRUE)); + } else { + System.setProperty(STRICT_HEADER_NAME_VALIDATION_RFC_9110, String.valueOf(Boolean.FALSE)); + } + if (isStrictHeaderValueValidationSet) { + System.setProperty(STRICT_HEADER_VALUE_VALIDATION_RFC_9110, String.valueOf(Boolean.TRUE)); + } else { + System.setProperty(STRICT_HEADER_VALUE_VALIDATION_RFC_9110, String.valueOf(Boolean.FALSE)); + } + } + + @After + public void after() throws Exception { + System.setProperty(STRICT_HEADER_NAME_VALIDATION_RFC_9110, + isStrictHeaderNameValidationSetBefore != null ? isStrictHeaderNameValidationSetBefore : + String.valueOf(Boolean.FALSE)); + System.setProperty(STRICT_HEADER_VALUE_VALIDATION_RFC_9110, + isStrictHeaderValueValidationSetBefore != null ? isStrictHeaderValueValidationSetBefore : + String.valueOf(Boolean.FALSE)); + } + + public HttpResponseParseTest(boolean isStrictHeaderNameValidationSet, boolean isStrictHeaderValueValidationSet) { + this.isStrictHeaderNameValidationSet = isStrictHeaderNameValidationSet; + this.isStrictHeaderValueValidationSet = isStrictHeaderValueValidationSet; + this.isStrictHeaderNameValidationSetBefore = System.getProperty(STRICT_HEADER_NAME_VALIDATION_RFC_9110); + this.isStrictHeaderValueValidationSetBefore = System.getProperty(STRICT_HEADER_VALUE_VALIDATION_RFC_9110); + } + + @Test public void testHeaderlessResponseLine() throws Exception { doHttpResponseTest("HTTP/1.0", 200, "OK", Collections.>emptyMap(), "\r\n"); } + @Test public void testSimpleHeaders() throws Exception { Map> headers = new HashMap<>(); headers.put("Header1", new Pair<>("localhost", "localhost")); @@ -71,7 +128,12 @@ public void testSimpleHeaders() throws Exception { doHttpResponseTest("HTTP/1.0", 200, "ALL RIGHT", headers, "\r\n"); } + @Test public void testMultiLineHeaders() throws Exception { + if (isStrictHeaderValueValidationSet) { + // Multiline headers should not be supported + return; + } Map> headers = new HashMap<>(); headers.put("Header1", new Pair<>("localhost", "localhost")); headers.put("Multi-line", new Pair<>("first\r\n second\r\n third", "first seconds third")); @@ -79,7 +141,12 @@ public void testMultiLineHeaders() throws Exception { doHttpResponseTest("HTTP/1.0", 200, "DONE", headers, "\r\n"); } + @Test public void testHeadersN() throws Exception { + if (isStrictHeaderValueValidationSet) { + // Multiline headers should not be supported + return; + } Map> headers = new HashMap<>(); headers.put("Header1", new Pair<>("localhost", "localhost")); headers.put("Multi-line", new Pair<>("first\n second\n third", "first seconds third")); @@ -87,6 +154,7 @@ public void testHeadersN() throws Exception { doHttpResponseTest("HTTP/1.0", 200, "DONE", headers, "\n"); } + @Test public void testDecoder100continueThen200() { try { doTestDecoder("HTTP/1.1 100 Continue\n\nHTTP/1.1 200 OK\n\n", 4096); @@ -97,6 +165,7 @@ public void testDecoder100continueThen200() { } } + @Test public void testDecoderOK() { try { doTestDecoder("HTTP/1.0 404 Not found\n\n", 4096); @@ -107,6 +176,7 @@ public void testDecoderOK() { } } + @Test public void testDecoderOverflowProtocol() { try { doTestDecoder("HTTP/1.0 404 Not found\n\n", 2); @@ -116,6 +186,7 @@ public void testDecoderOverflowProtocol() { } } + @Test public void testDecoderOverflowCode() { try { doTestDecoder("HTTP/1.0 404 Not found\n\n", 11); @@ -125,6 +196,7 @@ public void testDecoderOverflowCode() { } } + @Test public void testDecoderOverflowPhrase() { try { doTestDecoder("HTTP/1.0 404 Not found\n\n", 19); @@ -134,9 +206,10 @@ public void testDecoderOverflowPhrase() { } } + @Test public void testDecoderOverflowHeader() { try { - doTestDecoder("HTTP/1.0 404 Not found\nHeader1: somevalue\n\n", 30); + doTestDecoder("HTTP/1.0 404 Not found\nHeader1: somevalue\r\n\n", 31); assertTrue("Overflow exception had to be thrown", false); } catch (IllegalStateException e) { assertTrue(true); diff --git a/modules/http/src/test/java/org/glassfish/grizzly/http/HttpSemanticsTest.java b/modules/http/src/test/java/org/glassfish/grizzly/http/HttpSemanticsTest.java index 8e3e4c934d..1f991932f5 100644 --- a/modules/http/src/test/java/org/glassfish/grizzly/http/HttpSemanticsTest.java +++ b/modules/http/src/test/java/org/glassfish/grizzly/http/HttpSemanticsTest.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 @@ -677,7 +677,7 @@ public void testHttpHeadersLimit() throws Throwable { public void testExplicitConnectionCloseHeader() throws Throwable { final TCPNIOConnection connection = new TCPNIOConnection(TCPNIOTransportBuilder.newInstance().build(), null); - Buffer requestBuf = Buffers.wrap(connection.getMemoryManager(), "GET /path HTTP/1.1\n" + "Host: localhost:" + PORT + '\n' + '\n'); + Buffer requestBuf = Buffers.wrap(connection.getMemoryManager(), "GET /path HTTP/1.1\n" + "Host: localhost:" + PORT + "\r\n" + '\n'); FilterChainContext ctx = FilterChainContext.create(connection); ctx.setMessage(requestBuf);