SONAR-8625 Return 404 on unknown WS controller

diff --git a/server/sonar-server/src/main/java/org/sonar/server/ws/WebServiceFilter.java b/server/sonar-server/src/main/java/org/sonar/server/ws/WebServiceFilter.java
index e0a9aeb9ebc2231d1d241b7a498395326c45eea4..4d297b7996d2c59fb3d049b37176b343b658140a 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/ws/WebServiceFilter.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/ws/WebServiceFilter.java
@@ -21,8 +21,9 @@
 package org.sonar.server.ws;
 
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Stream;
 import javax.servlet.FilterChain;
 import javax.servlet.FilterConfig;
 import javax.servlet.ServletException;
@@ -30,33 +31,42 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.api.web.ServletFilter;
+import org.sonar.core.util.stream.Collectors;
 
 import static org.sonar.server.property.ws.PropertiesWs.CONTROLLER_PROPERTIES;
 
 /**
- * This filter is used to execute Java WS.
+ * This filter is used to execute Web Services.
  *
- * If the url match a Java WS, the output of the WS is returned and no other filers are executed.
- * If the url doesn't match a Java WS, then it's calling remaining filters (for instance to execute Rails WS).
+ * Every urls beginning with '/api' and every web service urls are taken into account, except :
+ * <ul>
+ *   <li>web services that directly implemented with servlet filter, see {@link ServletFilterHandler})</li>
+ *   <li>deprecated '/api/properties' web service, see {@link DeprecatedPropertiesWsFilter}</li>
+ * </ul>
  */
 public class WebServiceFilter extends ServletFilter {
 
   private final WebServiceEngine webServiceEngine;
-  private final List<String> includeUrls = new ArrayList<>();
-  private final List<String> excludeUrls = new ArrayList<>();
+  private final Set<String> includeUrls;
+  private final Set<String> excludeUrls;
 
   public WebServiceFilter(WebServiceEngine webServiceEngine) {
     this.webServiceEngine = webServiceEngine;
-    webServiceEngine.controllers()
-      .forEach(controller -> controller.actions()
-        .forEach(action -> {
-          // Rest and servlet filter WS should not be executed by the web service engine
-          if (shouldBeExecutedByWebServiceEngine(controller, action)) {
-            includeUrls.add("/" + controller.path() + "/*");
-          } else {
-            excludeUrls.add("/" + action.path() + "*");
-          }
-        }));
+    this.includeUrls = Stream.concat(
+      Stream.of("/api/*"),
+      webServiceEngine.controllers()
+        .stream()
+        .flatMap(controller -> controller.actions().stream()
+          .map(toPath())))
+      .collect(Collectors.toSet());
+    this.excludeUrls = Stream.concat(
+      Stream.of("/" + CONTROLLER_PROPERTIES + "*"),
+      webServiceEngine.controllers()
+        .stream()
+        .flatMap(controller -> controller.actions().stream()
+          .filter(action -> action.handler() instanceof ServletFilterHandler)
+          .map(toPath())))
+      .collect(Collectors.toSet());
   }
 
   @Override
@@ -86,9 +96,8 @@ public class WebServiceFilter extends ServletFilter {
     // Nothing to do
   }
 
-  private static boolean shouldBeExecutedByWebServiceEngine(WebService.Controller controller, WebService.Action action) {
-    return !(action.handler() instanceof ServletFilterHandler)
-      && !controller.path().equals(CONTROLLER_PROPERTIES);
+  private static Function<WebService.Action, String> toPath() {
+    return action -> "/" + action.path() + "/*";
   }
 
 }

diff --git a/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceFilterTest.java b/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceFilterTest.java
index de0eaeafa1fb7e8944c8228e7a5ba83cd67ee074..b8e16314fa4b81fdce6c3e8b921c98e579fb7bc5 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceFilterTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceFilterTest.java
@@ -47,14 +47,14 @@ public class WebServiceFilterTest {
   @Rule
   public ExpectedException expectedException = ExpectedException.none();
 
-  WebServiceEngine webServiceEngine = mock(WebServiceEngine.class);
+  private WebServiceEngine webServiceEngine = mock(WebServiceEngine.class);
 
-  HttpServletRequest request = mock(HttpServletRequest.class);
-  HttpServletResponse response = mock(HttpServletResponse.class);
-  FilterChain chain = mock(FilterChain.class);
-  ServletOutputStream responseOutput = mock(ServletOutputStream.class);
+  private HttpServletRequest request = mock(HttpServletRequest.class);
+  private HttpServletResponse response = mock(HttpServletResponse.class);
+  private FilterChain chain = mock(FilterChain.class);
+  private ServletOutputStream responseOutput = mock(ServletOutputStream.class);
 
-  WebServiceFilter underTest;
+  private WebServiceFilter underTest;
 
   @Before
   public void setUp() throws Exception {
@@ -63,20 +63,39 @@ public class WebServiceFilterTest {
   }
 
   @Test
-  public void do_get_pattern() throws Exception {
+  public void match_declared_web_services() throws Exception {
     initWebServiceEngine(
       newWsUrl("api/issues", "search"),
-      newWsUrl("batch", "index"),
-      newWsUrl("api/authentication", "login").setHandler(ServletFilterHandler.INSTANCE));
+      newWsUrl("batch", "index"));
 
     assertThat(underTest.doGetPattern().matches("/api/issues/search")).isTrue();
     assertThat(underTest.doGetPattern().matches("/api/issues/search.protobuf")).isTrue();
     assertThat(underTest.doGetPattern().matches("/batch/index")).isTrue();
 
-    assertThat(underTest.doGetPattern().matches("/api/resources/index")).isFalse();
+    assertThat(underTest.doGetPattern().matches("/foo")).isFalse();
+  }
+
+  @Test
+  public void match_undeclared_web_services_starting_with_api() throws Exception {
+    initWebServiceEngine(newWsUrl("api/issues", "search"));
+
+    assertThat(underTest.doGetPattern().matches("/api/resources/index")).isTrue();
+    assertThat(underTest.doGetPattern().matches("/api/user_properties")).isTrue();
+  }
+
+  @Test
+  public void does_not_match_web_services_using_servlet_filter() throws Exception {
+    initWebServiceEngine(newWsUrl("api/authentication", "login").setHandler(ServletFilterHandler.INSTANCE));
+
     assertThat(underTest.doGetPattern().matches("/api/authentication/login")).isFalse();
+  }
+
+  @Test
+  public void does_not_match_api_properties_ws() throws Exception {
+    initWebServiceEngine(newWsUrl("api/properties", "index"));
+
     assertThat(underTest.doGetPattern().matches("/api/properties")).isFalse();
-    assertThat(underTest.doGetPattern().matches("/foo")).isFalse();
+    assertThat(underTest.doGetPattern().matches("/api/properties/index")).isFalse();
   }
 
   @Test