This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push: new d8ece17eac Fix BZ 66512 - Ignore invalid HHTP response headers for AJP d8ece17eac is described below commit d8ece17eac0c1a9c40a810eacc3c8370f10fa32e Author: Mark Thomas <ma...@apache.org> AuthorDate: Mon Mar 6 20:41:44 2023 +0000 Fix BZ 66512 - Ignore invalid HHTP response headers for AJP This aligns the behaviour of AJP with that of HTTP. https://bz.apache.org/bugzilla/show_bug.cgi?id=66512 --- java/org/apache/coyote/ajp/AjpProcessor.java | 56 ++++++++++------ java/org/apache/coyote/ajp/LocalStrings.properties | 1 + .../coyote/ajp/TestAbstractAjpProcessor.java | 75 ++++++++++++++++++++-- webapps/docs/changelog.xml | 4 ++ 4 files changed, 111 insertions(+), 25 deletions(-) diff --git a/java/org/apache/coyote/ajp/AjpProcessor.java b/java/org/apache/coyote/ajp/AjpProcessor.java index d2c0e46169..2b76af3eb2 100644 --- a/java/org/apache/coyote/ajp/AjpProcessor.java +++ b/java/org/apache/coyote/ajp/AjpProcessor.java @@ -984,32 +984,48 @@ public class AjpProcessor extends AbstractProcessor { headers.setValue("Content-Length").setLong(contentLength); } - // Write AJP message header tmpMB.recycle(); responseMsgPos = -1; - responseMessage.reset(); - responseMessage.appendByte(Constants.JK_AJP13_SEND_HEADERS); - // Write HTTP response line - responseMessage.appendInt(statusCode); - // Reason phrase is optional but mod_jk + httpd 2.x fails with a null - // reason phrase - bug 45026 - tmpMB.setString(Integer.toString(response.getStatus())); - responseMessage.appendBytes(tmpMB); - - // Write headers int numHeaders = headers.size(); - responseMessage.appendInt(numHeaders); for (int i = 0; i < numHeaders; i++) { - MessageBytes hN = headers.getName(i); - int hC = Constants.getResponseAjpIndex(hN.toString()); - if (hC > 0) { - responseMessage.appendInt(hC); - } else { - responseMessage.appendBytes(hN); + if (i == 0) { + // Write AJP message header + responseMessage.reset(); + responseMessage.appendByte(Constants.JK_AJP13_SEND_HEADERS); + + // Write HTTP response line + responseMessage.appendInt(statusCode); + // Reason phrase is optional but mod_jk + httpd 2.x fails with a null + // reason phrase - bug 45026 + tmpMB.setString(Integer.toString(response.getStatus())); + responseMessage.appendBytes(tmpMB); + + // Start headers + responseMessage.appendInt(numHeaders); + } + + try { + // Write headers + MessageBytes hN = headers.getName(i); + int hC = Constants.getResponseAjpIndex(hN.toString()); + if (hC > 0) { + responseMessage.appendInt(hC); + } else { + responseMessage.appendBytes(hN); + } + MessageBytes hV=headers.getValue(i); + responseMessage.appendBytes(hV); + } catch (IllegalArgumentException iae) { + // Log the problematic header + log.warn(sm.getString("ajpprocessor.response.invalidHeader", headers.getName(i), + headers.getValue(i)), iae); + // Remove the problematic header + headers.removeHeader(i); + numHeaders--; + // Reset loop and start again + i = -1; } - MessageBytes hV=headers.getValue(i); - responseMessage.appendBytes(hV); } // Write to buffer diff --git a/java/org/apache/coyote/ajp/LocalStrings.properties b/java/org/apache/coyote/ajp/LocalStrings.properties index 467035da3f..623f82c4a9 100644 --- a/java/org/apache/coyote/ajp/LocalStrings.properties +++ b/java/org/apache/coyote/ajp/LocalStrings.properties @@ -26,6 +26,7 @@ ajpprocessor.header.tooLong=Header message of length [{0}] received but the pack ajpprocessor.readtimeout=Timeout attempting to read data from the socket ajpprocessor.request.prepare=Error preparing request ajpprocessor.request.process=Error processing request +ajpprocessor.response.invalidHeader=The HTTP response header [{0}] with value [{1}] has been removed from the response because it is invalid ajpprocessor.unknownAttribute=Rejecting request due to unknown request attribute [{0}] received from reverse proxy ajpprotocol.noSSL=SSL is not supported with AJP. The SSL host configuration for [{0}] was ignored diff --git a/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java b/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java index 37f9bb1d35..8fd82bd920 100644 --- a/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java +++ b/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java @@ -826,11 +826,74 @@ public class TestAbstractAjpProcessor extends TomcatBaseTest { } + /* + * https://bz.apache.org/bugzilla/show_bug.cgi?id=66512 + */ + @Test + public void testInvalidHeader() throws Exception { + + Tomcat tomcat = getTomcatInstance(); + + // No file system docBase required + Context ctx = tomcat.addContext("", null); + + Tomcat.addServlet(ctx, "bug66512", new InvalidHeaderServlet()); + ctx.addServletMappingDecoded("/", "bug66512"); + + tomcat.start(); + + SimpleAjpClient ajpClient = new SimpleAjpClient(); + ajpClient.setPort(getPort()); + ajpClient.connect(); + + validateCpong(ajpClient.cping()); + + TesterAjpMessage forwardMessage = ajpClient.createForwardMessage(); + forwardMessage.end(); + + TesterAjpMessage responseHeaderMessage = ajpClient.sendMessage(forwardMessage, null); + + // Expect 2 messages: headers, end + Map<String,List<String>> responseHeaders = validateResponseHeaders(responseHeaderMessage, 200, "200"); + Assert.assertTrue(responseHeaders.containsKey(InvalidHeaderServlet.VALID_HEADER_A_NAME)); + Assert.assertFalse(responseHeaders.containsKey(InvalidHeaderServlet.INVALID_HEADER_B_NAME)); + Assert.assertTrue(responseHeaders.containsKey(InvalidHeaderServlet.VALID_HEADER_C_NAME)); + + validateResponseEnd(ajpClient.readMessage(), true); + + // Double check the connection is still open + validateCpong(ajpClient.cping()); + + ajpClient.disconnect(); + } + + + private static class InvalidHeaderServlet extends HttpServlet { + + private static final long serialVersionUID = 1L; + + private static final String VALID_HEADER_A_NAME = "X-Bug66512-A"; + private static final String VALID_HEADER_A_VALUE = "AaAaA"; + private static final String VALID_HEADER_C_NAME = "X-Bug66512-C"; + private static final String VALID_HEADER_C_VALUE = "CcCcC"; + + private static final String INVALID_HEADER_B_NAME = "X-Bug66512-B"; + private static final String INVALID_HEADER_B_VALUE = "Bb\u039abB"; + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + resp.setHeader(VALID_HEADER_A_NAME, VALID_HEADER_A_VALUE); + resp.setHeader(INVALID_HEADER_B_NAME, INVALID_HEADER_B_VALUE); + resp.setHeader(VALID_HEADER_C_NAME, VALID_HEADER_C_VALUE); + } + } + + /** * Process response header packet and checks the status. Any other data is * ignored. */ - private void validateResponseHeaders(TesterAjpMessage message, + private Map<String,List<String>> validateResponseHeaders(TesterAjpMessage message, int expectedStatus, String expectedMessage) throws Exception { // First two bytes should always be AB Assert.assertEquals((byte) 'A', message.buf[0]); @@ -854,12 +917,14 @@ public class TestAbstractAjpProcessor extends TomcatBaseTest { // Get the number of headers int headerCount = message.readInt(); + Map<String,List<String>> headerMap = new HashMap<>(); for (int i = 0; i < headerCount; i++) { - // Read the header name - message.readHeaderName(); - // Read the header value - message.readString(); + String headerName = message.readHeaderName(); + String heaverValue = message.readString(); + headerMap.computeIfAbsent(headerName, k -> new ArrayList<>()).add(heaverValue); } + + return headerMap; } private void validateGetBody(TesterAjpMessage message) { diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 10e53df939..f2718b42d0 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -130,6 +130,10 @@ HTTP responses) when used with direct buffers. Patch suggested by Arjen Poutsma. (markt) </fix> + <fix> + <bug>66512</bug>: Align AJP handling of invalid HTTP response headers + (they are now removed from the response) with HTTP. (markt) + </fix> </changelog> </subsection> <subsection name="Other"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org