Skip to content

Commit

Permalink
Fix 66591 - Make sure every response includes a Send Headers packet
Browse files Browse the repository at this point in the history
  • Loading branch information
markt-asf committed May 3, 2023
1 parent a477bb0 commit 739c738
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 36 deletions.
74 changes: 39 additions & 35 deletions java/org/apache/coyote/ajp/AjpProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
56 changes: 55 additions & 1 deletion test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -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<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.
*/
Expand Down Expand Up @@ -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);
}

Expand Down
5 changes: 5 additions & 0 deletions webapps/docs/changelog.xml
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@
<code>allowHostHeaderMismatch</code>. These are now hard-coded to the
previous defaults. (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">
Expand Down

0 comments on commit 739c738

Please sign in to comment.