diff --git a/java/org/apache/coyote/ajp/AjpProcessor.java b/java/org/apache/coyote/ajp/AjpProcessor.java index 93d671cb7fc8..ccf2e78ba1d5 100644 --- a/java/org/apache/coyote/ajp/AjpProcessor.java +++ b/java/org/apache/coyote/ajp/AjpProcessor.java @@ -956,43 +956,47 @@ protected final void prepareResponse() throws IOException { 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); - // 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); - } + boolean needAjpMessageHeader = true; + while (needAjpMessageHeader) { + // Write AJP message header + responseMessage.reset(); + responseMessage.appendByte(Constants.JK_AJP13_SEND_HEADERS); - try { - // Write headers - MessageBytes hN = headers.getName(i); - int hC = Constants.getResponseAjpIndex(hN.toString()); - if (hC > 0) { - responseMessage.appendInt(hC); - } else { - responseMessage.appendBytes(hN); + // 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); + + 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 111b033e851d..22b0466e6fb3 100644 --- a/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java +++ b/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java @@ -883,6 +883,60 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se } + /* + * 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> 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. */ @@ -943,7 +997,7 @@ private String extractResponseBody(TesterAjpMessage message) throws Exception { 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 0de19ecbcd7c..89d263c6cf54 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -152,6 +152,11 @@ allowHostHeaderMismatch. These are now hard-coded to the previous defaults. (markt) + + 66591: Fix a regression introduced in the fix for + 66512 that meant that an AJP Send Headers was not sent for + responses where no HTTP headers were set. (markt) +