Issue #10231 - ee10 DefaultServlet HTTP method support. (#10232)

* Issue #10231 - ee10 & ee9 DefaultServlet HTTP method support. + HEAD support & testing + TRACE disabled & testing + OPTIONS support & testing + POST disabled & testing
jetty · Aug 5, 2023 · 5535179 · 5535179
1 parent 9357c26
commit 5535179
Show file tree

Hide file tree

Showing 6 changed files with 229 additions and 3 deletions.
diff --git a/...-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java b/...-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java
@@ -558,6 +558,20 @@ protected void doHead(HttpServletRequest req, HttpServletResponse resp) throws S
         doGet(req, resp);
     }
 
+    @Override
+    protected void doTrace(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
+    {
+        // Always return 405: Method Not Allowed for DefaultServlet
+        resp.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
+    }
+
+    @Override
+    protected void doOptions(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
+    {
+        // override to eliminate TRACE that the default HttpServlet impl adds
+        resp.setHeader("Allow", "GET, HEAD, OPTIONS");
+    }
+
     private Resource getResource(URI uri)
     {
         String uriPath = uri.getRawPath();

diff --git a/...0/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DefaultServletTest.java b/...0/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DefaultServletTest.java
@@ -84,6 +84,7 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.anyOf;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.emptyString;
 import static org.hamcrest.Matchers.endsWith;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
@@ -173,6 +174,110 @@ public void testGet() throws Exception
         assertThat(response.toString(), response.getContent(), is("How now brown cow"));
     }
 
+    @Test
+    public void testHead() throws Exception
+    {
+        Path file = docRoot.resolve("file.txt");
+
+        context.addServlet(DefaultServlet.class, "/");
+
+        String rawResponse;
+        HttpTester.Response response;
+
+        rawResponse = connector.getResponse("""
+            HEAD /context/file.txt HTTP/1.1\r
+            Host: local\r
+            Connection: close\r
+            \r
+            """);
+        response = HttpTester.parseResponse(rawResponse);
+        assertThat(response.toString(), response.getStatus(), is(HttpStatus.NOT_FOUND_404));
+
+        Files.writeString(file, "How now brown cow", UTF_8);
+
+        rawResponse = connector.getResponse("""
+            HEAD /context/file.txt HTTP/1.1\r
+            Host: local\r
+            Connection: close\r
+            \r
+            """);
+        response = HttpTester.parseResponse(rawResponse);
+        assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200));
+        assertThat(response.toString(), response.getContent(), emptyString());
+    }
+
+    @Test
+    public void testPost() throws Exception
+    {
+        Path file = docRoot.resolve("file.txt");
+
+        context.addServlet(DefaultServlet.class, "/");
+
+        String rawResponse;
+        HttpTester.Response response;
+
+        rawResponse = connector.getResponse("""
+            POST /context/file.txt HTTP/1.1\r
+            Host: local\r
+            Connection: close\r
+            Content-Length: 5\r
+            \r
+            abcde
+            """);
+        response = HttpTester.parseResponse(rawResponse);
+        assertThat(response.toString(), response.getStatus(), is(HttpStatus.METHOD_NOT_ALLOWED_405));
+
+        Files.writeString(file, "How now brown cow", UTF_8);
+
+        rawResponse = connector.getResponse("""
+            POST /context/file.txt HTTP/1.1\r
+            Host: local\r
+            Connection: close\r
+            Content-Length: 5\r
+            \r
+            abcde
+            """);
+        response = HttpTester.parseResponse(rawResponse);
+        assertThat(response.toString(), response.getStatus(), is(HttpStatus.METHOD_NOT_ALLOWED_405));
+    }
+
+    @Test
+    public void testTrace() throws Exception
+    {
+        context.addServlet(DefaultServlet.class, "/");
+
+        String rawResponse;
+        HttpTester.Response response;
+
+        rawResponse = connector.getResponse("""
+            TRACE /context/file.txt HTTP/1.1\r
+            Host: local\r
+            Connection: close\r
+            \r
+            """);
+        response = HttpTester.parseResponse(rawResponse);
+        assertThat(response.toString(), response.getStatus(), is(HttpStatus.METHOD_NOT_ALLOWED_405));
+    }
+
+    @Test
+    public void testOptions() throws Exception
+    {
+        context.addServlet(DefaultServlet.class, "/");
+
+        String rawResponse;
+        HttpTester.Response response;
+
+        rawResponse = connector.getResponse("""
+            OPTIONS /context/ HTTP/1.1\r
+            Host: local\r
+            Connection: close\r
+            \r
+            """);
+        response = HttpTester.parseResponse(rawResponse);
+        assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200));
+        assertThat(response.get(HttpHeader.ALLOW), is("GET, HEAD, OPTIONS"));
+    }
+
     @Test
     public void testGetBinaryWithUtfResponseEncoding() throws Exception
     {

diff --git a/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/DefaultServlet.java b/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/DefaultServlet.java
@@ -510,7 +510,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response)
     protected void doPost(HttpServletRequest request, HttpServletResponse response)
         throws ServletException, IOException
     {
-        doGet(request, response);
+        response.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
     }
 
     @Override
@@ -531,7 +531,7 @@ protected void doTrace(HttpServletRequest request, HttpServletResponse response)
     protected void doOptions(HttpServletRequest request, HttpServletResponse response)
         throws ServletException, IOException
     {
-        response.setHeader("Allow", "GET,HEAD,POST,OPTIONS");
+        response.setHeader("Allow", "GET, HEAD, OPTIONS");
     }
 
     @Override

diff --git a/...ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java b/...ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java
@@ -77,6 +77,7 @@
 import static org.hamcrest.Matchers.anyOf;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.emptyString;
 import static org.hamcrest.Matchers.endsWith;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
@@ -2162,6 +2163,110 @@ public void testGetUtf8NfdFile() throws Exception
         }
     }
 
+    @Test
+    public void testHead() throws Exception
+    {
+        Path file = docRoot.resolve("file.txt");
+
+        context.addServlet(DefaultServlet.class, "/");
+
+        String rawResponse;
+        HttpTester.Response response;
+
+        rawResponse = connector.getResponse("""
+            HEAD /context/file.txt HTTP/1.1\r
+            Host: local\r
+            Connection: close\r
+            \r
+            """);
+        response = HttpTester.parseResponse(rawResponse);
+        assertThat(response.toString(), response.getStatus(), is(HttpStatus.NOT_FOUND_404));
+
+        Files.writeString(file, "How now brown cow", UTF_8);
+
+        rawResponse = connector.getResponse("""
+            HEAD /context/file.txt HTTP/1.1\r
+            Host: local\r
+            Connection: close\r
+            \r
+            """);
+        response = HttpTester.parseResponse(rawResponse);
+        assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200));
+        assertThat(response.toString(), response.getContent(), emptyString());
+    }
+
+    @Test
+    public void testPost() throws Exception
+    {
+        Path file = docRoot.resolve("file.txt");
+
+        context.addServlet(DefaultServlet.class, "/");
+
+        String rawResponse;
+        HttpTester.Response response;
+
+        rawResponse = connector.getResponse("""
+            POST /context/file.txt HTTP/1.1\r
+            Host: local\r
+            Connection: close\r
+            Content-Length: 5\r
+            \r
+            abcde
+            """);
+        response = HttpTester.parseResponse(rawResponse);
+        assertThat(response.toString(), response.getStatus(), is(HttpStatus.METHOD_NOT_ALLOWED_405));
+
+        Files.writeString(file, "How now brown cow", UTF_8);
+
+        rawResponse = connector.getResponse("""
+            POST /context/file.txt HTTP/1.1\r
+            Host: local\r
+            Connection: close\r
+            Content-Length: 5\r
+            \r
+            abcde
+            """);
+        response = HttpTester.parseResponse(rawResponse);
+        assertThat(response.toString(), response.getStatus(), is(HttpStatus.METHOD_NOT_ALLOWED_405));
+    }
+
+    @Test
+    public void testTrace() throws Exception
+    {
+        context.addServlet(DefaultServlet.class, "/");
+
+        String rawResponse;
+        HttpTester.Response response;
+
+        rawResponse = connector.getResponse("""
+            TRACE /context/file.txt HTTP/1.1\r
+            Host: local\r
+            Connection: close\r
+            \r
+            """);
+        response = HttpTester.parseResponse(rawResponse);
+        assertThat(response.toString(), response.getStatus(), is(HttpStatus.METHOD_NOT_ALLOWED_405));
+    }
+
+    @Test
+    public void testOptions() throws Exception
+    {
+        context.addServlet(DefaultServlet.class, "/");
+
+        String rawResponse;
+        HttpTester.Response response;
+
+        rawResponse = connector.getResponse("""
+            OPTIONS /context/ HTTP/1.1\r
+            Host: local\r
+            Connection: close\r
+            \r
+            """);
+        response = HttpTester.parseResponse(rawResponse);
+        assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200));
+        assertThat(response.get(HttpHeader.ALLOW), is("GET, HEAD, OPTIONS"));
+    }
+
     public static class OutputFilter implements Filter
     {
         @Override

diff --git a/...ee9/jetty-ee9-servlets/src/test/java/org/eclipse/jetty/ee9/servlets/AbstractGzipTest.java b/...ee9/jetty-ee9-servlets/src/test/java/org/eclipse/jetty/ee9/servlets/AbstractGzipTest.java
@@ -45,7 +45,7 @@ public abstract class AbstractGzipTest
 {
     protected static final int DEFAULT_OUTPUT_BUFFER_SIZE = new HttpConfiguration().getOutputBufferSize();
 
-    protected WorkDir workDir;
+    public WorkDir workDir;
 
     protected FilterInputStream newContentEncodingFilterInputStream(String contentEncoding, InputStream inputStream) throws IOException
     {

diff --git a/...tty-ee9-servlets/src/test/java/org/eclipse/jetty/ee9/servlets/GzipDefaultServletTest.java b/...tty-ee9-servlets/src/test/java/org/eclipse/jetty/ee9/servlets/GzipDefaultServletTest.java
@@ -85,6 +85,7 @@ public void testIsGzipByMethod(String method) throws Exception
         ServletContextHandler servletContextHandler = new ServletContextHandler();
         servletContextHandler.setContextPath("/context");
         servletContextHandler.setBaseResource(ResourceFactory.of(server).newResource(contextDir));
+        // The WibbleDefaultServlet overrides the method behaviors
         ServletHolder holder = new ServletHolder("default", WibbleDefaultServlet.class);
         holder.setInitParameter("etags", "true");
         servletContextHandler.addServlet(holder, "/");
@@ -141,6 +142,7 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) throws
         {
             switch (req.getMethod())
             {
+                case "POST":
                 case "WIBBLE":
                     // Disregard the method given, use GET instead.
                     doGet(req, resp);