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 2214c80305 Fix 66591 - Make sure every response includes a Send Headers packet 2214c80305 is described below commit 2214c8030522aa9b2a367dfa5d9acff1a03666ae Author: Mark Thomas <ma...@apache.org> AuthorDate: Wed May 3 17:05:48 2023 +0100 Fix 66591 - Make sure every response includes a Send Headers packet --- java/org/apache/coyote/ajp/AjpProcessor.java | 102 +++++++++++---------- .../coyote/ajp/TestAbstractAjpProcessor.java | 56 ++++++++++- webapps/docs/changelog.xml | 5 + 3 files changed, 113 insertions(+), 50 deletions(-) diff --git a/java/org/apache/coyote/ajp/AjpProcessor.java b/java/org/apache/coyote/ajp/AjpProcessor.java index 103f90f700..bf2a570d76 100644 --- a/java/org/apache/coyote/ajp/AjpProcessor.java +++ b/java/org/apache/coyote/ajp/AjpProcessor.java @@ -1058,59 +1058,63 @@ public class AjpProcessor extends AbstractProcessor { responseMsgPos = -1; int numHeaders = headers.size(); - for (int i = 0; i < numHeaders; i++) { - if (i == 0) { - // Write AJP message header - responseMessage.reset(); - responseMessage.appendByte(Constants.JK_AJP13_SEND_HEADERS); - - // Write HTTP response line - responseMessage.appendInt(statusCode); - if (sendReasonPhrase) { - String message = null; - if (org.apache.coyote.Constants.USE_CUSTOM_STATUS_MSG_IN_HEADER && - HttpMessages.isSafeInHttpHeader(response.getMessage())) { - message = response.getMessage(); - } - if (message == null) { - message = HttpMessages.getInstance(response.getLocale()).getMessage(response.getStatus()); - } - if (message == null) { - // mod_jk + httpd 2.x fails with a null status message - bug 45026 - message = Integer.toString(response.getStatus()); - } - tmpMB.setString(message); - } else { - // Reason phrase is optional but mod_jk + httpd 2.x fails with a null - // reason phrase - bug 45026 - tmpMB.setString(Integer.toString(response.getStatus())); + boolean needAjpMessageHeader = true; + while (needAjpMessageHeader) { + // Write AJP message header + responseMessage.reset(); + responseMessage.appendByte(Constants.JK_AJP13_SEND_HEADERS); + + // Write HTTP response line + responseMessage.appendInt(statusCode); + if (sendReasonPhrase) { + String message = null; + if (org.apache.coyote.Constants.USE_CUSTOM_STATUS_MSG_IN_HEADER && + HttpMessages.isSafeInHttpHeader(response.getMessage())) { + message = response.getMessage(); } - responseMessage.appendBytes(tmpMB); - - // Start headers - responseMessage.appendInt(numHeaders); + if (message == null) { + message = HttpMessages.getInstance(response.getLocale()).getMessage(response.getStatus()); + } + if (message == null) { + // mod_jk + httpd 2.x fails with a null status message - bug 45026 + message = Integer.toString(response.getStatus()); + } + tmpMB.setString(message); + } else { + // 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); - try { - // Write headers - MessageBytes hN = headers.getName(i); - int hC = Constants.getResponseAjpIndex(hN.toString()); - if (hC > 0) { - responseMessage.appendInt(hC); - } else { - responseMessage.appendBytes(hN); + // Start headers + responseMessage.appendInt(numHeaders); + + needAjpMessageHeader = false; + + for (int i = 0; i < numHeaders; i++) { + 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--; + // Restart writing of AJP message + needAjpMessageHeader = true; + break; } - 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; } } diff --git a/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java b/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java index 0ab9f77618..c16d2ad978 100644 --- a/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java +++ b/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java @@ -885,6 +885,60 @@ public class TestAbstractAjpProcessor extends TomcatBaseTest { } + /* + * https://bz.apache.org/bugzilla/show_bug.cgi?id=66591 + */ + @Test + public void testNoHeaders() throws Exception { + + Tomcat tomcat = getTomcatInstance(); + + // No file system docBase required + Context ctx = tomcat.addContext("", null); + + Tomcat.addServlet(ctx, "bug66591", new NoHeadersServlet()); + ctx.addServletMappingDecoded("/", "bug66591"); + + 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 3 messages: headers, body chunk, end + Map<String, List<String>> responseHeaders = validateResponseHeaders(responseHeaderMessage, 200, "200"); + Assert.assertTrue(responseHeaders.isEmpty()); + + String body = extractResponseBody(ajpClient.readMessage()); + Assert.assertTrue(body.isEmpty()); + + validateResponseEnd(ajpClient.readMessage(), true); + + // Double check the connection is still open + validateCpong(ajpClient.cping()); + + ajpClient.disconnect(); + } + + + private static class NoHeadersServlet extends HttpServlet { + + private static final long serialVersionUID = 1L; + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + resp.flushBuffer(); + } + } + + /** * Process response header packet and checks the status. Any other data is ignored. */ @@ -950,7 +1004,7 @@ public class TestAbstractAjpProcessor extends TomcatBaseTest { Assert.assertEquals(0x03, message.readByte()); int len = message.readInt(); - Assert.assertTrue(len > 0); + Assert.assertTrue(len >= 0); return message.readString(len); } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index e0fb10ed2a..0b18883d98 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -163,6 +163,11 @@ and <code>allowHostHeaderMismatch</code> as they have been removed in Tomcat 11 onwards. (markt) </update> + <fix> + <bug>66591</bug>: Fix a regression introduced in the fix for + <bug>66512</bug> that meant that an AJP Send Headers was not sent for + responses where no HTTP headers were set. (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