Skip to content

Commit

Permalink
Issue #11659 - Properly ignore OWS before field values. (#11661)
Browse files Browse the repository at this point in the history
* implemented changes to CACHE and parseFields to handle OWS properly for all UNMATCHED_VALUE headers.
* added 3 new OWS test cases (that fail in 12.0.x HEAD btw) to handle this OWS case.
  • Loading branch information
joakime authored Apr 19, 2024
1 parent 6d37ce1 commit 8b1c6bc
Show file tree
Hide file tree
Showing 2 changed files with 132 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ public class HttpParser
public static final Logger LOG = LoggerFactory.getLogger(HttpParser.class);
public static final int INITIAL_URI_LENGTH = 256;
private static final int MAX_CHUNK_LENGTH = Integer.MAX_VALUE / 16 - 16;
private static final String UNMATCHED_VALUE = "\u0000";

/**
* Cache of common {@link HttpField}s including: <UL>
Expand Down Expand Up @@ -165,7 +166,7 @@ public class HttpParser
Map<String, HttpField> map = new LinkedHashMap<>();
for (HttpHeader h : HttpHeader.values())
{
HttpField httpField = new HttpField(h, (String)null);
HttpField httpField = new HttpField(h, UNMATCHED_VALUE);
map.put(httpField.toString(), httpField);
}
return map;
Expand Down Expand Up @@ -1377,25 +1378,28 @@ else if (_endOfContent == EndOfContent.UNKNOWN_CONTENT)
String n = cachedField.getName();
String v = cachedField.getValue();

if (CASE_SENSITIVE_FIELD_NAME.isAllowedBy(_complianceMode))
if (v != UNMATCHED_VALUE)
{
// Have to get the fields exactly from the buffer to match case
String en = BufferUtil.toString(buffer, buffer.position() - 1, n.length(), StandardCharsets.US_ASCII);
if (!n.equals(en))
if (CASE_SENSITIVE_FIELD_NAME.isAllowedBy(_complianceMode))
{
reportComplianceViolation(CASE_SENSITIVE_FIELD_NAME, en);
n = en;
cachedField = new HttpField(cachedField.getHeader(), n, v);
// Have to get the fields exactly from the buffer to match case
String en = BufferUtil.toString(buffer, buffer.position() - 1, n.length(), StandardCharsets.US_ASCII);
if (!n.equals(en))
{
reportComplianceViolation(CASE_SENSITIVE_FIELD_NAME, en);
n = en;
cachedField = new HttpField(cachedField.getHeader(), n, v);
}
}
}

if (v != null && isHeaderCacheCaseSensitive())
{
String ev = BufferUtil.toString(buffer, buffer.position() + n.length() + 1, v.length(), StandardCharsets.ISO_8859_1);
if (!v.equals(ev))
if (isHeaderCacheCaseSensitive())
{
v = ev;
cachedField = new HttpField(cachedField.getHeader(), n, v);
String ev = BufferUtil.toString(buffer, buffer.position() + n.length() + 1, v.length(), StandardCharsets.ISO_8859_1);
if (!v.equals(ev))
{
v = ev;
cachedField = new HttpField(cachedField.getHeader(), n, v);
}
}
}

Expand All @@ -1404,7 +1408,7 @@ else if (_endOfContent == EndOfContent.UNKNOWN_CONTENT)

int posAfterName = buffer.position() + n.length() + 1;

if (v == null || (posAfterName + v.length()) >= buffer.limit())
if (v == UNMATCHED_VALUE || (posAfterName + v.length()) >= buffer.limit())
{
// Header only
setState(FieldState.VALUE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1971,7 +1971,8 @@ public void testBadCR(String eoln)
"+10",
"1.0",
"1,0",
"10,"
"10,",
"10A"
})
public void testBadContentLengths(String contentLength)
{
Expand All @@ -1994,6 +1995,116 @@ public void testBadContentLengths(String contentLength)
assertEquals(HttpParser.State.CLOSED, parser.getState());
}

@ParameterizedTest
@ValueSource(strings = {
" 10 ",
"10 ",
" 10",
"\t10",
"\t10\t",
"10\t",
" \t \t \t 10"
})
public void testContentLengthWithOWS(String contentLength)
{
String rawRequest = """
GET /test HTTP/1.1\r
Host: localhost\r
Content-Length: @LEN@\r
\r
1234567890
""".replace("@LEN@", contentLength);
ByteBuffer buffer = BufferUtil.toBuffer(rawRequest);

HttpParser.RequestHandler handler = new Handler();
HttpParser parser = new HttpParser(handler);
parseAll(parser, buffer);

assertEquals("GET", _methodOrVersion);
assertEquals("/test", _uriOrStatus);
assertEquals("HTTP/1.1", _versionOrReason);
assertEquals("Host", _hdr[0]);
assertEquals("localhost", _val[0]);

assertEquals(_content.length(), 10);
assertEquals(parser.getContentLength(), 10);
assertTrue(_headerCompleted);
assertTrue(_messageCompleted);
}

@ParameterizedTest
@ValueSource(strings = {
" chunked ",
"chunked ",
" chunked",
"\tchunked",
"\tchunked\t",
"chunked\t",
" \t \t \t chunked"
})
public void testTransferEncodingWithOWS(String transferEncoding)
{
String rawRequest = """
GET /test HTTP/1.1\r
Host: localhost\r
Transfer-Encoding: @TE@\r
\r
1\r
X\r
0\r
\r
""".replace("@TE@", transferEncoding);
ByteBuffer buffer = BufferUtil.toBuffer(rawRequest);

HttpParser.RequestHandler handler = new Handler();
HttpParser parser = new HttpParser(handler);
parseAll(parser, buffer);

assertEquals("GET", _methodOrVersion);
assertEquals("/test", _uriOrStatus);
assertEquals("HTTP/1.1", _versionOrReason);
assertEquals("Host", _hdr[0]);
assertEquals("localhost", _val[0]);
assertEquals("Transfer-Encoding", _hdr[1]);
assertEquals("chunked", _val[1]);

assertTrue(_headerCompleted);
assertTrue(_messageCompleted);
}

@ParameterizedTest
@ValueSource(strings = {
" testhost ",
"testhost ",
" testhost",
"\ttesthost",
"\ttesthost\t",
"testhost\t",
" \t \t \t testhost"
})
public void testHostWithOWS(String host)
{
String rawRequest = """
GET /test HTTP/1.1\r
Host: @HOST@\r
\r
""".replace("@HOST@", host);
ByteBuffer buffer = BufferUtil.toBuffer(rawRequest);

HttpParser.RequestHandler handler = new Handler();
HttpParser parser = new HttpParser(handler);
parseAll(parser, buffer);

assertEquals("GET", _methodOrVersion);
assertEquals("/test", _uriOrStatus);
assertEquals("HTTP/1.1", _versionOrReason);
assertEquals("Host", _hdr[0]);
assertEquals("testhost", _val[0]);

assertTrue(_headerCompleted);
assertTrue(_messageCompleted);
}

@ParameterizedTest
@ValueSource(strings = {"\r\n", "\n"})
public void testMultipleContentLengthWithLargerThenCorrectValue(String eoln)
Expand Down

0 comments on commit 8b1c6bc

Please sign in to comment.