From: Julien HENRY <julien.henry@sonarsource.com>
Date: Tue, 3 Apr 2018 12:27:43 +0000 (+0200)
Subject: Fix WS routing
X-Git-Tag: 7.5~1414
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=6ab3e013a2cc0f3c90c6cff8f89ab015977a561f;p=sonarqube.git

Fix WS routing

When a regular WS path is a suffix of a "ServletFilter" path, the regular WS can't be reached.
---

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 4691c046295..1efd88c1065 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
@@ -95,7 +95,7 @@ public class WebServiceFilter extends ServletFilter {
   }
 
   private static Function<WebService.Action, String> toPath() {
-    return action -> "/" + action.path() + "/*";
+    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 a7d5969a97e..b2c7c21c359 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
@@ -62,7 +62,7 @@ public class WebServiceFilterTest {
   }
 
   @Test
-  public void match_declared_web_services() {
+  public void match_declared_web_services_with_optional_suffix() {
     initWebServiceEngine(
       newWsUrl("api/issues", "search"),
       newWsUrl("batch", "index"));
@@ -70,6 +70,7 @@ public class WebServiceFilterTest {
     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("/batch/index.protobuf")).isTrue();
 
     assertThat(underTest.doGetPattern().matches("/foo")).isFalse();
   }
@@ -89,6 +90,16 @@ public class WebServiceFilterTest {
     assertThat(underTest.doGetPattern().matches("/api/authentication/login")).isFalse();
   }
 
+  @Test
+  public void does_not_match_servlet_filter_that_prefix_a_ws() {
+    initWebServiceEngine(
+        newWsUrl("api/foo", "action").setHandler(ServletFilterHandler.INSTANCE),
+        newWsUrl("api/foo", "action_2"));
+
+    assertThat(underTest.doGetPattern().matches("/api/foo/action")).isFalse();
+    assertThat(underTest.doGetPattern().matches("/api/foo/action_2")).isTrue();
+  }
+
   @Test
   public void does_not_match_api_properties_ws() {
     initWebServiceEngine(newWsUrl("api/properties", "index"));
diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/web/ServletFilter.java b/sonar-plugin-api/src/main/java/org/sonar/api/web/ServletFilter.java
index 5fd7540ac1d..93e549358dd 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/web/ServletFilter.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/web/ServletFilter.java
@@ -35,6 +35,7 @@ import org.sonar.api.server.ServerSide;
 import static com.google.common.base.Preconditions.checkArgument;
 import static java.util.Arrays.asList;
 import static java.util.Collections.unmodifiableList;
+import static org.apache.commons.lang.StringUtils.substringBeforeLast;
 
 /**
  * @since 3.1
@@ -160,7 +161,8 @@ public abstract class ServletFilter implements Filter {
       /**
        * Add inclusion patterns. Supported formats are:
        * <ul>
-       *   <li>path prefixed by / and ended by *, for example "/api/foo/*", to match all paths "/api/foo" and "api/api/foo/something/else"</li>
+       *   <li>path prefixed by / and ended by * or /*, for example "/api/foo/*", to match all paths "/api/foo" and "api/api/foo/something/else"</li>
+       *   <li>path prefixed by / and ended by .*, for example "/api/foo.*", to match exact path "/api/foo" with any suffix like "/api/foo.protobuf"</li>
        *   <li>path prefixed by *, for example "*\/foo", to match all paths "/api/foo" and "something/else/foo"</li>
        *   <li>path with leading slash and no wildcard, for example "/api/foo", to match exact path "/api/foo"</li>
        * </ul>
@@ -206,8 +208,15 @@ public abstract class ServletFilter implements Filter {
         checkArgument(countStars == 1, "URL pattern accepts only zero or one wildcard character '*': %s", pattern);
         if (pattern.charAt(0) == '/') {
           checkArgument(pattern.endsWith(WILDCARD_CHAR), "URL pattern must end with wildcard character '*': %s", pattern);
-          // remove the ending /* or *
-          String path = pattern.replaceAll("/?\\*", "");
+          if (pattern.endsWith("/*")) {
+            String path = pattern.substring(0, pattern.length() - "/*".length());
+            return url -> url.startsWith(path);
+          }
+          if (pattern.endsWith(".*")) {
+            String path = pattern.substring(0, pattern.length() - ".*".length());
+            return url -> substringBeforeLast(url, ".").equals(path);
+          }
+          String path = pattern.substring(0, pattern.length() - "*".length());
           return url -> url.startsWith(path);
         }
         checkArgument(pattern.startsWith(WILDCARD_CHAR), "URL pattern must start with wildcard character '*': %s", pattern);
diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/web/ServletFilterTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/web/ServletFilterTest.java
index 6d98e78e2b0..43e3379cf57 100644
--- a/sonar-plugin-api/src/test/java/org/sonar/api/web/ServletFilterTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/web/ServletFilterTest.java
@@ -158,6 +158,17 @@ public class ServletFilterTest {
     assertThat(pattern.matches("/foo/index")).isTrue();
   }
 
+  @Test
+  public void use_include_and_exclude_prefix() {
+    ServletFilter.UrlPattern pattern = ServletFilter.UrlPattern.builder()
+        .includes("/foo_2")
+        .excludes("/foo")
+        .build();
+    assertThat(pattern.matches("/")).isFalse();
+    assertThat(pattern.matches("/foo_2")).isTrue();
+    assertThat(pattern.matches("/foo")).isFalse();
+  }
+
   @Test
   public void exclude_pattern_has_higher_priority_than_include_pattern() {
     ServletFilter.UrlPattern pattern = ServletFilter.UrlPattern.builder()