You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2023/05/03 16:10:42 UTC
[tomcat] branch 8.5.x updated: Fix 66591 - Make sure every response includes a Send Headers packet
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org