KNOX-1157 - Scoped rewrite rules are treated as global rules in some cases (Wei Han...
authorWei Han <weihan@uber.com>
Thu, 10 May 2018 18:10:22 +0000 (11:10 -0700)
committerPhil Zampino <pzampino@apache.org>
Wed, 16 May 2018 18:33:56 +0000 (14:33 -0400)
gateway-provider-rewrite/src/main/java/org/apache/knox/gateway/filter/rewrite/ext/ScopedMatcher.java
gateway-provider-rewrite/src/test/java/org/apache/knox/gateway/filter/rewrite/api/UrlRewriteProcessorTest.java
gateway-provider-rewrite/src/test/resources/org/apache/knox/gateway/filter/rewrite/api/UrlRewriteProcessorTest/rewrite-with-same-rules-different-scope.xml

index 08efde4..f9bd0b1 100644 (file)
@@ -23,6 +23,7 @@ import org.apache.knox.gateway.util.urltemplate.Template;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.HashMap;
 
 /**
  * A simple extension to the matcher that takes into account scopes for rules along with the templates themselves.
@@ -33,12 +34,11 @@ public class ScopedMatcher extends Matcher<UrlRewriteRuleProcessorHolder> {
 
   public static final String GLOBAL_SCOPE = "GLOBAL";
 
-  private List<Matcher<UrlRewriteRuleProcessorHolder>> matchers;
+  private HashMap<String, Matcher<UrlRewriteRuleProcessorHolder>> matchers;
 
   public ScopedMatcher() {
     super();
-    matchers = new ArrayList<>();
-    matchers.add(new Matcher<UrlRewriteRuleProcessorHolder>());
+    matchers = new HashMap<>();
   }
 
   @Override
@@ -59,7 +59,7 @@ public class ScopedMatcher extends Matcher<UrlRewriteRuleProcessorHolder> {
 
   public Match match(Template input, String scope) {
     List<Match> matches = new ArrayList<>();
-    for (Matcher<UrlRewriteRuleProcessorHolder> matcher : matchers) {
+    for (Matcher<UrlRewriteRuleProcessorHolder> matcher : matchers.values()) {
       Match match = matcher.match(input);
       if (match != null) {
         matches.add(match);
@@ -113,17 +113,11 @@ public class ScopedMatcher extends Matcher<UrlRewriteRuleProcessorHolder> {
    * @return a matcher
    */
   private Matcher<UrlRewriteRuleProcessorHolder> getMatcher(Template template, UrlRewriteRuleProcessorHolder holder) {
-    for (Matcher<UrlRewriteRuleProcessorHolder> matcher : matchers) {
-      UrlRewriteRuleProcessorHolder matchersHolder = matcher.get(template);
-      if (matchersHolder == null) {
-        return matcher;
-      } else if (holder.getScope() == null && matchersHolder.getScope() == null) {
-        return matcher;
-      }
+    String scope = holder.getScope();
+    if (!matchers.containsKey(scope)) {
+      matchers.put(scope, new Matcher<UrlRewriteRuleProcessorHolder>());
     }
-    Matcher<UrlRewriteRuleProcessorHolder> matcher = new Matcher<>();
-    matchers.add(matcher);
-    return matcher;
-  }
 
+    return matchers.get(scope);
+  }
 }
index ff1093b..dd6fc53 100644 (file)
@@ -184,6 +184,26 @@ public class UrlRewriteProcessorTest {
         "Expect rewrite to contain the correct path.",
         outputUrl.toString(), is( "input-mock-scheme-2://input-mock-host-2:42/test-input-path" ) );
 
+    //Test the scenario where input could match two different rules
+    inputUrl = Parser.parseLiteral( "/foo/bar" );
+
+    roles.remove(0);
+    roles.add("service-1");
+    outputUrl = processor.rewrite( environment, inputUrl, UrlRewriter.Direction.OUT, null);
+
+    assertThat(
+            "Expect rewrite to contain the correct path.",
+            outputUrl.toString(), is( "/foo/service-1" ) );
+
+    roles.remove(0);
+    roles.add("service-2");
+
+    outputUrl = processor.rewrite( environment, inputUrl, UrlRewriter.Direction.OUT, null);
+
+    assertThat(
+            "Expect rewrite to contain the correct path.",
+            outputUrl.toString(), is( "/foo/service-2" ) );
+
     processor.destroy();
   }
 
index 6c27476..d1e76db 100644 (file)
         <rewrite template="input-mock-scheme-2://input-mock-host-2:{port}/{path=**}" />
     </rule>
 
+    <rule name="service-1/test-rule-8" dir="OUT" pattern="/foo/{**}">
+        <rewrite template="/foo/service-1" />
+    </rule>
+
+    <rule name="service-2/test-rule-9" dir="OUT" pattern="/foo/bar">
+        <rewrite template="/foo/service-2" />
+    </rule>
+
 </rules>