Skip to content

Commit

Permalink
Issue #11659 - Properly ignore OWS for field values.
Browse files Browse the repository at this point in the history
  • Loading branch information
joakime committed Apr 18, 2024
1 parent a774047 commit 19917d9
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 31 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 @@ -1228,21 +1229,11 @@ private long convertContentLength(String valueString)

for (int i = 0; i < length; i++)
{
char ch = valueString.charAt(i);
HttpTokens.Token t = HttpTokens.getToken(ch);
char c = valueString.charAt(i);
if (c < '0' || c > '9')
throw new BadMessageException("Invalid Content-Length Value", new NumberFormatException());

switch (t.getType())
{
case SPACE:
case HTAB:
// ignore OWS
continue;
case DIGIT:
value = Math.addExact(Math.multiplyExact(value, 10), ch - '0');
break;
default:
throw new BadMessageException("Invalid Content-Length Value", new NumberFormatException());
}
value = Math.addExact(Math.multiplyExact(value, 10), c - '0');
}
return value;
}
Expand Down Expand Up @@ -1387,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 @@ -1414,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 @@ -2023,7 +2023,6 @@ public void testContentLengthWithOWS(String contentLength)
assertEquals("GET", _methodOrVersion);
assertEquals("/test", _uriOrStatus);
assertEquals("HTTP/1.1", _versionOrReason);
assertEquals("localhost", _val[0]);
assertEquals("Host", _hdr[0]);
assertEquals("localhost", _val[0]);

Expand All @@ -2033,6 +2032,79 @@ public void testContentLengthWithOWS(String contentLength)
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 19917d9

Please sign in to comment.