From ab0fd108dae208ae9c176b572d678066b66894d3 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 2 Feb 2017 12:02:15 +0100 Subject: [PATCH] SONAR-8625 Return 404 on unknown WS controller --- .../org/sonar/server/ws/WebServiceFilter.java | 49 +++++++++++-------- .../sonar/server/ws/WebServiceFilterTest.java | 41 +++++++++++----- 2 files changed, 59 insertions(+), 31 deletions(-) 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 e0a9aeb9ebc..4d297b7996d 100644 --- 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 : + * */ public class WebServiceFilter extends ServletFilter { private final WebServiceEngine webServiceEngine; - private final List includeUrls = new ArrayList<>(); - private final List excludeUrls = new ArrayList<>(); + private final Set includeUrls; + private final Set 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 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 de0eaeafa1f..b8e16314fa4 100644 --- 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 -- 2.39.5