Skip to content

Commit e4d596e

Browse files
joakimegregw
joakime
and
gregw
authored
Backport of various cleanups in HttpParser from jetty-10.0.x (#10352)
* Backport of various cleanups in HttpParser from jetty-10.0.x * Handle complianceViolation() in old style for 9.4.x Signed-off-by: gregw <gregw@webtide.com> Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com> Co-authored-by: Greg Wilkins <gregw@webtide.com>
1 parent d4d8832 commit e4d596e

File tree

2 files changed

+51
-84
lines changed

2 files changed

+51
-84
lines changed

‎jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,7 @@ private HttpTokens.Token next(ByteBuffer buffer)
499499
/* Quick lookahead for the start state looking for a request method or an HTTP version,
500500
* otherwise skip white space until something else to parse.
501501
*/
502-
private boolean quickStart(ByteBuffer buffer)
502+
private void quickStart(ByteBuffer buffer)
503503
{
504504
if (_requestHandler != null)
505505
{
@@ -510,7 +510,7 @@ private boolean quickStart(ByteBuffer buffer)
510510
buffer.position(buffer.position() + _methodString.length() + 1);
511511

512512
setState(State.SPACE1);
513-
return false;
513+
return;
514514
}
515515
}
516516
else if (_responseHandler != null)
@@ -520,7 +520,7 @@ else if (_responseHandler != null)
520520
{
521521
buffer.position(buffer.position() + _version.asString().length() + 1);
522522
setState(State.SPACE1);
523-
return false;
523+
return;
524524
}
525525
}
526526

@@ -541,7 +541,7 @@ else if (_responseHandler != null)
541541
_string.setLength(0);
542542
_string.append(t.getChar());
543543
setState(_requestHandler != null ? State.METHOD : State.RESPONSE_VERSION);
544-
return false;
544+
return;
545545
}
546546
case OTEXT:
547547
case SPACE:
@@ -559,7 +559,6 @@ else if (_responseHandler != null)
559559
throw new BadMessageException(HttpStatus.BAD_REQUEST_400);
560560
}
561561
}
562-
return false;
563562
}
564563

565564
private void setString(String s)
@@ -974,23 +973,20 @@ private void parsedHeader()
974973
case CONTENT_LENGTH:
975974
if (_hasTransferEncoding && complianceViolation(TRANSFER_ENCODING_WITH_CONTENT_LENGTH))
976975
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, "Transfer-Encoding and Content-Length");
977-
976+
long contentLength = convertContentLength(_valueString);
978977
if (_hasContentLength)
979978
{
980979
if (complianceViolation(MULTIPLE_CONTENT_LENGTHS))
981980
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, MULTIPLE_CONTENT_LENGTHS.description);
982-
if (convertContentLength(_valueString) != _contentLength)
983-
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, MULTIPLE_CONTENT_LENGTHS.description);
981+
if (contentLength != _contentLength)
982+
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, MULTIPLE_CONTENT_LENGTHS.getDescription());
984983
}
985984
_hasContentLength = true;
986985

987986
if (_endOfContent != EndOfContent.CHUNKED_CONTENT)
988987
{
989-
_contentLength = convertContentLength(_valueString);
990-
if (_contentLength <= 0)
991-
_endOfContent = EndOfContent.NO_CONTENT;
992-
else
993-
_endOfContent = EndOfContent.CONTENT_LENGTH;
988+
_contentLength = contentLength;
989+
_endOfContent = EndOfContent.CONTENT_LENGTH;
994990
}
995991
break;
996992

@@ -1107,15 +1103,21 @@ private void parsedTrailer()
11071103

11081104
private long convertContentLength(String valueString)
11091105
{
1110-
try
1111-
{
1112-
return Long.parseLong(valueString);
1113-
}
1114-
catch (NumberFormatException e)
1106+
if (valueString == null || valueString.length() == 0)
1107+
throw new BadMessageException("Invalid Content-Length Value", new NumberFormatException());
1108+
1109+
long value = 0;
1110+
int length = valueString.length();
1111+
1112+
for (int i = 0; i < length; i++)
11151113
{
1116-
LOG.ignore(e);
1117-
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, "Invalid Content-Length Value", e);
1114+
char c = valueString.charAt(i);
1115+
if (c < '0' || c > '9')
1116+
throw new BadMessageException("Invalid Content-Length Value", new NumberFormatException());
1117+
1118+
value = Math.addExact(Math.multiplyExact(value, 10L), c - '0');
11181119
}
1120+
return value;
11191121
}
11201122

11211123
/*
@@ -1511,12 +1513,11 @@ public boolean parseNext(ByteBuffer buffer)
15111513
_methodString = null;
15121514
_endOfContent = EndOfContent.UNKNOWN_CONTENT;
15131515
_header = null;
1514-
if (quickStart(buffer))
1515-
return true;
1516+
quickStart(buffer);
15161517
}
15171518

15181519
// Request/response line
1519-
if (_state.ordinal() >= State.START.ordinal() && _state.ordinal() < State.HEADER.ordinal())
1520+
if (_state.ordinal() < State.HEADER.ordinal())
15201521
{
15211522
if (parseLine(buffer))
15221523
return true;
@@ -2036,7 +2037,6 @@ default void onComplianceViolation(HttpCompliance compliance, HttpComplianceSect
20362037
}
20372038
}
20382039

2039-
@SuppressWarnings("serial")
20402040
private static class IllegalCharacterException extends BadMessageException
20412041
{
20422042
private IllegalCharacterException(State state, HttpTokens.Token token, ByteBuffer buffer)

‎jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java

Lines changed: 27 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import static org.hamcrest.Matchers.contains;
4242
import static org.hamcrest.Matchers.containsString;
4343
import static org.hamcrest.Matchers.is;
44+
import static org.hamcrest.Matchers.notNullValue;
4445
import static org.hamcrest.Matchers.nullValue;
4546
import static org.junit.jupiter.api.Assertions.assertEquals;
4647
import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -1672,7 +1673,7 @@ public void testNoURI2()
16721673
}
16731674

16741675
@Test
1675-
public void testUnknownReponseVersion()
1676+
public void testUnknownResponseVersion()
16761677
{
16771678
ByteBuffer buffer = BufferUtil.toBuffer(
16781679
"HPPT/7.7 200 OK\r\n" +
@@ -1815,65 +1816,31 @@ public void testBadCR()
18151816
assertEquals(HttpParser.State.CLOSED, parser.getState());
18161817
}
18171818

1818-
@Test
1819-
public void testBadContentLength0()
1820-
{
1821-
ByteBuffer buffer = BufferUtil.toBuffer(
1822-
"GET / HTTP/1.0\r\n" +
1823-
"Content-Length: abc\r\n" +
1824-
"Connection: close\r\n" +
1825-
"\r\n");
1826-
1827-
HttpParser.RequestHandler handler = new Handler();
1828-
HttpParser parser = new HttpParser(handler);
1829-
1830-
parser.parseNext(buffer);
1831-
assertEquals("GET", _methodOrVersion);
1832-
assertEquals("Invalid Content-Length Value", _bad);
1833-
assertFalse(buffer.hasRemaining());
1834-
assertEquals(HttpParser.State.CLOSE, parser.getState());
1835-
parser.atEOF();
1836-
parser.parseNext(BufferUtil.EMPTY_BUFFER);
1837-
assertEquals(HttpParser.State.CLOSED, parser.getState());
1838-
}
1839-
1840-
@Test
1841-
public void testBadContentLength1()
1842-
{
1843-
ByteBuffer buffer = BufferUtil.toBuffer(
1844-
"GET / HTTP/1.0\r\n" +
1845-
"Content-Length: 9999999999999999999999999999999999999999999999\r\n" +
1846-
"Connection: close\r\n" +
1847-
"\r\n");
1848-
1849-
HttpParser.RequestHandler handler = new Handler();
1850-
HttpParser parser = new HttpParser(handler);
1851-
1852-
parser.parseNext(buffer);
1853-
assertEquals("GET", _methodOrVersion);
1854-
assertEquals("Invalid Content-Length Value", _bad);
1855-
assertFalse(buffer.hasRemaining());
1856-
assertEquals(HttpParser.State.CLOSE, parser.getState());
1857-
parser.atEOF();
1858-
parser.parseNext(BufferUtil.EMPTY_BUFFER);
1859-
assertEquals(HttpParser.State.CLOSED, parser.getState());
1860-
}
1861-
1862-
@Test
1863-
public void testBadContentLength2()
1864-
{
1865-
ByteBuffer buffer = BufferUtil.toBuffer(
1866-
"GET / HTTP/1.0\r\n" +
1867-
"Content-Length: 1.5\r\n" +
1868-
"Connection: close\r\n" +
1869-
"\r\n");
1819+
@ParameterizedTest
1820+
@ValueSource(strings = {
1821+
"abc",
1822+
"1.5",
1823+
"9999999999999999999999999999999999999999999999",
1824+
"-10",
1825+
"+10",
1826+
"1.0",
1827+
"1,0",
1828+
"10,"
1829+
})
1830+
public void testBadContentLengths(String contentLength)
1831+
{
1832+
ByteBuffer buffer = BufferUtil.toBuffer(
1833+
"GET /test HTTP/1.1\r\n" +
1834+
"Host: localhost\r\n" +
1835+
"Content-Length: " + contentLength + "\r\n" +
1836+
"\r\n" +
1837+
"1234567890\r\n");
18701838

18711839
HttpParser.RequestHandler handler = new Handler();
1872-
HttpParser parser = new HttpParser(handler);
1840+
HttpParser parser = new HttpParser(handler, HttpCompliance.RFC2616_LEGACY);
1841+
parseAll(parser, buffer);
18731842

1874-
parser.parseNext(buffer);
1875-
assertEquals("GET", _methodOrVersion);
1876-
assertEquals("Invalid Content-Length Value", _bad);
1843+
assertThat(_bad, notNullValue());
18771844
assertFalse(buffer.hasRemaining());
18781845
assertEquals(HttpParser.State.CLOSE, parser.getState());
18791846
parser.atEOF();
@@ -2084,7 +2051,7 @@ public void testIPv6Host()
20842051
@Test
20852052
public void testBadIPv6Host()
20862053
{
2087-
try (StacklessLogging s = new StacklessLogging(HttpParser.class))
2054+
try (StacklessLogging ignored = new StacklessLogging(HttpParser.class))
20882055
{
20892056
ByteBuffer buffer = BufferUtil.toBuffer(
20902057
"GET / HTTP/1.1\r\n" +
@@ -2930,8 +2897,8 @@ public void init()
29302897
private String _methodOrVersion;
29312898
private String _uriOrStatus;
29322899
private String _versionOrReason;
2933-
private List<HttpField> _fields = new ArrayList<>();
2934-
private List<HttpField> _trailers = new ArrayList<>();
2900+
private final List<HttpField> _fields = new ArrayList<>();
2901+
private final List<HttpField> _trailers = new ArrayList<>();
29352902
private String[] _hdr;
29362903
private String[] _val;
29372904
private int _headers;

0 commit comments

Comments
 (0)