This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 7c122e0f2c Fix BZ 66512 - Ignore invalid HHTP response headers for AJP
7c122e0f2c is described below
commit 7c122e0f2c6f80cd5a1812afda5b0d5f751636aa
Author: Mark Thomas <[email protected]>
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 41dd7af795..023b7e00e4 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 dc44e6a75f..0bcbfc81d9 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -140,6 +140,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: [email protected]
For additional commands, e-mail: [email protected]
