From 47447b5cf502d5682e217fbccbda21dcd84c799c Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 11 Nov 2019 01:13:13 +0100 Subject: [PATCH 1/2] Issue #4282 - Review HttpParser handling in case of no content. Fixed handling of returning true from headerComplete() and contentComplete(). Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/http/HttpParser.java | 119 ++-- .../eclipse/jetty/http/HttpParserTest.java | 627 ++++++++++++++++++ 2 files changed, 702 insertions(+), 44 deletions(-) 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 e5b09b69714..10619ce1dbb 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 @@ -134,6 +134,7 @@ public class HttpParser CHUNK_SIZE, CHUNK_PARAMS, CHUNK, + CONTENT_END, TRAILER, END, CLOSE, // The associated stream/endpoint should be closed @@ -564,15 +565,19 @@ public class HttpParser { boolean handleHeader = _handler.headerComplete(); _headerComplete = true; - boolean handleContentMessage = handleContentMessage(); - return handleHeader || handleContentMessage; + if (handleHeader) + return true; + setState(State.CONTENT_END); + return handleContentMessage(); } private boolean handleContentMessage() { boolean handleContent = _handler.contentComplete(); - boolean handleMessage = _handler.messageComplete(); - return handleContent || handleMessage; + if (handleContent) + return true; + setState(State.END); + return _handler.messageComplete(); } /* Parse a request or response line @@ -743,7 +748,7 @@ public class HttpParser case LF: setState(State.HEADER); - handle = _responseHandler.startResponse(_version, _responseStatus, null); + _responseHandler.startResponse(_version, _responseStatus, null); break; default: @@ -762,10 +767,11 @@ public class HttpParser // HTTP/0.9 if (complianceViolation(HttpComplianceSection.NO_HTTP_0_9, "No request version")) throw new BadMessageException("HTTP/0.9 not supported"); - handle = _requestHandler.startRequest(_methodString, _uri.toString(), HttpVersion.HTTP_0_9); - setState(State.END); + _requestHandler.startRequest(_methodString, _uri.toString(), HttpVersion.HTTP_0_9); + setState(State.CONTENT); + _endOfContent = EndOfContent.NO_CONTENT; BufferUtil.clear(buffer); - handle |= handleHeaderContentMessage(); + handle = handleHeaderContentMessage(); break; case ALPHA: @@ -841,7 +847,7 @@ public class HttpParser if (_responseHandler != null) { setState(State.HEADER); - handle = _responseHandler.startResponse(_version, _responseStatus, null); + _responseHandler.startResponse(_version, _responseStatus, null); } else { @@ -849,10 +855,11 @@ public class HttpParser if (complianceViolation(HttpComplianceSection.NO_HTTP_0_9, "No request version")) throw new BadMessageException("HTTP/0.9 not supported"); - handle = _requestHandler.startRequest(_methodString, _uri.toString(), HttpVersion.HTTP_0_9); - setState(State.END); + _requestHandler.startRequest(_methodString, _uri.toString(), HttpVersion.HTTP_0_9); + setState(State.CONTENT); + _endOfContent = EndOfContent.NO_CONTENT; BufferUtil.clear(buffer); - handle |= handleHeaderContentMessage(); + handle = handleHeaderContentMessage(); } break; @@ -879,7 +886,7 @@ public class HttpParser setState(State.HEADER); - handle = _requestHandler.startRequest(_methodString, _uri.toString(), _version); + _requestHandler.startRequest(_methodString, _uri.toString(), _version); continue; case ALPHA: @@ -901,7 +908,7 @@ public class HttpParser case LF: String reason = takeString(); setState(State.HEADER); - handle = _responseHandler.startResponse(_version, _responseStatus, reason); + _responseHandler.startResponse(_version, _responseStatus, reason); continue; case ALPHA: @@ -1207,11 +1214,6 @@ public class HttpParser _headerComplete = true; return handle; } - case NO_CONTENT: - { - setState(State.END); - return handleHeaderContentMessage(); - } default: { setState(State.CONTENT); @@ -1508,8 +1510,16 @@ public class HttpParser // Handle HEAD response if (_responseStatus > 0 && _headResponse) { - setState(State.END); - return handleContentMessage(); + if (_state != State.CONTENT_END) + { + setState(State.CONTENT_END); + return handleContentMessage(); + } + else + { + setState(State.END); + return _handler.messageComplete(); + } } else { @@ -1528,11 +1538,18 @@ public class HttpParser // handle end states if (_state == State.END) { - // eat white space - while (buffer.remaining() > 0 && buffer.get(buffer.position()) <= HttpTokens.SPACE) + // Eat CR or LF white space, but not SP. + int whiteSpace = 0; + while (buffer.remaining() > 0) { + byte b = buffer.get(buffer.position()); + if (b != HttpTokens.CARRIAGE_RETURN && b != HttpTokens.LINE_FEED) + break; buffer.get(); + ++whiteSpace; } + if (debug && whiteSpace > 0) + LOG.debug("Discarded {} CR or LF characters", whiteSpace); } else if (isClose() || isClosed()) { @@ -1540,18 +1557,13 @@ public class HttpParser } // Handle EOF - if (_eof && !buffer.hasRemaining()) + if (isAtEOF() && !buffer.hasRemaining()) { switch (_state) { case CLOSED: break; - case START: - setState(State.CLOSED); - _handler.earlyEOF(); - break; - case END: case CLOSE: setState(State.CLOSED); @@ -1562,13 +1574,18 @@ public class HttpParser if (_fieldState == FieldState.FIELD) { // Be forgiving of missing last CRLF + setState(State.CONTENT_END); + boolean handle = handleContentMessage(); + if (handle && _state == State.CONTENT_END) + return true; setState(State.CLOSED); - return handleContentMessage(); + return handle; } setState(State.CLOSED); _handler.earlyEOF(); break; + case START: case CONTENT: case CHUNKED_CONTENT: case CHUNK_SIZE: @@ -1614,17 +1631,28 @@ public class HttpParser protected boolean parseContent(ByteBuffer buffer) { int remaining = buffer.remaining(); - if (remaining == 0 && _state == State.CONTENT) + if (remaining == 0) { - long content = _contentLength - _contentPosition; - if (content == 0) + switch (_state) { - setState(State.END); - return handleContentMessage(); + case CONTENT: + long content = _contentLength - _contentPosition; + if (_endOfContent == EndOfContent.NO_CONTENT || content == 0) + { + setState(State.CONTENT_END); + return handleContentMessage(); + } + break; + case CONTENT_END: + setState(_endOfContent == EndOfContent.EOF_CONTENT ? State.CLOSED : State.END); + return _handler.messageComplete(); + default: + // No bytes to parse, return immediately. + return false; } } - // Handle _content + // Handle content. while (_state.ordinal() < State.TRAILER.ordinal() && remaining > 0) { switch (_state) @@ -1640,9 +1668,9 @@ public class HttpParser case CONTENT: { long content = _contentLength - _contentPosition; - if (content == 0) + if (_endOfContent == EndOfContent.NO_CONTENT || content == 0) { - setState(State.END); + setState(State.CONTENT_END); return handleContentMessage(); } else @@ -1665,7 +1693,7 @@ public class HttpParser if (_contentPosition == _contentLength) { - setState(State.END); + setState(State.CONTENT_END); return handleContentMessage(); } } @@ -1790,10 +1818,10 @@ public class HttpParser break; } - case CLOSED: + case CONTENT_END: { - BufferUtil.clear(buffer); - return false; + setState(_endOfContent == EndOfContent.EOF_CONTENT ? State.CLOSED : State.END); + return _handler.messageComplete(); } default: @@ -1877,8 +1905,8 @@ public class HttpParser return String.format("%s{s=%s,%d of %d}", getClass().getSimpleName(), _state, - _contentPosition, - _contentLength); + getContentRead(), + getContentLength()); } /* Event Handler interface @@ -1941,6 +1969,7 @@ public class HttpParser /** * @return the size in bytes of the per parser header cache */ + // TODO: move this to the Parser in Jetty 10. int getHeaderCacheSize(); } @@ -1954,6 +1983,7 @@ public class HttpParser * @param version the http version in use * @return true if handling parsing should return. */ + // TODO: make this a void method in Jetty 10. boolean startRequest(String method, String uri, HttpVersion version); } @@ -1967,6 +1997,7 @@ public class HttpParser * @param reason the response reason phrase * @return true if handling parsing should return */ + // TODO: make this a void method in Jetty 10. boolean startResponse(HttpVersion version, int status, String reason); } 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 b6244fa6143..f01e48c15b3 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.is; import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -2227,6 +2228,629 @@ public class HttpParserTest assertNull(_bad); } + @Test + public void testForContentLengthZeroHeaderCompleteTrueDoesNotEmitContentComplete() + { + HttpParser.ResponseHandler handler = new Handler() + { + @Override + public boolean headerComplete() + { + super.headerComplete(); + return true; + } + }; + HttpParser parser = new HttpParser(handler); + + ByteBuffer buffer = BufferUtil.toBuffer( + "HTTP/1.1 200 OK\r\n" + + "Content-Length: 0\r\n" + + "\r\n"); + boolean handle = parser.parseNext(buffer); + assertTrue(handle); + assertFalse(buffer.hasRemaining()); + assertFalse(_contentCompleted); + assertFalse(_messageCompleted); + + // Need to parse more to advance the parser. + handle = parser.parseNext(buffer); + assertTrue(handle); + assertTrue(_contentCompleted); + assertTrue(_messageCompleted); + } + + @Test + public void testForEmptyChunkedContentHeaderCompleteTrueDoesNotEmitContentComplete() + { + HttpParser.ResponseHandler handler = new Handler() + { + @Override + public boolean headerComplete() + { + super.headerComplete(); + return true; + } + }; + HttpParser parser = new HttpParser(handler); + + ByteBuffer buffer = BufferUtil.toBuffer( + "HTTP/1.1 200 OK\r\n" + + "Transfer-Encoding: chunked\r\n" + + "\r\n" + + "0\r\n" + + "\r\n"); + boolean handle = parser.parseNext(buffer); + assertTrue(handle); + assertTrue(buffer.hasRemaining()); + assertFalse(_contentCompleted); + assertFalse(_messageCompleted); + + // Need to parse more to advance the parser. + handle = parser.parseNext(buffer); + assertTrue(handle); + assertTrue(_contentCompleted); + assertTrue(_messageCompleted); + } + + @Test + public void testForContentLengthZeroContentCompleteTrueDoesNotEmitMessageComplete() + { + HttpParser.ResponseHandler handler = new Handler() + { + @Override + public boolean contentComplete() + { + super.contentComplete(); + return true; + } + }; + HttpParser parser = new HttpParser(handler); + + ByteBuffer buffer = BufferUtil.toBuffer( + "HTTP/1.1 200 OK\r\n" + + "Content-Length: 0\r\n" + + "\r\n"); + boolean handle = parser.parseNext(buffer); + assertTrue(handle); + assertFalse(buffer.hasRemaining()); + assertFalse(_messageCompleted); + + // Need to parse more to advance the parser. + handle = parser.parseNext(buffer); + assertTrue(handle); + assertTrue(_messageCompleted); + } + + @Test + public void testForEmptyChunkedContentContentCompleteTrueDoesNotEmitMessageComplete() + { + HttpParser.ResponseHandler handler = new Handler() + { + @Override + public boolean contentComplete() + { + super.contentComplete(); + return true; + } + }; + HttpParser parser = new HttpParser(handler); + + ByteBuffer buffer = BufferUtil.toBuffer( + "HTTP/1.1 200 OK\r\n" + + "Transfer-Encoding: chunked\r\n" + + "\r\n" + + "0\r\n" + + "\r\n"); + boolean handle = parser.parseNext(buffer); + assertTrue(handle); + assertTrue(buffer.hasRemaining()); + assertFalse(_messageCompleted); + + // Need to parse more to advance the parser. + handle = parser.parseNext(buffer); + assertTrue(handle); + assertTrue(_messageCompleted); + } + + @Test + public void testHeaderAfterContentLengthZeroContentCompleteTrue() + { + HttpParser.ResponseHandler handler = new Handler() + { + @Override + public boolean contentComplete() + { + super.contentComplete(); + return true; + } + }; + HttpParser parser = new HttpParser(handler); + + String header = "Header: Foobar\r\n"; + ByteBuffer buffer = BufferUtil.toBuffer( + "HTTP/1.1 200 OK\r\n" + + "Content-Length: 0\r\n" + + "\r\n" + + header); + boolean handle = parser.parseNext(buffer); + assertTrue(handle); + assertTrue(buffer.hasRemaining()); + assertEquals(header, BufferUtil.toString(buffer)); + assertTrue(_contentCompleted); + assertFalse(_messageCompleted); + + // Need to parse more to advance the parser. + handle = parser.parseNext(buffer); + assertTrue(handle); + assertTrue(buffer.hasRemaining()); + assertEquals(header, BufferUtil.toString(buffer)); + assertTrue(_messageCompleted); + } + + @Test + public void testSmallContentLengthContentCompleteTrue() + { + HttpParser.ResponseHandler handler = new Handler() + { + @Override + public boolean contentComplete() + { + super.contentComplete(); + return true; + } + }; + HttpParser parser = new HttpParser(handler); + + String header = "Header: Foobar\r\n"; + ByteBuffer buffer = BufferUtil.toBuffer( + "HTTP/1.1 200 OK\r\n" + + "Content-Length: 1\r\n" + + "\r\n" + + "0" + + header); + boolean handle = parser.parseNext(buffer); + assertTrue(handle); + assertTrue(buffer.hasRemaining()); + assertEquals(header, BufferUtil.toString(buffer)); + assertTrue(_contentCompleted); + assertFalse(_messageCompleted); + + // Need to parse more to advance the parser. + handle = parser.parseNext(buffer); + assertTrue(handle); + assertTrue(buffer.hasRemaining()); + assertEquals(header, BufferUtil.toString(buffer)); + assertTrue(_messageCompleted); + } + + @Test + public void testHeaderAfterSmallContentLengthContentCompleteTrue() + { + HttpParser.ResponseHandler handler = new Handler() + { + @Override + public boolean contentComplete() + { + super.contentComplete(); + return true; + } + }; + HttpParser parser = new HttpParser(handler); + + ByteBuffer buffer = BufferUtil.toBuffer( + "HTTP/1.1 200 OK\r\n" + + "Content-Length: 1\r\n" + + "\r\n" + + "0"); + boolean handle = parser.parseNext(buffer); + assertTrue(handle); + assertFalse(buffer.hasRemaining()); + assertTrue(_contentCompleted); + assertFalse(_messageCompleted); + + // Need to parse more to advance the parser. + handle = parser.parseNext(buffer); + assertTrue(handle); + assertFalse(buffer.hasRemaining()); + assertTrue(_messageCompleted); + } + + @Test + public void testEOFContentContentCompleteTrue() + { + HttpParser.ResponseHandler handler = new Handler() + { + @Override + public boolean contentComplete() + { + super.contentComplete(); + return true; + } + }; + HttpParser parser = new HttpParser(handler); + + ByteBuffer buffer = BufferUtil.toBuffer( + "HTTP/1.1 200 OK\r\n" + + "\r\n" + + "0"); + boolean handle = parser.parseNext(buffer); + assertFalse(handle); + assertFalse(buffer.hasRemaining()); + assertEquals("0", _content); + assertFalse(_contentCompleted); + assertFalse(_messageCompleted); + + parser.atEOF(); + + // Need to parse more to advance the parser. + handle = parser.parseNext(buffer); + assertTrue(handle); + assertFalse(buffer.hasRemaining()); + assertTrue(_contentCompleted); + assertFalse(_messageCompleted); + + // Need to parse more to advance the parser. + handle = parser.parseNext(buffer); + assertTrue(handle); + assertFalse(buffer.hasRemaining()); + assertTrue(_messageCompleted); + } + + @Test + public void testHEADRequestHeaderCompleteTrue() + { + HttpParser.ResponseHandler handler = new Handler() + { + @Override + public boolean headerComplete() + { + super.headerComplete(); + return true; + } + + @Override + public boolean contentComplete() + { + super.contentComplete(); + return true; + } + }; + HttpParser parser = new HttpParser(handler); + parser.setHeadResponse(true); + + ByteBuffer buffer = BufferUtil.toBuffer( + "HTTP/1.1 200 OK\r\n" + + "\r\n"); + boolean handle = parser.parseNext(buffer); + assertTrue(handle); + assertFalse(buffer.hasRemaining()); + assertFalse(_contentCompleted); + assertFalse(_messageCompleted); + + // Need to parse more to advance the parser. + handle = parser.parseNext(buffer); + assertTrue(handle); + assertFalse(buffer.hasRemaining()); + assertTrue(_contentCompleted); + assertFalse(_messageCompleted); + + // Need to parse more to advance the parser. + handle = parser.parseNext(buffer); + assertTrue(handle); + assertFalse(buffer.hasRemaining()); + assertTrue(_messageCompleted); + } + + @Test + public void testNoContentHeaderCompleteTrue() + { + HttpParser.ResponseHandler handler = new Handler() + { + @Override + public boolean headerComplete() + { + super.headerComplete(); + return true; + } + + @Override + public boolean contentComplete() + { + super.contentComplete(); + return true; + } + }; + HttpParser parser = new HttpParser(handler); + + // HTTP 304 does not have a body. + ByteBuffer buffer = BufferUtil.toBuffer( + "HTTP/1.1 304 Not Modified\r\n" + + "\r\n"); + boolean handle = parser.parseNext(buffer); + assertTrue(handle); + assertFalse(buffer.hasRemaining()); + assertFalse(_contentCompleted); + assertFalse(_messageCompleted); + + // Need to parse more to advance the parser. + handle = parser.parseNext(buffer); + assertTrue(handle); + assertFalse(buffer.hasRemaining()); + assertTrue(_contentCompleted); + assertFalse(_messageCompleted); + + // Need to parse more to advance the parser. + handle = parser.parseNext(buffer); + assertTrue(handle); + assertFalse(buffer.hasRemaining()); + assertTrue(_messageCompleted); + } + + @Test + public void testCRLFAfterResponseHeaderCompleteTrue() + { + HttpParser.ResponseHandler handler = new Handler() + { + @Override + public boolean headerComplete() + { + super.headerComplete(); + return true; + } + }; + HttpParser parser = new HttpParser(handler); + + ByteBuffer buffer = BufferUtil.toBuffer( + "HTTP/1.1 304 Not Modified\r\n" + + "\r\n" + + "\r\n" + + "\r\n" + + "HTTP/1.1 200 OK\r\n" + + "Content-Length: 0\r\n" + + "\r\n" + + "\r\n" + + "\r\n" + + "HTTP/1.1 303 See Other\r\n" + + "Content-Length: 0\r\n" + + "\r\n"); + boolean handle = parser.parseNext(buffer); + assertTrue(handle); + assertTrue(buffer.hasRemaining()); + assertEquals("304", _uriOrStatus); + assertFalse(_contentCompleted); + assertFalse(_messageCompleted); + + // Need to parse more to advance the parser. + handle = parser.parseNext(buffer); + assertTrue(handle); + assertTrue(buffer.hasRemaining()); + assertTrue(_contentCompleted); + assertTrue(_messageCompleted); + + // Parse next response. + parser.reset(); + init(); + handle = parser.parseNext(buffer); + assertTrue(handle); + assertTrue(buffer.hasRemaining()); + assertEquals("200", _uriOrStatus); + assertFalse(_contentCompleted); + assertFalse(_messageCompleted); + + // Need to parse more to advance the parser. + handle = parser.parseNext(buffer); + assertTrue(handle); + assertTrue(buffer.hasRemaining()); + assertTrue(_contentCompleted); + assertTrue(_messageCompleted); + + // Parse next response. + parser.reset(); + init(); + handle = parser.parseNext(buffer); + assertTrue(handle); + assertFalse(buffer.hasRemaining()); + assertEquals("303", _uriOrStatus); + assertFalse(_contentCompleted); + assertFalse(_messageCompleted); + + // Need to parse more to advance the parser. + handle = parser.parseNext(buffer); + assertTrue(handle); + assertFalse(buffer.hasRemaining()); + assertTrue(_contentCompleted); + assertTrue(_messageCompleted); + } + + @Test + public void testCRLFAfterResponseContentCompleteTrue() + { + HttpParser.ResponseHandler handler = new Handler() + { + @Override + public boolean contentComplete() + { + super.contentComplete(); + return true; + } + }; + HttpParser parser = new HttpParser(handler); + + ByteBuffer buffer = BufferUtil.toBuffer( + "HTTP/1.1 304 Not Modified\r\n" + + "\r\n" + + "\r\n" + + "\r\n" + + "HTTP/1.1 200 OK\r\n" + + "Content-Length: 0\r\n" + + "\r\n" + + "\r\n" + + "\r\n" + + "HTTP/1.1 303 See Other\r\n" + + "Content-Length: 0\r\n" + + "\r\n"); + boolean handle = parser.parseNext(buffer); + assertTrue(handle); + assertTrue(buffer.hasRemaining()); + assertEquals("304", _uriOrStatus); + assertTrue(_contentCompleted); + assertFalse(_messageCompleted); + + // Need to parse more to advance the parser. + handle = parser.parseNext(buffer); + assertTrue(handle); + assertTrue(buffer.hasRemaining()); + assertTrue(_messageCompleted); + + // Parse next response. + parser.reset(); + init(); + handle = parser.parseNext(buffer); + assertTrue(handle); + assertTrue(buffer.hasRemaining()); + assertEquals("200", _uriOrStatus); + assertTrue(_contentCompleted); + assertFalse(_messageCompleted); + + // Need to parse more to advance the parser. + handle = parser.parseNext(buffer); + assertTrue(handle); + assertTrue(buffer.hasRemaining()); + assertTrue(_messageCompleted); + + // Parse next response. + parser.reset(); + init(); + handle = parser.parseNext(buffer); + assertTrue(handle); + assertFalse(buffer.hasRemaining()); + assertEquals("303", _uriOrStatus); + assertTrue(_contentCompleted); + assertFalse(_messageCompleted); + + // Need to parse more to advance the parser. + handle = parser.parseNext(buffer); + assertTrue(handle); + assertFalse(buffer.hasRemaining()); + assertTrue(_messageCompleted); + } + + @Test + public void testCRLFAfterResponseMessageCompleteFalse() + { + HttpParser.ResponseHandler handler = new Handler() + { + @Override + public boolean messageComplete() + { + super.messageComplete(); + return false; + } + }; + HttpParser parser = new HttpParser(handler); + + ByteBuffer buffer = BufferUtil.toBuffer( + "HTTP/1.1 304 Not Modified\r\n" + + "\r\n" + + "\r\n" + + "\r\n" + + "HTTP/1.1 200 OK\r\n" + + "Content-Length: 0\r\n" + + "\r\n" + + "\r\n" + + "\r\n" + + "HTTP/1.1 303 See Other\r\n" + + "Content-Length: 0\r\n" + + "\r\n"); + boolean handle = parser.parseNext(buffer); + assertFalse(handle); + assertTrue(buffer.hasRemaining()); + assertEquals("304", _uriOrStatus); + assertTrue(_contentCompleted); + assertTrue(_messageCompleted); + + // Parse next response. + parser.reset(); + init(); + handle = parser.parseNext(buffer); + assertFalse(handle); + assertTrue(buffer.hasRemaining()); + assertEquals("200", _uriOrStatus); + assertTrue(_contentCompleted); + assertTrue(_messageCompleted); + + // Parse next response. + parser.reset(); + init(); + handle = parser.parseNext(buffer); + assertFalse(handle); + assertFalse(buffer.hasRemaining()); + assertEquals("303", _uriOrStatus); + assertTrue(_contentCompleted); + assertTrue(_messageCompleted); + } + + @Test + public void testSPAfterResponseMessageCompleteFalse() + { + HttpParser.ResponseHandler handler = new Handler() + { + @Override + public boolean messageComplete() + { + super.messageComplete(); + return false; + } + }; + HttpParser parser = new HttpParser(handler); + + ByteBuffer buffer = BufferUtil.toBuffer( + "HTTP/1.1 304 Not Modified\r\n" + + "\r\n" + + " " + // Single SP. + "HTTP/1.1 200 OK\r\n" + + "Content-Length: 0\r\n" + + "\r\n"); + boolean handle = parser.parseNext(buffer); + assertFalse(handle); + assertTrue(buffer.hasRemaining()); + assertEquals("304", _uriOrStatus); + assertTrue(_contentCompleted); + assertTrue(_messageCompleted); + + // Parse next response. + parser.reset(); + init(); + handle = parser.parseNext(buffer); + assertFalse(handle); + assertFalse(buffer.hasRemaining()); + assertNotNull(_bad); + + buffer = BufferUtil.toBuffer( + "HTTP/1.1 200 OK\r\n" + + "Content-Length: 0\r\n" + + "\r\n" + + " " + // Single SP. + "HTTP/1.1 303 See Other\r\n" + + "Content-Length: 0\r\n" + + "\r\n"); + parser = new HttpParser(handler); + handle = parser.parseNext(buffer); + assertFalse(handle); + assertTrue(buffer.hasRemaining()); + assertEquals("200", _uriOrStatus); + assertTrue(_contentCompleted); + assertTrue(_messageCompleted); + + // Parse next response. + parser.reset(); + init(); + handle = parser.parseNext(buffer); + assertFalse(handle); + assertFalse(buffer.hasRemaining()); + assertNotNull(_bad); + } + @BeforeEach public void init() { @@ -2239,6 +2863,7 @@ public class HttpParserTest _val = null; _headers = 0; _headerCompleted = false; + _contentCompleted = false; _messageCompleted = false; _complianceViolation.clear(); } @@ -2257,6 +2882,7 @@ public class HttpParserTest private int _headers; private boolean _early; private boolean _headerCompleted; + private boolean _contentCompleted; private boolean _messageCompleted; private final List _complianceViolation = new ArrayList<>(); @@ -2322,6 +2948,7 @@ public class HttpParserTest @Override public boolean contentComplete() { + _contentCompleted = true; return false; } From ca72a8fd7195f1006f71bbf5ab5d2a27c2d9afb5 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 11 Nov 2019 17:27:56 +1100 Subject: [PATCH 2/2] Issue #4282 HttpParser no content Added HTTP/0.9 test Signed-off-by: Greg Wilkins --- .../eclipse/jetty/http/HttpParserTest.java | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) 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 8c622f0cac9..7c89ac068b5 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 @@ -2228,6 +2228,39 @@ public class HttpParserTest assertNull(_bad); } + @Test + public void testForHTTP09HeaderCompleteTrueDoesNotEmitContentComplete() + { + HttpParser.RequestHandler handler = new Handler() + { + @Override + public boolean headerComplete() + { + super.headerComplete(); + return true; + } + }; + + HttpParser parser = new HttpParser(handler, HttpCompliance.RFC2616_LEGACY); + ByteBuffer buffer = BufferUtil.toBuffer("GET /path\r\n"); + boolean handle = parser.parseNext(buffer); + assertTrue(handle); + assertFalse(buffer.hasRemaining()); + assertFalse(_contentCompleted); + assertFalse(_messageCompleted); + + assertEquals("GET", _methodOrVersion); + assertEquals("/path", _uriOrStatus); + assertEquals("HTTP/0.9", _versionOrReason); + assertEquals(-1, _headers); + + // Need to parse more to advance the parser. + handle = parser.parseNext(buffer); + assertTrue(handle); + assertTrue(_contentCompleted); + assertTrue(_messageCompleted); + } + @Test public void testForContentLengthZeroHeaderCompleteTrueDoesNotEmitContentComplete() {