This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 8.5.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push: new 040d899 Fix BZ 65272. Restore the use of LF as an HTTP line terminator 040d899 is described below commit 040d8993e491b7deece7a09da7be7b11642f444f Author: Mark Thomas <ma...@apache.org> AuthorDate: Wed Apr 28 18:23:07 2021 +0100 Fix BZ 65272. Restore the use of LF as an HTTP line terminator --- .../apache/coyote/http11/Http11InputBuffer.java | 16 ++++++++++++---- .../coyote/http11/TestHttp11InputBuffer.java | 22 ++++------------------ .../coyote/http11/TestHttp11InputBufferCRLF.java | 8 ++++++++ webapps/docs/changelog.xml | 7 +++++++ 4 files changed, 31 insertions(+), 22 deletions(-) diff --git a/java/org/apache/coyote/http11/Http11InputBuffer.java b/java/org/apache/coyote/http11/Http11InputBuffer.java index b8fcee3..c50b4d1 100644 --- a/java/org/apache/coyote/http11/Http11InputBuffer.java +++ b/java/org/apache/coyote/http11/Http11InputBuffer.java @@ -564,10 +564,15 @@ public class Http11InputBuffer implements InputBuffer, ApplicationBufferHandler prevChr = chr; chr = byteBuffer.get(); if (chr == Constants.CR) { - // Possible end of request line. Need LF next. + // Possible end of request line. Need LF next else invalid. } else if (prevChr == Constants.CR && chr == Constants.LF) { + // CRLF is the standard line terminator end = pos - 1; parsingRequestLineEol = true; + } else if (chr == Constants.LF) { + // LF is an optional line terminator + end = pos; + parsingRequestLineEol = true; } else if (prevChr == Constants.CR || !HttpParser.isHttpProtocol(chr)) { String invalidProtocol = parseInvalid(parsingRequestLineStart, byteBuffer); throw new IllegalArgumentException(sm.getString("iib.invalidHttpProtocol", invalidProtocol)); @@ -840,7 +845,8 @@ public class Http11InputBuffer implements InputBuffer, ApplicationBufferHandler if (chr == Constants.CR && prevChr != Constants.CR) { // Possible start of CRLF - process the next byte. - } else if (prevChr == Constants.CR && chr == Constants.LF) { + } else if (chr == Constants.LF) { + // CRLF or LF is an acceptable line terminator return HeaderParseStatus.DONE; } else { if (prevChr == Constants.CR) { @@ -952,7 +958,8 @@ public class Http11InputBuffer implements InputBuffer, ApplicationBufferHandler chr = byteBuffer.get(); if (chr == Constants.CR) { // Possible start of CRLF - process the next byte. - } else if (prevChr == Constants.CR && chr == Constants.LF) { + } else if (chr == Constants.LF) { + // CRLF or LF is an acceptable line terminator eol = true; } else if (prevChr == Constants.CR) { // Invalid value @@ -1030,7 +1037,8 @@ public class Http11InputBuffer implements InputBuffer, ApplicationBufferHandler chr = byteBuffer.get(); if (chr == Constants.CR) { // Skip - } else if (prevChr == Constants.CR && chr == Constants.LF) { + } else if (chr == Constants.LF) { + // CRLF or LF is an acceptable line terminator eol = true; } else { headerData.lastSignificantChar = pos; diff --git a/test/org/apache/coyote/http11/TestHttp11InputBuffer.java b/test/org/apache/coyote/http11/TestHttp11InputBuffer.java index ea38be0..36a65e1 100644 --- a/test/org/apache/coyote/http11/TestHttp11InputBuffer.java +++ b/test/org/apache/coyote/http11/TestHttp11InputBuffer.java @@ -198,6 +198,10 @@ public class TestHttp11InputBuffer extends TomcatBaseTest { // TAB is allowed continue; } + if (i == '\n') { + // LF is the optional line terminator + continue; + } doTestBug51557InvalidCharInValue((char) i); tearDown(); setUp(); @@ -688,24 +692,6 @@ public class TestHttp11InputBuffer extends TomcatBaseTest { @Test - public void testInvalidEndOfRequestLine02() { - - String[] request = new String[1]; - request[0] = - "GET /test HTTP/1.1" + LF + - "Host: localhost:8080" + CRLF + - "Connection: close" + CRLF + - CRLF; - - InvalidClient client = new InvalidClient(request); - - client.doRequest(); - Assert.assertTrue(client.getResponseLine(), client.isResponse400()); - Assert.assertTrue(client.isResponseBodyOK()); - } - - - @Test public void testInvalidHeader01() { String[] request = new String[1]; diff --git a/test/org/apache/coyote/http11/TestHttp11InputBufferCRLF.java b/test/org/apache/coyote/http11/TestHttp11InputBufferCRLF.java index a953031..2753c21 100644 --- a/test/org/apache/coyote/http11/TestHttp11InputBufferCRLF.java +++ b/test/org/apache/coyote/http11/TestHttp11InputBufferCRLF.java @@ -109,6 +109,14 @@ public class TestHttp11InputBufferCRLF extends TomcatBaseTest { CRLF, Boolean.FALSE, Boolean.FALSE, parameterSets); + // Standard HTTP/1.1 request using LF rather than CRLF + addRequestWithSplits("GET /test HTTP/1.1" + LF + + "Host: localhost:8080" + LF + + "Connection: close" + LF + + LF, + Boolean.FALSE, parameterSets); + + return parameterSets; } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index d5bbb68..5ce8918 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -139,6 +139,13 @@ request line, ensure that all the available data is included in the error message. (markt) </fix> + <fix> + <bug>65272</bug>: Restore the optional HTTP feature that allows + <code>LF</code> to be treated as a line terminator for the request line + and/or HTTP headers lines as well as the standard <code>CRLF</code>. + This behaviour was previously removed as a side-effect of the fix for + CVE-2020-1935. (markt) + </fix> </changelog> </subsection> <subsection name="Jasper"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org