Skip to content

Commit

Permalink
Issue #2212 Enhances validation of HTTP header names
Browse files Browse the repository at this point in the history
  • Loading branch information
@carryel
carryel committed Sep 14, 2024
1 parent 4316ca1 commit ed2eba1
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 12 deletions.
57 changes: 48 additions & 9 deletions modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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++;
Expand Down Expand Up @@ -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;

Expand All @@ -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) {
Expand Down Expand Up @@ -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++;
Expand Down Expand Up @@ -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;
Expand All @@ -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) {
Expand Down
55 changes: 52 additions & 3 deletions modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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<String, Pair<String, String>> headers = new HashMap<>();
headers.put("Host", new Pair<>("localhost", "localhost"));
headers.put("Ignore\r\nContent-length", new Pair<>("2345", "2345"));
final Map<String, Pair<String, String>> 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<String, Pair<String, String>> headers = new HashMap<>();
headers.put("Host", new Pair<>("localhost", "localhost"));
Expand Down Expand Up @@ -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<String, Pair<String, String>> headers,
Map<String, Pair<String, String>> 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<String, Pair<String, String>> 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<String, String> method, Pair<String, String> requestURI, Pair<String, String> protocol,
Map<String, Pair<String, String>> headers, String eol, boolean preserveHeaderCase) throws Exception {
doHttpRequestTest(method, requestURI, protocol, headers, headers, eol, preserveHeaderCase);
}

@SuppressWarnings("unchecked")
private void doHttpRequestTest(Pair<String, String> method, Pair<String, String> requestURI,
Pair<String, String> protocol, Map<String, Pair<String, String>> headers,
Map<String, Pair<String, String>> expectedHeaders, String eol,
boolean preserveHeaderCase) throws Exception {
final FutureImpl<Boolean> parseResult = SafeFutureImpl.create();

Connection connection = null;
Expand All @@ -222,7 +271,7 @@ private void doHttpRequestTest(Pair<String, String> method, Pair<String, String>
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());
Expand Down

0 comments on commit ed2eba1

Please sign in to comment.