Skip to content

Commit

Permalink
Backport of various cleanups in HttpParser from jetty-10.0.x (#10352
Browse files Browse the repository at this point in the history 
)

* 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>
  • Loading branch information
@joakime @gregw
joakime and gregw committed Aug 21, 2023
1 parent d4d8832 commit e4d596e
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 84 deletions.
48 changes: 24 additions & 24 deletions jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
View file
Expand Up @@ -499,7 +499,7 @@ private HttpTokens.Token next(ByteBuffer buffer)
/* Quick lookahead for the start state looking for a request method or an HTTP version,
* otherwise skip white space until something else to parse.
*/
private boolean quickStart(ByteBuffer buffer)
private void quickStart(ByteBuffer buffer)
{
if (_requestHandler != null)
{
Expand All @@ -510,7 +510,7 @@ private boolean quickStart(ByteBuffer buffer)
buffer.position(buffer.position() + _methodString.length() + 1);

setState(State.SPACE1);
return false;
return;
}
}
else if (_responseHandler != null)
Expand All @@ -520,7 +520,7 @@ else if (_responseHandler != null)
{
buffer.position(buffer.position() + _version.asString().length() + 1);
setState(State.SPACE1);
return false;
return;
}
}

Expand All @@ -541,7 +541,7 @@ else if (_responseHandler != null)
_string.setLength(0);
_string.append(t.getChar());
setState(_requestHandler != null ? State.METHOD : State.RESPONSE_VERSION);
return false;
return;
}
case OTEXT:
case SPACE:
Expand All @@ -559,7 +559,6 @@ else if (_responseHandler != null)
throw new BadMessageException(HttpStatus.BAD_REQUEST_400);
}
}
return false;
}

private void setString(String s)
Expand Down Expand Up @@ -974,23 +973,20 @@ private void parsedHeader()
case CONTENT_LENGTH:
if (_hasTransferEncoding && complianceViolation(TRANSFER_ENCODING_WITH_CONTENT_LENGTH))
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, "Transfer-Encoding and Content-Length");

long contentLength = convertContentLength(_valueString);
if (_hasContentLength)
{
if (complianceViolation(MULTIPLE_CONTENT_LENGTHS))
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, MULTIPLE_CONTENT_LENGTHS.description);
if (convertContentLength(_valueString) != _contentLength)
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, MULTIPLE_CONTENT_LENGTHS.description);
if (contentLength != _contentLength)
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, MULTIPLE_CONTENT_LENGTHS.getDescription());
}
_hasContentLength = true;

if (_endOfContent != EndOfContent.CHUNKED_CONTENT)
{
_contentLength = convertContentLength(_valueString);
if (_contentLength <= 0)
_endOfContent = EndOfContent.NO_CONTENT;
else
_endOfContent = EndOfContent.CONTENT_LENGTH;
_contentLength = contentLength;
_endOfContent = EndOfContent.CONTENT_LENGTH;
}
break;

Expand Down Expand Up @@ -1107,15 +1103,21 @@ private void parsedTrailer()

private long convertContentLength(String valueString)
{
try
{
return Long.parseLong(valueString);
}
catch (NumberFormatException e)
if (valueString == null || valueString.length() == 0)
throw new BadMessageException("Invalid Content-Length Value", new NumberFormatException());

long value = 0;
int length = valueString.length();

for (int i = 0; i < length; i++)
{
LOG.ignore(e);
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, "Invalid Content-Length Value", e);
char c = valueString.charAt(i);
if (c < '0' || c > '9')
throw new BadMessageException("Invalid Content-Length Value", new NumberFormatException());

value = Math.addExact(Math.multiplyExact(value, 10L), c - '0');
}
return value;
}

/*
Expand Down Expand Up @@ -1511,12 +1513,11 @@ public boolean parseNext(ByteBuffer buffer)
_methodString = null;
_endOfContent = EndOfContent.UNKNOWN_CONTENT;
_header = null;
if (quickStart(buffer))
return true;
quickStart(buffer);
}

// Request/response line
if (_state.ordinal() >= State.START.ordinal() && _state.ordinal() < State.HEADER.ordinal())
if (_state.ordinal() < State.HEADER.ordinal())
{
if (parseLine(buffer))
return true;
Expand Down Expand Up @@ -2036,7 +2037,6 @@ default void onComplianceViolation(HttpCompliance compliance, HttpComplianceSect
}
}

@SuppressWarnings("serial")
private static class IllegalCharacterException extends BadMessageException
{
private IllegalCharacterException(State state, HttpTokens.Token token, ByteBuffer buffer)
Expand Down
87 changes: 27 additions & 60 deletions jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java
View file
Expand Up @@ -41,6 +41,7 @@
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
Expand Down Expand Up @@ -1672,7 +1673,7 @@ public void testNoURI2()
}

@Test
public void testUnknownReponseVersion()
public void testUnknownResponseVersion()
{
ByteBuffer buffer = BufferUtil.toBuffer(
"HPPT/7.7 200 OK\r\n" +
Expand Down Expand Up @@ -1815,65 +1816,31 @@ public void testBadCR()
assertEquals(HttpParser.State.CLOSED, parser.getState());
}

@Test
public void testBadContentLength0()
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / HTTP/1.0\r\n" +
"Content-Length: abc\r\n" +
"Connection: close\r\n" +
"\r\n");

HttpParser.RequestHandler handler = new Handler();
HttpParser parser = new HttpParser(handler);

parser.parseNext(buffer);
assertEquals("GET", _methodOrVersion);
assertEquals("Invalid Content-Length Value", _bad);
assertFalse(buffer.hasRemaining());
assertEquals(HttpParser.State.CLOSE, parser.getState());
parser.atEOF();
parser.parseNext(BufferUtil.EMPTY_BUFFER);
assertEquals(HttpParser.State.CLOSED, parser.getState());
}

@Test
public void testBadContentLength1()
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / HTTP/1.0\r\n" +
"Content-Length: 9999999999999999999999999999999999999999999999\r\n" +
"Connection: close\r\n" +
"\r\n");

HttpParser.RequestHandler handler = new Handler();
HttpParser parser = new HttpParser(handler);

parser.parseNext(buffer);
assertEquals("GET", _methodOrVersion);
assertEquals("Invalid Content-Length Value", _bad);
assertFalse(buffer.hasRemaining());
assertEquals(HttpParser.State.CLOSE, parser.getState());
parser.atEOF();
parser.parseNext(BufferUtil.EMPTY_BUFFER);
assertEquals(HttpParser.State.CLOSED, parser.getState());
}

@Test
public void testBadContentLength2()
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / HTTP/1.0\r\n" +
"Content-Length: 1.5\r\n" +
"Connection: close\r\n" +
"\r\n");
@ParameterizedTest
@ValueSource(strings = {
"abc",
"1.5",
"9999999999999999999999999999999999999999999999",
"-10",
"+10",
"1.0",
"1,0",
"10,"
})
public void testBadContentLengths(String contentLength)
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET /test HTTP/1.1\r\n" +
"Host: localhost\r\n" +
"Content-Length: " + contentLength + "\r\n" +
"\r\n" +
"1234567890\r\n");

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

parser.parseNext(buffer);
assertEquals("GET", _methodOrVersion);
assertEquals("Invalid Content-Length Value", _bad);
assertThat(_bad, notNullValue());
assertFalse(buffer.hasRemaining());
assertEquals(HttpParser.State.CLOSE, parser.getState());
parser.atEOF();
Expand Down Expand Up @@ -2084,7 +2051,7 @@ public void testIPv6Host()
@Test
public void testBadIPv6Host()
{
try (StacklessLogging s = new StacklessLogging(HttpParser.class))
try (StacklessLogging ignored = new StacklessLogging(HttpParser.class))
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / HTTP/1.1\r\n" +
Expand Down Expand Up @@ -2930,8 +2897,8 @@ public void init()
private String _methodOrVersion;
private String _uriOrStatus;
private String _versionOrReason;
private List<HttpField> _fields = new ArrayList<>();
private List<HttpField> _trailers = new ArrayList<>();
private final List<HttpField> _fields = new ArrayList<>();
private final List<HttpField> _trailers = new ArrayList<>();
private String[] _hdr;
private String[] _val;
private int _headers;
Expand Down

0 comments on commit e4d596e

Please sign in to comment.