Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-12-SP4:Update
tomcat.30738
tomcat-8.0.53-CVE-2020-1935.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File tomcat-8.0.53-CVE-2020-1935.patch of Package tomcat.30738
Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/AbstractHttp11Protocol.java =================================================================== --- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/AbstractHttp11Protocol.java +++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/AbstractHttp11Protocol.java @@ -78,27 +78,56 @@ public abstract class AbstractHttp11Prot } - private boolean rejectIllegalHeaderName = false; + private boolean rejectIllegalHeader = false; /** - * If an HTTP request is received that contains an illegal header name (i.e. - * the header name is not a token) will the request be rejected (with a 400 - * response) or will the illegal header be ignored. + * If an HTTP request is received that contains an illegal header name or + * value (e.g. the header name is not a token) will the request be rejected + * (with a 400 response) or will the illegal header be ignored? * * @return {@code true} if the request will be rejected or {@code false} if * the header will be ignored */ - public boolean getRejectIllegalHeaderName() { return rejectIllegalHeaderName; } + public boolean getRejectIllegalHeader() { return rejectIllegalHeader; } /** - * If an HTTP request is received that contains an illegal header name (i.e. - * the header name is not a token) should the request be rejected (with a - * 400 response) or should the illegal header be ignored. + * If an HTTP request is received that contains an illegal header name or + * value (e.g. the header name is not a token) should the request be + * rejected (with a 400 response) or should the illegal header be ignored? + * + * @param rejectIllegalHeader {@code true} to reject requests with illegal + * header names or values, {@code false} to + * ignore the header + */ + public void setRejectIllegalHeader(boolean rejectIllegalHeader) { + this.rejectIllegalHeader = rejectIllegalHeader; + } + /** + * If an HTTP request is received that contains an illegal header name or + * value (e.g. the header name is not a token) will the request be rejected + * (with a 400 response) or will the illegal header be ignored? + * + * @return {@code true} if the request will be rejected or {@code false} if + * the header will be ignored + * + * @deprecated Now an alias for {@link #getRejectIllegalHeader()}. Will be + * removed in Tomcat 10 onwards. + */ + @Deprecated + public boolean getRejectIllegalHeaderName() { return rejectIllegalHeader; } + /** + * If an HTTP request is received that contains an illegal header name or + * value (e.g. the header name is not a token) should the request be + * rejected (with a 400 response) or should the illegal header be ignored? * * @param rejectIllegalHeaderName {@code true} to reject requests with - * illegal header names, {@code false} to - * ignore the header + * illegal header names or values, + * {@code false} to ignore the header + * + * @deprecated Now an alias for {@link #setRejectIllegalHeader(boolean)}. + * Will be removed in Tomcat 10 onwards. */ + @Deprecated public void setRejectIllegalHeaderName(boolean rejectIllegalHeaderName) { - this.rejectIllegalHeaderName = rejectIllegalHeaderName; + this.rejectIllegalHeader = rejectIllegalHeaderName; } Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/AbstractInputBuffer.java =================================================================== --- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/AbstractInputBuffer.java +++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/AbstractInputBuffer.java @@ -110,6 +110,11 @@ public abstract class AbstractInputBuffe protected int lastActiveFilter; + /** + * Do HTTP headers with illegal names and/or values cause the request to be + * rejected? Note that the field name is not quite right but cannot be + * easily changed without breaking binary compatibility. + */ protected boolean rejectIllegalHeaderName; Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11AprProcessor.java =================================================================== --- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/Http11AprProcessor.java +++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11AprProcessor.java @@ -61,7 +61,7 @@ public class Http11AprProcessor extends // ----------------------------------------------------------- Constructors - public Http11AprProcessor(int headerBufferSize, boolean rejectIllegalHeaderName, + public Http11AprProcessor(int headerBufferSize, boolean rejectIllegalHeader, AprEndpoint endpoint, int maxTrailerSize, Set<String> allowedTrailerHeaders, int maxExtensionSize, int maxSwallowSize, String relaxedPathChars, String relaxedQueryChars) { @@ -70,7 +70,7 @@ public class Http11AprProcessor extends httpParser = new HttpParser(relaxedPathChars, relaxedQueryChars); - inputBuffer = new InternalAprInputBuffer(request, headerBufferSize, rejectIllegalHeaderName, + inputBuffer = new InternalAprInputBuffer(request, headerBufferSize, rejectIllegalHeader, httpParser); request.setInputBuffer(inputBuffer); Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11AprProtocol.java =================================================================== --- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/Http11AprProtocol.java +++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11AprProtocol.java @@ -290,7 +290,7 @@ public class Http11AprProtocol extends A @Override protected Http11AprProcessor createProcessor() { Http11AprProcessor processor = new Http11AprProcessor( - proto.getMaxHttpHeaderSize(), proto.getRejectIllegalHeaderName(), + proto.getMaxHttpHeaderSize(), proto.getRejectIllegalHeader(), (AprEndpoint)proto.endpoint, proto.getMaxTrailerSize(), proto.getAllowedTrailerHeadersAsSet(), proto.getMaxExtensionSize(), proto.getMaxSwallowSize(), proto.getRelaxedPathChars(), Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11NioProcessor.java =================================================================== --- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/Http11NioProcessor.java +++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11NioProcessor.java @@ -65,7 +65,7 @@ public class Http11NioProcessor extends // ----------------------------------------------------------- Constructors - public Http11NioProcessor(int maxHttpHeaderSize, boolean rejectIllegalHeaderName, + public Http11NioProcessor(int maxHttpHeaderSize, boolean rejectIllegalHeader, NioEndpoint endpoint, int maxTrailerSize, Set<String> allowedTrailerHeaders, int maxExtensionSize, int maxSwallowSize, String relaxedPathChars, String relaxedQueryChars) { @@ -75,7 +75,7 @@ public class Http11NioProcessor extends httpParser = new HttpParser(relaxedPathChars, relaxedQueryChars); inputBuffer = new InternalNioInputBuffer(request, maxHttpHeaderSize, - rejectIllegalHeaderName, httpParser); + rejectIllegalHeader, httpParser); request.setInputBuffer(inputBuffer); outputBuffer = new InternalNioOutputBuffer(response, maxHttpHeaderSize); Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11NioProtocol.java =================================================================== --- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/Http11NioProtocol.java +++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11NioProtocol.java @@ -264,7 +264,7 @@ public class Http11NioProtocol extends A @Override public Http11NioProcessor createProcessor() { Http11NioProcessor processor = new Http11NioProcessor( - proto.getMaxHttpHeaderSize(), proto.getRejectIllegalHeaderName(), + proto.getMaxHttpHeaderSize(), proto.getRejectIllegalHeader(), (NioEndpoint)proto.endpoint, proto.getMaxTrailerSize(), proto.getAllowedTrailerHeadersAsSet(), proto.getMaxExtensionSize(), proto.getMaxSwallowSize(), proto.getRelaxedPathChars(), Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11Processor.java =================================================================== --- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/Http11Processor.java +++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11Processor.java @@ -50,7 +50,7 @@ public class Http11Processor extends Abs // ------------------------------------------------------------ Constructor - public Http11Processor(int headerBufferSize, boolean rejectIllegalHeaderName, + public Http11Processor(int headerBufferSize, boolean rejectIllegalHeader, JIoEndpoint endpoint, int maxTrailerSize, Set<String> allowedTrailerHeaders, int maxExtensionSize, int maxSwallowSize, String relaxedPathChars, String relaxedQueryChars) { @@ -59,7 +59,7 @@ public class Http11Processor extends Abs httpParser = new HttpParser(relaxedPathChars, relaxedQueryChars); - inputBuffer = new InternalInputBuffer(request, headerBufferSize, rejectIllegalHeaderName, + inputBuffer = new InternalInputBuffer(request, headerBufferSize, rejectIllegalHeader, httpParser); request.setInputBuffer(inputBuffer); Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11Protocol.java =================================================================== --- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/Http11Protocol.java +++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11Protocol.java @@ -165,7 +165,7 @@ public class Http11Protocol extends Abst @Override protected Http11Processor createProcessor() { Http11Processor processor = new Http11Processor( - proto.getMaxHttpHeaderSize(), proto.getRejectIllegalHeaderName(), + proto.getMaxHttpHeaderSize(), proto.getRejectIllegalHeader(), (JIoEndpoint)proto.endpoint, proto.getMaxTrailerSize(), proto.getAllowedTrailerHeadersAsSet(), proto.getMaxExtensionSize(), proto.getMaxSwallowSize(), proto.getRelaxedPathChars(), Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/InternalAprInputBuffer.java =================================================================== --- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/InternalAprInputBuffer.java +++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/InternalAprInputBuffer.java @@ -54,7 +54,7 @@ public class InternalAprInputBuffer exte * Alternate constructor. */ public InternalAprInputBuffer(Request request, int headerBufferSize, - boolean rejectIllegalHeaderName, HttpParser httpParser) { + boolean rejectIllegalHeader, HttpParser httpParser) { this.request = request; headers = request.getMimeHeaders(); @@ -66,7 +66,7 @@ public class InternalAprInputBuffer exte bbuf = ByteBuffer.allocateDirect((headerBufferSize / 1500 + 1) * 1500); } - this.rejectIllegalHeaderName = rejectIllegalHeaderName; + this.rejectIllegalHeaderName = rejectIllegalHeader; this.httpParser = httpParser; inputStreamInputBuffer = new SocketInputBuffer(); @@ -354,6 +354,8 @@ public class InternalAprInputBuffer exte // byte chr = 0; + byte prevChr = 0; + while (true) { // Read new bytes if needed @@ -362,19 +364,23 @@ public class InternalAprInputBuffer exte throw new EOFException(sm.getString("iib.eof.error")); } + prevChr = chr; chr = buf[pos]; - if (chr == Constants.CR) { - // Skip - } else if (chr == Constants.LF) { - pos++; + if (chr == Constants.CR && prevChr != Constants.CR) { + // Possible start of CRLF - process the next byte. + } else if (prevChr == Constants.CR && chr == Constants.LF) { + pos++; return false; } else { + if (prevChr == Constants.CR) { + // Must have read two bytes (first was CR, second was not LF) + pos--; + } break; } pos++; - } // Mark the current buffer position @@ -458,10 +464,24 @@ public class InternalAprInputBuffer exte throw new EOFException(sm.getString("iib.eof.error")); } - if (buf[pos] == Constants.CR) { - // Skip - } else if (buf[pos] == Constants.LF) { + prevChr = chr; + chr = buf[pos]; + if (chr == Constants.CR) { + // Possible start of CRLF - process the next byte. + } else if (prevChr == Constants.CR && chr == Constants.LF) { eol = true; + } else if (prevChr == Constants.CR) { + // Invalid value + // Delete the header (it will be the most recent one) + headers.removeHeader(headers.size() - 1); + skipLine(start); + return true; + } else if (chr != Constants.HT && HttpParser.isControl(chr)) { + // Invalid value + // Delete the header (it will be the most recent one) + headers.removeHeader(headers.size() - 1); + skipLine(start); + return true; } else if (buf[pos] == Constants.SP) { buf[realPos] = buf[pos]; realPos++; @@ -514,6 +534,9 @@ public class InternalAprInputBuffer exte lastRealByte = pos - 1; } + byte chr = 0; + byte prevChr = 0; + while (!eol) { // Read new bytes if needed @@ -522,9 +545,12 @@ public class InternalAprInputBuffer exte throw new EOFException(sm.getString("iib.eof.error")); } - if (buf[pos] == Constants.CR) { + prevChr = chr; + chr = buf[pos]; + + if (chr == Constants.CR) { // Skip - } else if (buf[pos] == Constants.LF) { + } else if (prevChr == Constants.CR && chr == Constants.LF) { eol = true; } else { lastRealByte = pos; Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/InternalInputBuffer.java =================================================================== --- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/InternalInputBuffer.java +++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/InternalInputBuffer.java @@ -53,14 +53,14 @@ public class InternalInputBuffer extends * Default constructor. */ public InternalInputBuffer(Request request, int headerBufferSize, - boolean rejectIllegalHeaderName, HttpParser httpParser) { + boolean rejectIllegalHeader, HttpParser httpParser) { this.request = request; headers = request.getMimeHeaders(); buf = new byte[headerBufferSize]; - this.rejectIllegalHeaderName = rejectIllegalHeaderName; + this.rejectIllegalHeaderName = rejectIllegalHeader; this.httpParser = httpParser; inputStreamInputBuffer = new InputStreamInputBuffer(); @@ -313,6 +313,8 @@ public class InternalInputBuffer extends // byte chr = 0; + byte prevChr = 0; + while (true) { // Read new bytes if needed @@ -321,19 +323,23 @@ public class InternalInputBuffer extends throw new EOFException(sm.getString("iib.eof.error")); } + prevChr = chr; chr = buf[pos]; - if (chr == Constants.CR) { - // Skip - } else if (chr == Constants.LF) { + if (chr == Constants.CR && prevChr != Constants.CR) { + // Possible start of CRLF - process the next byte. + } else if (prevChr == Constants.CR && chr == Constants.LF) { pos++; return false; } else { + if (prevChr == Constants.CR) { + // Must have read two bytes (first was CR, second was not LF) + pos--; + } break; } pos++; - } // Mark the current buffer position @@ -418,15 +424,29 @@ public class InternalInputBuffer extends throw new EOFException(sm.getString("iib.eof.error")); } - if (buf[pos] == Constants.CR) { - // Skip - } else if (buf[pos] == Constants.LF) { + prevChr = chr; + chr = buf[pos]; + if (chr == Constants.CR) { + // Possible start of CRLF - process the next byte. + } else if (prevChr == Constants.CR && chr == Constants.LF) { eol = true; - } else if (buf[pos] == Constants.SP) { - buf[realPos] = buf[pos]; + } else if (prevChr == Constants.CR) { + // Invalid value + // Delete the header (it will be the most recent one) + headers.removeHeader(headers.size() - 1); + skipLine(start); + return true; + } else if (chr != Constants.HT && HttpParser.isControl(chr)) { + // Invalid value + // Delete the header (it will be the most recent one) + headers.removeHeader(headers.size() - 1); + skipLine(start); + return true; + } else if (chr == Constants.SP) { + buf[realPos] = chr; realPos++; } else { - buf[realPos] = buf[pos]; + buf[realPos] = chr; realPos++; lastSignificantChar = realPos; } @@ -492,6 +512,9 @@ public class InternalInputBuffer extends lastRealByte = pos - 1; } + byte chr = 0; + byte prevChr = 0; + while (!eol) { // Read new bytes if needed @@ -500,9 +523,12 @@ public class InternalInputBuffer extends throw new EOFException(sm.getString("iib.eof.error")); } - if (buf[pos] == Constants.CR) { + prevChr = chr; + chr = buf[pos]; + + if (chr == Constants.CR) { // Skip - } else if (buf[pos] == Constants.LF) { + } else if (prevChr == Constants.CR && chr == Constants.LF) { eol = true; } else { lastRealByte = pos; Index: apache-tomcat-8.0.53-src/java/org/apache/tomcat/util/http/MimeHeaders.java =================================================================== --- apache-tomcat-8.0.53-src.orig/java/org/apache/tomcat/util/http/MimeHeaders.java +++ apache-tomcat-8.0.53-src/java/org/apache/tomcat/util/http/MimeHeaders.java @@ -366,7 +366,7 @@ public class MimeHeaders { * reset and swap with last header * @param idx the index of the header to remove. */ - private void removeHeader(int idx) { + public void removeHeader(int idx) { MimeHeaderField mh = headers[idx]; mh.recycle(); Index: apache-tomcat-8.0.53-src/java/org/apache/tomcat/util/http/parser/HttpParser.java =================================================================== --- apache-tomcat-8.0.53-src.orig/java/org/apache/tomcat/util/http/parser/HttpParser.java +++ apache-tomcat-8.0.53-src/java/org/apache/tomcat/util/http/parser/HttpParser.java @@ -337,6 +337,17 @@ public class HttpParser { } + public static boolean isControl(int c) { + // Fast for valid control characters, slower for some incorrect + // ones + try { + return IS_CONTROL[c]; + } catch (ArrayIndexOutOfBoundsException ex) { + return false; + } + } + + // Skip any LWS and position to read the next character. The next character // is returned as being able to 'peek()' it allows a small optimisation in // some cases. Index: apache-tomcat-8.0.53-src/test/org/apache/coyote/http11/TestInternalInputBuffer.java =================================================================== --- apache-tomcat-8.0.53-src.orig/test/org/apache/coyote/http11/TestInternalInputBuffer.java +++ apache-tomcat-8.0.53-src/test/org/apache/coyote/http11/TestInternalInputBuffer.java @@ -163,13 +163,13 @@ public class TestInternalInputBuffer ext @Test - public void testBug51557Separators() throws Exception { + public void testBug51557SeparatorsInName() throws Exception { char httpSeparators[] = new char[] { '\t', ' ', '\"', '(', ')', ',', '/', ':', ';', '<', '=', '>', '?', '@', '[', '\\', ']', '{', '}' }; for (char s : httpSeparators) { - doTestBug51557Char(s); + doTestBug51557CharInName(s); tearDown(); setUp(); } @@ -177,13 +177,38 @@ public class TestInternalInputBuffer ext @Test - public void testBug51557Ctl() throws Exception { + public void testBug51557CtlInName() throws Exception { for (int i = 0; i < 31; i++) { - doTestBug51557Char((char) i); + doTestBug51557CharInName((char) i); + tearDown(); + setUp(); + } + doTestBug51557CharInName((char) 127); + } + + + @Test + public void testBug51557CtlInValue() throws Exception { + for (int i = 0; i < 31; i++) { + if (i == '\t') { + // TAB is allowed + continue; + } + doTestBug51557InvalidCharInValue((char) i); + tearDown(); + setUp(); + } + doTestBug51557InvalidCharInValue((char) 127); + } + + + @Test + public void testBug51557ObsTextInValue() throws Exception { + for (int i = 128; i < 255; i++) { + doTestBug51557ValidCharInValue((char) i); tearDown(); setUp(); } - doTestBug51557Char((char) 127); } @@ -226,7 +251,33 @@ public class TestInternalInputBuffer ext } - private void doTestBug51557Char(char s) { + @Test + public void testBug51557CRStartName() { + + Bug51557Client client = new Bug51557Client("\rName=", + "invalid"); + + client.doRequest(); + Assert.assertTrue(client.isResponse200()); + Assert.assertEquals("abcd", client.getResponseBody()); + Assert.assertTrue(client.isResponseBodyOK()); + } + + + @Test + public void testBug51557CR2StartName() { + + Bug51557Client client = new Bug51557Client("\r\rName=", + "invalid"); + + client.doRequest(); + Assert.assertTrue(client.isResponse200()); + Assert.assertEquals("abcd", client.getResponseBody()); + Assert.assertTrue(client.isResponseBodyOK()); + } + + + private void doTestBug51557CharInName(char s) { Bug51557Client client = new Bug51557Client("X-Bug" + s + "51557", "invalid"); @@ -236,6 +287,29 @@ public class TestInternalInputBuffer ext Assert.assertTrue(client.isResponseBodyOK()); } + + private void doTestBug51557InvalidCharInValue(char s) { + Bug51557Client client = + new Bug51557Client("X-Bug51557-Invalid", "invalid" + s + "invalid"); + + client.doRequest(); + Assert.assertTrue("Testing [" + (int) s + "]", client.isResponse200()); + Assert.assertEquals("Testing [" + (int) s + "]", "abcd", client.getResponseBody()); + Assert.assertTrue(client.isResponseBodyOK()); + } + + + private void doTestBug51557ValidCharInValue(char s) { + Bug51557Client client = + new Bug51557Client("X-Bug51557-Valid", "valid" + s + "valid"); + + client.doRequest(); + Assert.assertTrue("Testing [" + (int) s + "]", client.isResponse200()); + Assert.assertEquals("Testing [" + (int) s + "]", "valid" + s + "validabcd", client.getResponseBody()); + Assert.assertTrue(client.isResponseBodyOK()); + } + + /** * Bug 51557 test client. */ @@ -243,12 +317,12 @@ public class TestInternalInputBuffer ext private final String headerName; private final String headerLine; - private final boolean rejectIllegalHeaderName; + private final boolean rejectIllegalHeader; public Bug51557Client(String headerName) { this.headerName = headerName; this.headerLine = headerName; - this.rejectIllegalHeaderName = false; + this.rejectIllegalHeader = false; } public Bug51557Client(String headerName, String headerValue) { @@ -256,10 +330,10 @@ public class TestInternalInputBuffer ext } public Bug51557Client(String headerName, String headerValue, - boolean rejectIllegalHeaderName) { + boolean rejectIllegalHeader) { this.headerName = headerName; this.headerLine = headerName + ": " + headerValue; - this.rejectIllegalHeaderName = rejectIllegalHeaderName; + this.rejectIllegalHeader = rejectIllegalHeader; } private Exception doRequest() { @@ -273,8 +347,8 @@ public class TestInternalInputBuffer ext try { Connector connector = tomcat.getConnector(); - connector.setProperty("rejectIllegalHeaderName", - Boolean.toString(rejectIllegalHeaderName)); + Assert.assertTrue(connector.setProperty( + "rejectIllegalHeader", Boolean.toString(rejectIllegalHeader))); tomcat.start(); setPort(connector.getLocalPort()); @@ -557,6 +631,75 @@ public class TestInternalInputBuffer ext "X-Header: Ignore" + CRLF + "X-Header" + (char) 130 + ": Broken" + CRLF + CRLF; + setRequest(request); + processRequest(); // blocks until response has been read + + // Close the connection + disconnect(); + } catch (Exception e) { + return e; + } + return null; + } + + @Override + public boolean isResponseBodyOK() { + if (getResponseBody() == null) { + return false; + } + if (!getResponseBody().contains("OK")) { + return false; + } + return true; + } + } + + + /** + * Test case for https://bz.apache.org/bugzilla/show_bug.cgi?id=59089 + */ + @Test + public void testBug59089() { + + Bug59089Client client = new Bug59089Client(); + + client.doRequest(); + Assert.assertTrue(client.isResponse200()); + Assert.assertTrue(client.isResponseBodyOK()); + } + + + /** + * Bug 59089 test client. + */ + private class Bug59089Client extends SimpleHttpClient { + + private Exception doRequest() { + + // Ensure body is read correctly + setUseContentLength(true); + + Tomcat tomcat = getTomcatInstance(); + + Context root = tomcat.addContext("", TEMP_DIR); + Tomcat.addServlet(root, "Bug59089", new TesterServlet()); + root.addServletMapping("/test", "Bug59089"); + + try { + Connector connector = tomcat.getConnector(); + Assert.assertTrue(connector.setProperty("rejectIllegalHeader", "false")); + tomcat.start(); + setPort(connector.getLocalPort()); + + // Open connection + connect(); + + String[] request = new String[1]; + request[0] = "GET http://localhost:8080/test HTTP/1.1" + CRLF + + "Host: localhost:8080" + CRLF + + "X-Header: Ignore" + CRLF + + "X-Header" + (char) 130 + ": Broken" + CRLF + CRLF; + setRequest(request); processRequest(); // blocks until response has been read Index: apache-tomcat-8.0.53-src/webapps/docs/changelog.xml =================================================================== --- apache-tomcat-8.0.53-src.orig/webapps/docs/changelog.xml +++ apache-tomcat-8.0.53-src/webapps/docs/changelog.xml @@ -174,6 +174,11 @@ not contain leading zeros in the IPv4 part. Based on a patch by Katya Stoycheva. (markt) </fix> + <fix> + Rename the HTTP Connector attribute <code>rejectIllegalHeaderName</code> + to <code>rejectIllegalHeader</code> and expand the underlying + implementation to include header values as well as names. (markt) + </fix> </changelog> </subsection> <subsection name="Jasper"> Index: apache-tomcat-8.0.53-src/webapps/docs/config/http.xml =================================================================== --- apache-tomcat-8.0.53-src.orig/webapps/docs/config/http.xml +++ apache-tomcat-8.0.53-src/webapps/docs/config/http.xml @@ -529,15 +529,20 @@ expected concurrent requests (synchronous and asynchronous).</p> </attribute> - <attribute name="rejectIllegalHeaderName" required="false"> - <p>If an HTTP request is received that contains an illegal header name - (i.e. the header name is not a token) this setting determines if the + <attribute name="rejectIllegalHeader" required="false"> + <p>If an HTTP request is received that contains an illegal header name or + value (e.g. the header name is not a token) this setting determines if the request will be rejected with a 400 response (<code>true</code>) or if the illegal header be ignored (<code>false</code>). The default value is <code>false</code> which will cause the request to be processed but the illegal header will be ignored.</p> </attribute> + <attribute name="rejectIllegalHeaderName" required="false"> + <p>This attribute is deprecated. It will be removed in Tomcat 10 onwards. + It is now an alias for <strong>rejectIllegalHeader</strong>.</p> + </attribute> + <attribute name="relaxedPathChars" required="false"> <p>The <a href="https://tools.ietf.org/rfc/rfc7230.txt">HTTP/1.1 specification</a> requires that certain characters are %nn encoded when Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/AbstractNioInputBuffer.java =================================================================== --- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/AbstractNioInputBuffer.java +++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/AbstractNioInputBuffer.java @@ -77,13 +77,13 @@ public abstract class AbstractNioInputBu * Alternate constructor. */ public AbstractNioInputBuffer(Request request, int headerBufferSize, - boolean rejectIllegalHeaderName, HttpParser httpParser) { + boolean rejectIllegalHeader, HttpParser httpParser) { this.request = request; headers = request.getMimeHeaders(); this.headerBufferSize = headerBufferSize; - this.rejectIllegalHeaderName = rejectIllegalHeaderName; + this.rejectIllegalHeaderName = rejectIllegalHeader; this.httpParser = httpParser; filterLibrary = new InputFilter[0]; @@ -428,6 +428,8 @@ public abstract class AbstractNioInputBu // byte chr = 0; + byte prevChr = 0; + while (headerParsePos == HeaderParsePosition.HEADER_START) { // Read new bytes if needed @@ -438,19 +440,23 @@ public abstract class AbstractNioInputBu } } + prevChr = chr; chr = buf[pos]; - if (chr == Constants.CR) { - // Skip - } else if (chr == Constants.LF) { + if (chr == Constants.CR && prevChr != Constants.CR) { + // Possible start of CRLF - process the next byte. + } else if (prevChr == Constants.CR && chr == Constants.LF) { pos++; return HeaderParseStatus.DONE; } else { + if (prevChr == Constants.CR) { + // Must have read two bytes (first was CR, second was not LF) + pos--; + } break; } pos++; - } if ( headerParsePos == HeaderParsePosition.HEADER_START ) { @@ -545,11 +551,22 @@ public abstract class AbstractNioInputBu } } + prevChr = chr; chr = buf[pos]; if (chr == Constants.CR) { - // Skip - } else if (chr == Constants.LF) { + // Possible start of CRLF - process the next byte. + } else if (prevChr == Constants.CR && chr == Constants.LF) { eol = true; + } else if (prevChr == Constants.CR) { + // Invalid value + // Delete the header (it will be the most recent one) + headers.removeHeader(headers.size() - 1); + return skipLine(); + } else if (chr != Constants.HT && HttpParser.isControl(chr)) { + // Invalid value + // Delete the header (it will be the most recent one) + headers.removeHeader(headers.size() - 1); + return skipLine(); } else if (chr == Constants.SP || chr == Constants.HT) { buf[headerData.realPos] = chr; headerData.realPos++; @@ -606,6 +623,9 @@ public abstract class AbstractNioInputBu headerParsePos = HeaderParsePosition.HEADER_SKIPLINE; boolean eol = false; + byte chr = 0; + byte prevChr = 0; + // Reading bytes until the end of the line while (!eol) { @@ -616,9 +636,12 @@ public abstract class AbstractNioInputBu } } - if (buf[pos] == Constants.CR) { + prevChr = chr; + chr = buf[pos]; + + if (chr == Constants.CR) { // Skip - } else if (buf[pos] == Constants.LF) { + } else if (prevChr == Constants.CR && chr == Constants.LF) { eol = true; } else { headerData.lastSignificantChar = pos;
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
