diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java index 990e53adcda4..4db6716487e9 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java @@ -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) { @@ -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) @@ -520,7 +520,7 @@ else if (_responseHandler != null) { buffer.position(buffer.position() + _version.asString().length() + 1); setState(State.SPACE1); - return false; + return; } } @@ -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: @@ -559,7 +559,6 @@ else if (_responseHandler != null) throw new BadMessageException(HttpStatus.BAD_REQUEST_400); } } - return false; } private void setString(String s) @@ -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; @@ -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; } /* @@ -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; @@ -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) diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java index a5bbb0c1f4ae..ff19c595f983 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java @@ -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; @@ -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" + @@ -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(); @@ -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" + @@ -2930,8 +2897,8 @@ public void init() private String _methodOrVersion; private String _uriOrStatus; private String _versionOrReason; - private List _fields = new ArrayList<>(); - private List _trailers = new ArrayList<>(); + private final List _fields = new ArrayList<>(); + private final List _trailers = new ArrayList<>(); private String[] _hdr; private String[] _val; private int _headers;