This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit be6a92b6256ed6ebc86e329e7b54ed9730b2ba4a
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 | 84 ++++++++++++----------
.../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, 95 insertions(+), 41 deletions(-)
diff --git a/java/org/apache/catalina/connector/CoyoteAdapter.java
b/java/org/apache/catalina/connector/CoyoteAdapter.java
index f9263b685d..fc1f3b54de 100644
--- a/java/org/apache/catalina/connector/CoyoteAdapter.java
+++ b/java/org/apache/catalina/connector/CoyoteAdapter.java
@@ -638,49 +638,55 @@ 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
- // Note: checkNormalize is deprecated because the test is no
- // longer required in Tomcat 10 onwards and has been
- // removed
- 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
+ // Note: checkNormalize is deprecated because the test is
no
+ // longer required in Tomcat 10 onwards and has been
+ // removed
+ 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);
+ }
}
}
@@ -836,7 +842,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 c76e757731..b6c5ca936b 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 593c05cdf7..f035d11e69 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -34,6 +34,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;
@@ -149,7 +150,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");
@@ -503,7 +504,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 df66faf462..87c0209312 100644
--- a/test/org/apache/coyote/http11/TestHttp11Processor.java
+++ b/test/org/apache/coyote/http11/TestHttp11Processor.java
@@ -1987,6 +1987,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 7d82d735bf..ff565d2628 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
