Return status code 405 for unknown HTTP methods

JDisc previously returned 501 for unknown HTTP methods as mandated by the HTTP / Servlet specification. This caused a lot of noise in our 5xx response metrics for JDisc instances directly exposed to the internet (external actors performing vulnerability testing). This change will cause unknown methods to be handled identically to unsupported methods.

2 files changed, 32 insertions, 6 deletions

diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java
index 5cbe7320f0e..a6b2deb4681 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java
@@ -4,6 +4,7 @@ package com.yahoo.jdisc.http.server.jetty;
 import com.yahoo.container.logging.AccessLogEntry;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.jdisc.handler.OverloadException;
+import com.yahoo.jdisc.http.HttpRequest.Method;
 
 import javax.servlet.ServletException;
 import javax.servlet.annotation.WebServlet;
@@ -11,12 +12,12 @@ import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
-import java.util.Arrays;
 import java.util.Enumeration;
-import java.util.HashSet;
 import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static com.yahoo.jdisc.http.core.HttpServletRequestUtils.getConnection;
 
@@ -78,8 +79,6 @@ class JDiscHttpServlet extends HttpServlet {
         dispatchHttpRequest(request, response);
     }
 
-    private static final Set<String> JETTY_UNSUPPORTED_METHODS = new HashSet<>(Arrays.asList("PATCH"));
-
     /**
      * Override to set connector attribute before the request becomes an upgrade request in the web socket case.
      * (After the upgrade, the HttpConnection is no longer available.)
@@ -93,10 +92,20 @@ class JDiscHttpServlet extends HttpServlet {
         context.metric.add(JettyHttpServer.Metrics.NUM_REQUESTS, 1, metricContext);
         context.metric.add(JettyHttpServer.Metrics.JDISC_HTTP_REQUESTS, 1, metricContext);
 
-        if (JETTY_UNSUPPORTED_METHODS.contains(request.getMethod().toUpperCase())) {
+
+        Set<String> servletSupportedMethods =
+                Stream.of(Method.OPTIONS, Method.GET, Method.HEAD, Method.POST, Method.PUT, Method.DELETE, Method.TRACE)
+                        .map(Method::name)
+                        .collect(Collectors.toSet());
+        String method = request.getMethod().toUpperCase();
+        if (servletSupportedMethods.contains(method)) {
+            super.service(request, response);
+        } else if (method.equals(Method.PATCH.name())) {
+            // PATCH method is not handled by the Servlet spec
             dispatchHttpRequest(request, response);
         } else {
-            super.service(request, response);
+            // Divergence from HTTP / Servlet spec: JDisc returns 405 for both unknown and known (but unsupported) methods.
+            response.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
         }
     }
 
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServletTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServletTest.java
index 7a645337967..230f59cbb34 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServletTest.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServletTest.java
@@ -14,11 +14,14 @@ import org.apache.http.client.methods.HttpOptions;
 import org.apache.http.client.methods.HttpPatch;
 import org.apache.http.client.methods.HttpPost;
 import org.apache.http.client.methods.HttpPut;
+import org.apache.http.client.methods.HttpRequestBase;
 import org.apache.http.client.methods.HttpTrace;
 import org.junit.Test;
 
+import java.io.IOException;
 import java.net.URI;
 
+import static com.yahoo.jdisc.Response.Status.METHOD_NOT_ALLOWED;
 import static com.yahoo.jdisc.Response.Status.OK;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -51,6 +54,15 @@ public class JDiscHttpServletTest {
         assertThat(driver.close(), is(true));
     }
 
+    @Test
+    public void requireThatServerResponds405ToUnknownMethods() throws IOException {
+        TestDriver driver = TestDrivers.newInstance(newEchoHandler());
+        final URI uri = driver.client().newUri("/status.html");
+        driver.client().execute(new UnknownMethodHttpRequest(uri))
+                .expectStatusCode(is(METHOD_NOT_ALLOWED));
+        assertThat(driver.close(), is(true));
+    }
+
     private static RequestHandler newEchoHandler() {
         return new AbstractRequestHandler() {
 
@@ -60,4 +72,9 @@ public class JDiscHttpServletTest {
             }
         };
     }
+
+    private static class UnknownMethodHttpRequest extends HttpRequestBase {
+        UnknownMethodHttpRequest(URI uri) { setURI(uri); }
+        @Override public String getMethod() { return "UNKNOWN_METHOD"; }
+    }
 }