Fix Jetty HTTP `TRACE` issue, disable HTTP methods except `POST`. (#7052)

CHANGES.md

@@ -58,6 +58,7 @@ Release Notes.
 * Make the number of core worker in meter converter thread pool configurable.
 * Add HTTP implementation of logs reporting protocol.
 * Make metrics exporter still work even when storage layer failed.
+* Fix Jetty HTTP `TRACE` issue, disable HTTP methods except `POST`.
 
 #### UI
 * Add logo for kong plugin.

oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/remote/JettyServerTest.java

+/*
+ * 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.skywalking.oap.server.core.remote;
+
+import java.io.IOException;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpDelete;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpHead;
+import org.apache.http.client.methods.HttpOptions;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.client.methods.HttpPut;
+import org.apache.http.client.methods.HttpTrace;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.skywalking.oap.server.library.server.jetty.JettyHandler;
+import org.apache.skywalking.oap.server.library.server.jetty.JettyServer;
+import org.apache.skywalking.oap.server.library.server.jetty.JettyServerConfig;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.powermock.reflect.Whitebox;
+
+public class JettyServerTest {
+    static JettyServer SERVER;
+
+    @BeforeClass
+    public static void beforeTest() throws Exception {
+        JettyServerConfig config = JettyServerConfig.builder()
+                                                    .host("0.0.0.0")
+                                                    .port(12800)
+                                                    .contextPath("/")
+                                                    .jettyIdleTimeOut(5000)
+                                                    .build();
+
+        SERVER = new JettyServer(config);
+        SERVER.initialize();
+        SERVER.addHandler(new TestPostHandler());
+        SERVER.start();
+
+    }
+
+    @AfterClass
+    public static void afterTest() throws Exception {
+        ((org.eclipse.jetty.server.Server) Whitebox.getInternalState(SERVER, "server")).stop();
+    }
+
+    @Test
+    public void test() {
+
+        String rootURI = "http://localhost:12800";
+        String testHandlerURI = "http://localhost:12800/test";
+        String testNoHandlerURI = "http://localhost:12800/test/noHandler";
+        CloseableHttpClient httpClient = HttpClientBuilder.create().build();
+        HttpGet httpGet = new HttpGet(rootURI);
+        HttpPost httpPost = new HttpPost(rootURI);
+        HttpPost httpPostTestHandler = new HttpPost(testHandlerURI);
+        HttpPost httpPostTestNoHandler = new HttpPost(testNoHandlerURI);
+        HttpTrace httpTrace = new HttpTrace(testHandlerURI);
+        HttpTrace httpTraceRoot = new HttpTrace(rootURI);
+        HttpPut httpPut = new HttpPut(testHandlerURI);
+        HttpDelete httpDelete = new HttpDelete(testHandlerURI);
+        HttpOptions httpOptions = new HttpOptions(testHandlerURI);
+        HttpHead httpHead = new HttpHead(testHandlerURI);
+        CloseableHttpResponse response = null;
+        try {
+            //get
+            response = httpClient.execute(httpGet);
+            Assert.assertEquals(response.getStatusLine().getStatusCode(), 405);
+            response.close();
+
+            //post root
+            response = httpClient.execute(httpPost);
+            Assert.assertEquals(response.getStatusLine().getStatusCode(), 404);
+            response.close();
+
+            //post
+            response = httpClient.execute(httpPostTestHandler);
+            Assert.assertEquals(response.getStatusLine().getStatusCode(), 200);
+            response.close();
+
+            //post no handler
+            response = httpClient.execute(httpPostTestNoHandler);
+            Assert.assertEquals(response.getStatusLine().getStatusCode(), 404);
+            response.close();
+
+            //trace
+            response = httpClient.execute(httpTrace);
+            Assert.assertEquals(response.getStatusLine().getStatusCode(), 405);
+            response.close();
+
+            //trace root
+            response = httpClient.execute(httpTraceRoot);
+            Assert.assertEquals(response.getStatusLine().getStatusCode(), 405);
+            response.close();
+
+            //put
+            response = httpClient.execute(httpPut);
+            Assert.assertEquals(response.getStatusLine().getStatusCode(), 405);
+            response.close();
+
+            //delete
+            response = httpClient.execute(httpDelete);
+            Assert.assertEquals(response.getStatusLine().getStatusCode(), 405);
+            response.close();
+
+            //options
+            response = httpClient.execute(httpOptions);
+            Assert.assertEquals(response.getStatusLine().getStatusCode(), 405);
+            response.close();
+
+            //head
+            response = httpClient.execute(httpHead);
+            Assert.assertEquals(response.getStatusLine().getStatusCode(), 405);
+            response.close();
+        } catch (IOException e) {
+            Assert.fail("Test failed!");
+            e.printStackTrace();
+        } finally {
+            try {
+                if (httpClient != null) {
+                    httpClient.close();
+                }
+                if (response != null) {
+                    response.close();
+                }
+            } catch (IOException e) {
+                e.printStackTrace();
+            }
+        }
+    }
+
+    static class TestPostHandler extends JettyHandler {
+
+        @Override
+        public String pathSpec() {
+            return "/test";
+        }
+
+        @Override
+        protected void doPost(final HttpServletRequest req,
+                              final HttpServletResponse resp) throws ServletException, IOException {
+            resp.setStatus(HttpServletResponse.SC_OK);
+        }
+
+    }
+}

oap-server/server-library/library-server/src/main/java/org/apache/skywalking/oap/server/library/server/jetty/JettyDefaultHandler.java

+/*
+ * 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.skywalking.oap.server.library.server.jetty;
+
+import java.io.IOException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+public class JettyDefaultHandler extends JettyHandler {
+    @Override
+    public String pathSpec() {
+        return "/";
+    }
+
+    @Override
+    protected final void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        resp.sendError(HttpServletResponse.SC_NOT_FOUND);
+    }
+
+}

oap-server/server-library/library-server/src/main/java/org/apache/skywalking/oap/server/library/server/jetty/JettyHandler.java

@@ -18,9 +18,64 @@
 
 package org.apache.skywalking.oap.server.library.server.jetty;
 
+import java.io.IOException;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 import org.apache.skywalking.oap.server.library.server.ServerHandler;
 
 public abstract class JettyHandler extends HttpServlet implements ServerHandler {
     public abstract String pathSpec();
+
+    @Override
+    protected final void service(HttpServletRequest req,
+                                 HttpServletResponse resp) throws ServletException, IOException {
+        String method = req.getMethod();
+        if (method.equals("POST")) {
+            super.service(req, resp);
+        } else {
+            resp.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
+        }
+    }
+
+    @Override
+    public final void service(ServletRequest req, ServletResponse res) throws ServletException, IOException {
+        super.service(req, res);
+    }
+
+    @Override
+    protected final void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException, ServletException {
+        super.doGet(req, resp);
+    }
+
+    @Override
+    protected final void doHead(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
+        super.doHead(req, resp);
+    }
+
+    @Override
+    protected final void doPut(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
+        super.doPut(req, resp);
+    }
+
+    @Override
+    protected final void doDelete(HttpServletRequest req,
+                                  HttpServletResponse resp) throws ServletException, IOException {
+        super.doDelete(req, resp);
+    }
+
+    @Override
+    protected final void doOptions(HttpServletRequest req,
+                                   HttpServletResponse resp) throws ServletException, IOException {
+        super.doOptions(req, resp);
+    }
+
+    @Override
+    protected final void doTrace(HttpServletRequest req,
+                                 HttpServletResponse resp) throws ServletException, IOException {
+        super.doTrace(req, resp);
+    }
 }

oap-server/server-library/library-server/src/main/java/org/apache/skywalking/oap/server/library/server/jetty/JettyJsonHandler.java

@@ -22,12 +22,6 @@ import com.google.gson.JsonElement;
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.PrintWriter;
-import java.util.Enumeration;
-import javax.servlet.ServletConfig;
-import javax.servlet.ServletContext;
-import javax.servlet.ServletException;
-import javax.servlet.ServletRequest;
-import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import org.slf4j.Logger;
@@ -38,21 +32,6 @@ import static java.util.Objects.nonNull;
 public abstract class JettyJsonHandler extends JettyHandler {
     private static final Logger LOGGER = LoggerFactory.getLogger(JettyJsonHandler.class);
 
-    @Override
-    protected final void doGet(HttpServletRequest req, HttpServletResponse resp) {
-        try {
-            reply(resp, doGet(req));
-        } catch (ArgumentsParseException | IOException e) {
-            try {
-                replyError(resp, e.getMessage(), HttpServletResponse.SC_BAD_REQUEST);
-            } catch (IOException replyException) {
-                LOGGER.error(replyException.getMessage(), e);
-            }
-        }
-    }
-
-    protected abstract JsonElement doGet(HttpServletRequest req) throws ArgumentsParseException;
-
     @Override
     protected final void doPost(HttpServletRequest req, HttpServletResponse resp) {
         try {
@@ -68,105 +47,6 @@ public abstract class JettyJsonHandler extends JettyHandler {
 
     protected abstract JsonElement doPost(HttpServletRequest req) throws ArgumentsParseException, IOException;
 
-    @Override
-    protected final void doHead(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
-        super.doHead(req, resp);
-    }
-
-    @Override
-    protected final long getLastModified(HttpServletRequest req) {
-        return super.getLastModified(req);
-    }
-
-    @Override
-    protected final void doPut(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
-        super.doPut(req, resp);
-    }
-
-    @Override
-    protected final void doDelete(HttpServletRequest req,
-                                  HttpServletResponse resp) throws ServletException, IOException {
-        super.doDelete(req, resp);
-    }
-
-    @Override
-    protected final void doOptions(HttpServletRequest req,
-                                   HttpServletResponse resp) throws ServletException, IOException {
-        super.doOptions(req, resp);
-    }
-
-    @Override
-    protected final void doTrace(HttpServletRequest req,
-                                 HttpServletResponse resp) throws ServletException, IOException {
-        super.doTrace(req, resp);
-    }
-
-    @Override
-    protected final void service(HttpServletRequest req,
-                                 HttpServletResponse resp) throws ServletException, IOException {
-        super.service(req, resp);
-    }
-
-    @Override
-    public final void service(ServletRequest req, ServletResponse res) throws ServletException, IOException {
-        super.service(req, res);
-    }
-
-    @Override
-    public final void destroy() {
-        super.destroy();
-    }
-
-    @Override
-    public final String getInitParameter(String name) {
-        return super.getInitParameter(name);
-    }
-
-    @Override
-    public final Enumeration<String> getInitParameterNames() {
-        return super.getInitParameterNames();
-    }
-
-    @Override
-    public final ServletConfig getServletConfig() {
-        return super.getServletConfig();
-    }
-
-    @Override
-    public final ServletContext getServletContext() {
-        return super.getServletContext();
-    }
-
-    @Override
-    public final String getServletInfo() {
-        return super.getServletInfo();
-    }
-
-    @Override
-    public final void init(ServletConfig config) throws ServletException {
-        super.init(config);
-    }
-
-    @Override
-    public final void init() throws ServletException {
-        super.init();
-    }
-
-    @Override
-    public final void log(String msg) {
-        super.log(msg);
-    }
-
-    @Override
-    public final void log(String message, Throwable t) {
-        super.log(message, t);
-    }
-
-    @Override
-    public final String getServletName() {
-        return super.getServletName();
-    }
-
     private void reply(HttpServletResponse response, JsonElement resJson) throws IOException {
         response.setContentType("application/json");
         response.setCharacterEncoding("utf-8");

oap-server/server-library/library-server/src/main/java/org/apache/skywalking/oap/server/library/server/jetty/JettyServer.java

@@ -73,6 +73,12 @@ public class JettyServer implements Server {
         LOGGER.info("http server root context path: {}", jettyServerConfig.getContextPath());
 
         server.setHandler(servletContextHandler);
+
+        JettyDefaultHandler defaultHandler = new JettyDefaultHandler();
+        ServletHolder defaultHolder = new ServletHolder();
+        defaultHolder.setServlet(defaultHandler);
+
+        servletContextHandler.addServlet(defaultHolder, defaultHandler.pathSpec());
     }
 
     public void addHandler(JettyHandler handler) {
@@ -133,4 +139,5 @@ public class JettyServer implements Server {
     public int hashCode() {
         return Objects.hash(jettyServerConfig.getHost(), jettyServerConfig.getPort());
     }
+
 }

oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/GraphQLQueryHandler.java

@@ -64,11 +64,6 @@ public class GraphQLQueryHandler extends JettyJsonHandler {
         return path;
     }
 
-    @Override
-    protected JsonElement doGet(HttpServletRequest req) {
-        throw new UnsupportedOperationException("GraphQL only supports POST method");
-    }
-
     @Override
     protected JsonElement doPost(HttpServletRequest req) throws IOException {
         BufferedReader reader = new BufferedReader(new InputStreamReader(req.getInputStream()));

oap-server/server-receiver-plugin/skywalking-browser-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/browser/provider/handler/rest/BrowserErrorLogReportBaseServletHandler.java

@@ -68,12 +68,6 @@ public abstract class BrowserErrorLogReportBaseServletHandler extends JettyHandl
         );
     }
 
-    @Override
-    protected void doGet(final HttpServletRequest req,
-                         final HttpServletResponse resp) throws ServletException, IOException {
-        throw new UnsupportedOperationException();
-    }
-
     @Override
     protected void doPost(final HttpServletRequest req,
                           final HttpServletResponse resp) throws ServletException, IOException {

oap-server/server-receiver-plugin/skywalking-browser-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/browser/provider/handler/rest/BrowserPerfDataReportServletHandler.java

@@ -68,12 +68,6 @@ public class BrowserPerfDataReportServletHandler extends JettyHandler {
         );
     }
 
-    @Override
-    protected void doGet(final HttpServletRequest req,
-                         final HttpServletResponse resp) throws ServletException, IOException {
-        throw new UnsupportedOperationException();
-    }
-
     @Override
     protected void doPost(final HttpServletRequest req,
                           final HttpServletResponse resp) throws ServletException, IOException {

oap-server/server-receiver-plugin/skywalking-management-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/register/provider/handler/v8/rest/ManagementServiceKeepAliveHandler.java

@@ -50,11 +50,6 @@ public class ManagementServiceKeepAliveHandler extends JettyJsonHandler {
                                           .getService(NamingControl.class);
     }
 
-    @Override
-    protected JsonElement doGet(final HttpServletRequest req) throws ArgumentsParseException {
-        throw new UnsupportedOperationException();
-    }
-
     @Override
     protected JsonElement doPost(final HttpServletRequest req) throws ArgumentsParseException, IOException {
         final InstanceProperties.Builder request = InstanceProperties.newBuilder();

oap-server/server-receiver-plugin/skywalking-management-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/register/provider/handler/v8/rest/ManagementServiceReportPropertiesHandler.java

@@ -54,11 +54,6 @@ public class ManagementServiceReportPropertiesHandler extends JettyJsonHandler {
                                           .getService(NamingControl.class);
     }
 
-    @Override
-    protected JsonElement doGet(final HttpServletRequest req) throws ArgumentsParseException {
-        throw new UnsupportedOperationException();
-    }
-
     @Override
     protected JsonElement doPost(final HttpServletRequest req) throws ArgumentsParseException, IOException {
         final InstanceProperties.Builder request = InstanceProperties.newBuilder();

oap-server/server-receiver-plugin/skywalking-trace-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/trace/provider/handler/v8/rest/TraceSegmentReportBaseServletHandler.java

@@ -59,11 +59,6 @@ public abstract class TraceSegmentReportBaseServletHandler extends JettyJsonHand
         );
     }
 
-    @Override
-    protected JsonElement doGet(HttpServletRequest req) {
-        throw new UnsupportedOperationException();
-    }
-
     @Override
     protected JsonElement doPost(HttpServletRequest req) {
         if (log.isDebugEnabled()) {