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 cc53fc0 Return correct response code for HTTP/2 when method is not
implemented
cc53fc0 is described below
commit cc53fc08df882a9294549b35d1f9fd005c96eabe
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Jun 16 14:26:49 2020 +0100
Return correct response code for HTTP/2 when method is not implemented
---
java/javax/servlet/http/HttpServlet.java | 36 +++-----
test/javax/servlet/http/TestHttpServlet.java | 102 +++++++++++++++++++++
.../coyote/http11/TestHttp11InputBufferCRLF.java | 1 -
test/org/apache/coyote/http2/TestHttpServlet.java | 59 ++++++++++++
webapps/docs/changelog.xml | 6 ++
5 files changed, 181 insertions(+), 23 deletions(-)
diff --git a/java/javax/servlet/http/HttpServlet.java
b/java/javax/servlet/http/HttpServlet.java
index aedbee6..57617e7 100644
--- a/java/javax/servlet/http/HttpServlet.java
+++ b/java/javax/servlet/http/HttpServlet.java
@@ -168,13 +168,8 @@ public abstract class HttpServlet extends GenericServlet {
protected void doGet(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException
{
- String protocol = req.getProtocol();
String msg = lStrings.getString("http.method_get_not_supported");
- if (protocol.endsWith("1.1")) {
- resp.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED, msg);
- } else {
- resp.sendError(HttpServletResponse.SC_BAD_REQUEST, msg);
- }
+ sendMethodNotAllowed(req, resp, msg);
}
@@ -308,13 +303,8 @@ public abstract class HttpServlet extends GenericServlet {
protected void doPost(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {
- String protocol = req.getProtocol();
String msg = lStrings.getString("http.method_post_not_supported");
- if (protocol.endsWith("1.1")) {
- resp.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED, msg);
- } else {
- resp.sendError(HttpServletResponse.SC_BAD_REQUEST, msg);
- }
+ sendMethodNotAllowed(req, resp, msg);
}
@@ -363,13 +353,8 @@ public abstract class HttpServlet extends GenericServlet {
protected void doPut(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {
- String protocol = req.getProtocol();
String msg = lStrings.getString("http.method_put_not_supported");
- if (protocol.endsWith("1.1")) {
- resp.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED, msg);
- } else {
- resp.sendError(HttpServletResponse.SC_BAD_REQUEST, msg);
- }
+ sendMethodNotAllowed(req, resp, msg);
}
@@ -411,12 +396,19 @@ public abstract class HttpServlet extends GenericServlet {
HttpServletResponse resp)
throws ServletException, IOException {
- String protocol = req.getProtocol();
String msg = lStrings.getString("http.method_delete_not_supported");
- if (protocol.endsWith("1.1")) {
- resp.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED, msg);
- } else {
+ sendMethodNotAllowed(req, resp, msg);
+ }
+
+
+ private void sendMethodNotAllowed(HttpServletRequest req,
HttpServletResponse resp, String msg) throws IOException {
+ String protocol = req.getProtocol();
+ // Note: Tomcat reports "" for HTTP/0.9 although some implementations
+ // may report HTTP/0.9
+ if (protocol.length() == 0 || protocol.endsWith("0.9") ||
protocol.endsWith("1.0")) {
resp.sendError(HttpServletResponse.SC_BAD_REQUEST, msg);
+ } else {
+ resp.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED, msg);
}
}
diff --git a/test/javax/servlet/http/TestHttpServlet.java
b/test/javax/servlet/http/TestHttpServlet.java
index 28be42d..00e4036 100644
--- a/test/javax/servlet/http/TestHttpServlet.java
+++ b/test/javax/servlet/http/TestHttpServlet.java
@@ -28,7 +28,10 @@ import javax.servlet.ServletException;
import org.junit.Assert;
import org.junit.Test;
+import org.apache.catalina.Context;
import org.apache.catalina.core.StandardContext;
+import org.apache.catalina.startup.SimpleHttpClient;
+import org.apache.catalina.startup.TesterServlet;
import org.apache.catalina.startup.Tomcat;
import org.apache.catalina.startup.TomcatBaseTest;
import org.apache.tomcat.util.buf.ByteChunk;
@@ -191,6 +194,105 @@ public class TestHttpServlet extends TomcatBaseTest {
}
+ @Test
+ public void testUnimplementedMethodHttp09() throws Exception {
+ doTestUnimplementedMethod("0.9");
+ }
+
+
+ @Test
+ public void testUnimplementedMethodHttp10() throws Exception {
+ doTestUnimplementedMethod("1.0");
+ }
+
+
+ @Test
+ public void testUnimplementedMethodHttp11() throws Exception {
+ doTestUnimplementedMethod("1.1");
+ }
+
+
+ /*
+ * See org.aoache.coyote.http2.TestHttpServlet for the HTTP/2 version of
+ * this test. It was placed in that package because it needed access to
+ * package private classes.
+ */
+
+
+ private void doTestUnimplementedMethod(String httpVersion) {
+ StringBuilder request = new StringBuilder("PUT /test");
+ boolean isHttp09 = "0.9".equals(httpVersion);
+ boolean isHttp10 = "1.0".equals(httpVersion);
+
+ if (!isHttp09) {
+ request.append(" HTTP/");
+ request.append(httpVersion);
+ }
+ request.append(SimpleHttpClient.CRLF);
+
+ request.append("Host: localhost:8080");
+ request.append(SimpleHttpClient.CRLF);
+
+ request.append("Connection: close");
+ request.append(SimpleHttpClient.CRLF);
+
+ request.append(SimpleHttpClient.CRLF);
+
+ Client client = new Client(request.toString(),
"0.9".equals(httpVersion));
+
+ client.doRequest();
+
+ if (isHttp09) {
+ Assert.assertTrue( client.getResponseBody(),
client.getResponseBody().contains(" 400 "));
+ } else if (isHttp10) {
+ Assert.assertTrue(client.getResponseLine(),
client.isResponse400());
+ } else {
+ Assert.assertTrue(client.getResponseLine(),
client.isResponse405());
+ }
+ }
+
+
+ private class Client extends SimpleHttpClient {
+
+ public Client(String request, boolean isHttp09) {
+ setRequest(new String[] {request});
+ setUseHttp09(isHttp09);
+ }
+
+ private Exception doRequest() {
+
+ Tomcat tomcat = getTomcatInstance();
+
+ Context root = tomcat.addContext("", TEMP_DIR);
+ Tomcat.addServlet(root, "TesterServlet", new TesterServlet());
+ root.addServletMappingDecoded("/test", "TesterServlet");
+
+ try {
+ tomcat.start();
+ setPort(tomcat.getConnector().getLocalPort());
+ setRequestPause(20);
+
+ // Open connection
+ connect();
+
+ processRequest(); // blocks until response has been read
+
+ // Close the connection
+ disconnect();
+ } catch (Exception e) {
+ e.printStackTrace();
+ return e;
+ }
+ return null;
+ }
+
+ @Override
+ public boolean isResponseBodyOK() {
+ return false;
+ }
+ }
+
+
private static class Bug57602ServletOuter extends HttpServlet {
private static final long serialVersionUID = 1L;
diff --git a/test/org/apache/coyote/http11/TestHttp11InputBufferCRLF.java
b/test/org/apache/coyote/http11/TestHttp11InputBufferCRLF.java
index ee033a1..786a95a 100644
--- a/test/org/apache/coyote/http11/TestHttp11InputBufferCRLF.java
+++ b/test/org/apache/coyote/http11/TestHttp11InputBufferCRLF.java
@@ -186,7 +186,6 @@ public class TestHttp11InputBufferCRLF extends
TomcatBaseTest {
// Open connection
connect();
- setRequest(request);
processRequest(); // blocks until response has been read
// Close the connection
diff --git a/test/org/apache/coyote/http2/TestHttpServlet.java
b/test/org/apache/coyote/http2/TestHttpServlet.java
new file mode 100644
index 0000000..99209b5
--- /dev/null
+++ b/test/org/apache/coyote/http2/TestHttpServlet.java
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.coyote.http2;
+
+import java.nio.ByteBuffer;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+/*
+ * Implement here rather than jakarta.servlet.http.TestHttpServley because it
+ * needs access to package private classes.
+ */
+public class TestHttpServlet extends Http2TestBase {
+
+ @Test
+ public void testUnimplementedMethodHttp2() throws Exception {
+ http2Connect();
+
+ // Build a POST request for a Servlet that does not implement POST
+ byte[] headersFrameHeader = new byte[9];
+ ByteBuffer headersPayload = ByteBuffer.allocate(128);
+ byte[] dataFrameHeader = new byte[9];
+ ByteBuffer dataPayload = ByteBuffer.allocate(0);
+
+ buildPostRequest(headersFrameHeader, headersPayload, false, null, -1,
"/empty", dataFrameHeader, dataPayload,
+ null, null, null, 3);
+
+ // Write the headers
+ writeFrame(headersFrameHeader, headersPayload);
+ // Body
+ writeFrame(dataFrameHeader, dataPayload);
+
+ parser.readFrame(true);
+ parser.readFrame(true);
+
+ String trace = output.getTrace();
+ String[] lines = trace.split("\n");
+
+ // Check the response code
+ Assert.assertEquals("3-HeadersStart", lines[0]);
+ Assert.assertEquals("3-Header-[:status]-[405]", lines[1]);
+ }
+
+}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index da9b0bc..961b90d 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -66,6 +66,12 @@
Add <code>application/wasm</code> to the media types recognised by
Tomcat. Based on a PR by Thiago Henrique Hüpner. (markt)
</add>
+ <fix>
+ Fix a bug in <code>HttpServlet</code> so that a <code>405</code>
+ response is returned for an HTTP/2 request if the mapped servlet does
+ implement the requested method rather than the more general
+ <code>400</code> response. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
