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
commit 242196a8af1c7ab8a865c56e4852c7b5d648cd17
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Feb 23 16:37:30 2023 +0000
Handle CONNECT with deliberate 501 rather than accidental 400
---
.../apache/catalina/connector/CoyoteAdapter.java | 78 ++++++++++++----------
.../catalina/connector/LocalStrings.properties | 1 +
java/org/apache/coyote/http2/Stream.java | 6 +-
.../apache/coyote/http11/TestHttp11Processor.java | 18 +++++
.../apache/coyote/http2/TestStreamProcessor.java | 23 +++++++
webapps/docs/changelog.xml | 4 ++
6 files changed, 92 insertions(+), 38 deletions(-)
diff --git a/java/org/apache/catalina/connector/CoyoteAdapter.java
b/java/org/apache/catalina/connector/CoyoteAdapter.java
index ed5d5fc31d..104a83f3e4 100644
--- a/java/org/apache/catalina/connector/CoyoteAdapter.java
+++ b/java/org/apache/catalina/connector/CoyoteAdapter.java
@@ -645,46 +645,52 @@ public class CoyoteAdapter implements Adapter {
MessageBytes decodedURI = req.decodedURI();
- if (undecodedURI.getType() == MessageBytes.T_BYTES) {
- // Copy the raw URI to the decodedURI
- decodedURI.duplicate(undecodedURI);
+ // Filter CONNECT method
+ if (req.method().equalsIgnoreCase("CONNECT")) {
+ response.sendError(HttpServletResponse.SC_NOT_IMPLEMENTED,
sm.getString("coyoteAdapter.connect"));
+ } else {
+ // No URI for CONNECT requests
+ if (undecodedURI.getType() == MessageBytes.T_BYTES) {
+ // Copy the raw URI to the decodedURI
+ decodedURI.duplicate(undecodedURI);
- // Parse (and strip out) the path parameters
- parsePathParameters(req, request);
+ // Parse (and strip out) the path parameters
+ parsePathParameters(req, request);
- // URI decoding
- // %xx decoding of the URL
- try {
- req.getURLDecoder().convert(decodedURI.getByteChunk(),
connector.getEncodedSolidusHandlingInternal());
- } catch (IOException ioe) {
- response.sendError(400, "Invalid URI: " + ioe.getMessage());
- }
- // Normalization
- if (normalize(req.decodedURI())) {
- // Character decoding
- convertURI(decodedURI, request);
- // Check that the URI is still normalized
- if (!checkNormalize(req.decodedURI())) {
+ // URI decoding
+ // %xx decoding of the URL
+ try {
+ req.getURLDecoder().convert(decodedURI.getByteChunk(),
connector.getEncodedSolidusHandlingInternal());
+ } catch (IOException ioe) {
+ response.sendError(400, "Invalid URI: " +
ioe.getMessage());
+ }
+ // Normalization
+ if (normalize(req.decodedURI())) {
+ // Character decoding
+ convertURI(decodedURI, request);
+ // Check that the URI is still normalized
+ if (!checkNormalize(req.decodedURI())) {
+ response.sendError(400, "Invalid URI");
+ }
+ } else {
response.sendError(400, "Invalid URI");
}
} else {
- response.sendError(400, "Invalid URI");
- }
- } else {
- /* The URI is chars or String, and has been sent using an in-memory
- * protocol handler. The following assumptions are made:
- * - req.requestURI() has been set to the 'original' non-decoded,
- * non-normalized URI
- * - req.decodedURI() has been set to the decoded, normalized form
- * of req.requestURI()
- */
- decodedURI.toChars();
- // Remove all path parameters; any needed path parameter should be
set
- // using the request object rather than passing it in the URL
- CharChunk uriCC = decodedURI.getCharChunk();
- int semicolon = uriCC.indexOf(';');
- if (semicolon > 0) {
- decodedURI.setChars(uriCC.getBuffer(), uriCC.getStart(),
semicolon);
+ /* The URI is chars or String, and has been sent using an
in-memory
+ * protocol handler. The following assumptions are made:
+ * - req.requestURI() has been set to the 'original'
non-decoded,
+ * non-normalized URI
+ * - req.decodedURI() has been set to the decoded, normalized
form
+ * of req.requestURI()
+ */
+ decodedURI.toChars();
+ // Remove all path parameters; any needed path parameter
should be set
+ // using the request object rather than passing it in the URL
+ CharChunk uriCC = decodedURI.getCharChunk();
+ int semicolon = uriCC.indexOf(';');
+ if (semicolon > 0) {
+ decodedURI.setChars(uriCC.getBuffer(), uriCC.getStart(),
semicolon);
+ }
}
}
@@ -840,7 +846,7 @@ public class CoyoteAdapter implements Adapter {
return false;
}
- // Filter trace method
+ // Filter TRACE method
if (!connector.getAllowTrace()
&& req.method().equalsIgnoreCase("TRACE")) {
Wrapper wrapper = request.getWrapper();
diff --git a/java/org/apache/catalina/connector/LocalStrings.properties
b/java/org/apache/catalina/connector/LocalStrings.properties
index 278aed06a1..7d09d69279 100644
--- a/java/org/apache/catalina/connector/LocalStrings.properties
+++ b/java/org/apache/catalina/connector/LocalStrings.properties
@@ -19,6 +19,7 @@ coyoteAdapter.authenticate=Authenticated user [{0}] provided
by connector
coyoteAdapter.authorize=Authorizing user [{0}] using Tomcat''s Realm
coyoteAdapter.checkRecycled.request=Encountered a non-recycled request and
recycled it forcedly.
coyoteAdapter.checkRecycled.response=Encountered a non-recycled response and
recycled it forcedly.
+coyoteAdapter.connect=HTTP requests using the CONNECT method are not supported
coyoteAdapter.debug=The variable [{0}] has value [{1}]
coyoteAdapter.nullRequest=An asynchronous dispatch may only happen on an
existing request
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index 5623a94838..178abe7b76 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -31,6 +31,7 @@ import org.apache.coyote.CloseNowException;
import org.apache.coyote.InputBuffer;
import org.apache.coyote.Request;
import org.apache.coyote.Response;
+import org.apache.coyote.http11.AbstractHttp11Protocol;
import org.apache.coyote.http11.HttpOutputBuffer;
import org.apache.coyote.http11.OutputFilter;
import org.apache.coyote.http11.filters.SavedRequestInputFilter;
@@ -147,7 +148,7 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
private void prepareRequest() {
if (coyoteRequest.scheme().isNull()) {
- if (handler.getProtocol().getHttp11Protocol().isSSLEnabled()) {
+ if (((AbstractHttp11Protocol<?>)
handler.getProtocol().getHttp11Protocol()).isSSLEnabled()) {
coyoteRequest.scheme().setString("https");
} else {
coyoteRequest.scheme().setString("http");
@@ -496,7 +497,8 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
final boolean receivedEndOfHeaders() throws ConnectionException {
- if (coyoteRequest.method().isNull() || coyoteRequest.scheme().isNull()
|| coyoteRequest.requestURI().isNull()) {
+ if (coyoteRequest.method().isNull() || coyoteRequest.scheme().isNull()
||
+ !coyoteRequest.method().equalsIgnoreCase("CONNECT") &&
coyoteRequest.requestURI().isNull()) {
throw new
ConnectionException(sm.getString("stream.header.required", getConnectionId(),
getIdAsString()),
Http2Error.PROTOCOL_ERROR);
}
diff --git a/test/org/apache/coyote/http11/TestHttp11Processor.java
b/test/org/apache/coyote/http11/TestHttp11Processor.java
index 18da26c2a6..397343a4e9 100644
--- a/test/org/apache/coyote/http11/TestHttp11Processor.java
+++ b/test/org/apache/coyote/http11/TestHttp11Processor.java
@@ -2003,6 +2003,24 @@ public class TestHttp11Processor extends TomcatBaseTest {
}
+ @Test
+ public void testConnect() throws Exception {
+ getTomcatInstanceTestWebapp(false, true);
+
+ String request =
+ "CONNECT example.local HTTP/1.1" + SimpleHttpClient.CRLF +
+ "Host: example.local" + SimpleHttpClient.CRLF +
+ SimpleHttpClient.CRLF;
+
+ Client client = new Client(getPort());
+ client.setRequest(new String[] {request});
+
+ client.connect();
+ client.processRequest();
+ Assert.assertTrue(client.isResponse501());
+ }
+
+
private static class TestPostNoReadServlet extends HttpServlet {
private static final long serialVersionUID = 1L;
diff --git a/test/org/apache/coyote/http2/TestStreamProcessor.java
b/test/org/apache/coyote/http2/TestStreamProcessor.java
index ff3f7d07cf..c84866399f 100644
--- a/test/org/apache/coyote/http2/TestStreamProcessor.java
+++ b/test/org/apache/coyote/http2/TestStreamProcessor.java
@@ -580,4 +580,27 @@ public class TestStreamProcessor extends Http2TestBase {
}
}
}
+
+
+ @Test
+ public void testConnect() throws Exception {
+ http2Connect();
+
+ List<Header> headers = new ArrayList<>(4);
+ headers.add(new Header(":method", "CONNECT"));
+ headers.add(new Header(":scheme", "http"));
+ headers.add(new Header(":authority", "example.local"));
+
+ byte[] headersFrameHeader = new byte[9];
+ ByteBuffer headersPayload = ByteBuffer.allocate(128);
+
+ buildGetRequest(headersFrameHeader, headersPayload, null, headers , 3);
+
+ writeFrame(headersFrameHeader, headersPayload);
+
+ parser.readFrame();
+
+ String trace = output.getTrace();
+ Assert.assertTrue(trace, trace.contains("3-Header-[:status]-[501]"));
+ }
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 0b5fc19137..da9612398f 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -112,6 +112,10 @@
types from mod_rewrite. Based on a pull request <pr>591</pr>
provided by Dimitrios Soumis. (markt)
</update>
+ <update>
+ Provide a more appropriate response (501 rather than 400) when
rejecting
+ an HTTP request using the CONNECT method. (markt)
+ </update>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
