Skip to content

Commit

Permalink
Issue #2212 Enhances validation of HTTP header names
Browse files Browse the repository at this point in the history 
+ 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.
  • Loading branch information
@carryel
carryel committed Dec 14, 2024
1 parent 9b09e76 commit d258c8b
Show file tree
Hide file tree
Showing 7 changed files with 259 additions and 103 deletions.
14 changes: 7 additions & 7 deletions modules/comet/src/test/java/org/glassfish/grizzly/comet/BasicCometTest.java
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down
113 changes: 59 additions & 54 deletions modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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);

Expand All @@ -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);
Expand Down
8 changes: 8 additions & 0 deletions modules/http/src/main/java/org/glassfish/grizzly/http/util/CookieHeaderParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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];
Expand Down
12 changes: 6 additions & 6 deletions modules/http/src/test/java/org/glassfish/grizzly/http/ChunkedTransferEncodingTest.java
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -307,13 +307,13 @@ private void doHttpRequestTest(int packetsNum, boolean hasContent, Map<String, P
.append(eol)
.append("Host: localhost:")
.append(PORT)
.append(eol)
.append("\r\n")
.append("Transfer-encoding: chunked")
.append(eol)
.append("Content-Type: application/x-www-form-urlencoded").append(eol);
.append("\r\n")
.append("Content-Type: application/x-www-form-urlencoded").append("\r\n");

if (i == packetsNum - 1) {
sb.append("Connection: close").append(eol);
sb.append("Connection: close").append("\r\n");
}

sb.append(eol);
Expand All @@ -327,7 +327,7 @@ private void doHttpRequestTest(int packetsNum, boolean hasContent, Map<String, P
for (Entry<String, Pair<String, String>> 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");
}
}

Expand Down
Loading

0 comments on commit d258c8b

Please sign in to comment.