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