410800 Make RewritePatternRule queryString aware

2 files changed, 126 insertions, 23 deletions

diff --git a/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RewritePatternRule.java b/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RewritePatternRule.java
index 48047b8647..d6bd65d15e 100644
--- a/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RewritePatternRule.java
+++ b/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RewritePatternRule.java
@@ -19,20 +19,21 @@
 package org.eclipse.jetty.rewrite.handler;
 
 import java.io.IOException;
-
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.eclipse.jetty.http.HttpURI;
 import org.eclipse.jetty.http.PathMap;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.util.URIUtil;
 
 /**
- * Rewrite the URI by replacing the matched {@link PathMap} path with a fixed string. 
+ * Rewrite the URI by replacing the matched {@link PathMap} path with a fixed string.
  */
 public class RewritePatternRule extends PatternRule implements Rule.ApplyURI
 {
     private String _replacement;
+    private String _query;
 
     /* ------------------------------------------------------------ */
     public RewritePatternRule()
@@ -44,32 +45,63 @@ public class RewritePatternRule extends PatternRule implements Rule.ApplyURI
     /* ------------------------------------------------------------ */
     /**
      * Whenever a match is found, it replaces with this value.
-     * 
-     * @param value the replacement string.
+     *
+     * @param replacement the replacement string.
      */
-    public void setReplacement(String value)
+    public void setReplacement(String replacement)
     {
-        _replacement = value;
+        String[] split = replacement.split("\\?", 2);
+        _replacement = split[0];
+        _query = split.length == 2 ? split[1] : null;
     }
 
     /* ------------------------------------------------------------ */
     /*
      * (non-Javadoc)
-     * @see org.eclipse.jetty.server.handler.rules.RuleBase#apply(javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse)
+     *
+     * @see org.eclipse.jetty.server.handler.rules.RuleBase#apply(javax.servlet.http.HttpServletRequest,
+     * javax.servlet.http.HttpServletResponse)
      */
     @Override
     public String apply(String target, HttpServletRequest request, HttpServletResponse response) throws IOException
     {
-        target = URIUtil.addPaths(_replacement, PathMap.pathInfo(_pattern,target));   
+        target = URIUtil.addPaths(_replacement, PathMap.pathInfo(_pattern, target));
         return target;
     }
 
     /* ------------------------------------------------------------ */
+
+    /**
+     * This method will add _query to the requests's queryString and also combine it with existing queryStrings in
+     * the request. However it won't take care for duplicate. E.g. if request.getQueryString contains a parameter
+     * "param1 = true" and _query will contain "param1=false" the result will be param1=true&param1=false.
+     * To cover this use case some more complex pattern matching is necessary. We can implement this if there's use
+     * cases.
+     *
+     * @param request
+     * @param oldTarget
+     * @param newTarget
+     * @throws IOException
+     */
     @Override
-    public void applyURI(Request request, String oldTarget, String newTarget) throws IOException 
+    public void applyURI(Request request, String oldTarget, String newTarget) throws IOException
     {
-        String uri = URIUtil.addPaths(_replacement, PathMap.pathInfo(_pattern,request.getRequestURI()));
-        request.setRequestURI(uri);
+        if (_query == null)
+        {
+            request.setRequestURI(newTarget);
+        }
+        else
+        {
+            String queryString = request.getQueryString();
+            if (queryString != null)
+                queryString = queryString + "&" + _query;
+            else
+                queryString = _query;
+            HttpURI uri = new HttpURI(newTarget + "?" + queryString);
+            request.setUri(uri);
+            request.setRequestURI(newTarget);
+            request.setQueryString(queryString);
+        }
     }
 
     /* ------------------------------------------------------------ */
diff --git a/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/RewritePatternRuleTest.java b/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/RewritePatternRuleTest.java
index 51ed119d12..20d52fb2dc 100644
--- a/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/RewritePatternRuleTest.java
+++ b/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/RewritePatternRuleTest.java
@@ -18,23 +18,25 @@
 
 package org.eclipse.jetty.rewrite.handler;
 
-import static org.junit.Assert.assertEquals;
-
 import java.io.IOException;
 
+import org.eclipse.jetty.http.HttpURI;
 import org.junit.Before;
 import org.junit.Test;
 
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
 public class RewritePatternRuleTest extends AbstractRuleTestCase
 {
-    private String[][] _tests=
-    {
-            {"/foo/bar","/","/replace"},
-            {"/foo/bar","/*","/replace/foo/bar"},
-            {"/foo/bar","/foo/*","/replace/bar"},
-            {"/foo/bar","/foo/bar","/replace"},
-            {"/foo/bar.txt","*.txt","/replace"},
-    };
+    private String[][] _tests =
+            {
+                    {"/foo/bar", "/", "/replace"},
+                    {"/foo/bar", "/*", "/replace/foo/bar"},
+                    {"/foo/bar", "/foo/*", "/replace/bar"},
+                    {"/foo/bar", "/foo/bar", "/replace"},
+                    {"/foo/bar.txt", "*.txt", "/replace"},
+            };
     private RewritePatternRule _rule;
 
     @Before
@@ -46,13 +48,82 @@ public class RewritePatternRuleTest extends AbstractRuleTestCase
     }
 
     @Test
-    public void testRequestUriEnabled() throws IOException
+    public void testMatchAndApplyAndApplyURI() throws IOException
     {
         for (String[] test : _tests)
         {
             _rule.setPattern(test[1]);
             String result = _rule.matchAndApply(test[0], _request, _response);
-            assertEquals(test[1], test[2], result);
+            assertThat(test[1], test[2], is(result));
+
+            _rule.applyURI(_request, null, result);
+            assertThat(_request.getRequestURI(), is(test[2]));
         }
     }
+
+    @Test
+    public void testReplacementWithQueryString() throws IOException
+    {
+        String replacement = "/replace?given=param";
+        String[] split = replacement.split("\\?", 2);
+        String path = split[0];
+        String queryString = split[1];
+
+        RewritePatternRule rewritePatternRule = new RewritePatternRule();
+        rewritePatternRule.setPattern("/old/context");
+        rewritePatternRule.setReplacement(replacement);
+
+        String result = rewritePatternRule.matchAndApply("/old/context", _request, _response);
+        assertThat(result, is(path));
+
+        rewritePatternRule.applyURI(_request, null, result);
+        assertThat("queryString matches expected", _request.getQueryString(), is(queryString));
+        assertThat("request URI matches expected", _request.getRequestURI(), is(path));
+
+    }
+
+    @Test
+    public void testRequestWithQueryString() throws IOException
+    {
+        String replacement = "/replace";
+        String queryString = "request=parameter";
+        _request.setUri(new HttpURI("/old/context"));
+        _request.setQueryString(queryString);
+
+        RewritePatternRule rewritePatternRule = new RewritePatternRule();
+        rewritePatternRule.setPattern("/old/context");
+        rewritePatternRule.setReplacement(replacement);
+
+        String result = rewritePatternRule.matchAndApply("/old/context", _request, _response);
+        assertThat("result matches expected", result, is(replacement));
+
+        rewritePatternRule.applyURI(_request, null, result);
+        assertThat("queryString matches expected", _request.getQueryString(), is(queryString));
+        assertThat("request URI matches expected", _request.getRequestURI(), is(replacement));
+    }
+
+    @Test
+    public void testRequestAndReplacementWithQueryString() throws IOException
+    {
+        String requestQueryString = "request=parameter";
+        String replacement = "/replace?given=param";
+        String[] split = replacement.split("\\?", 2);
+        String path = split[0];
+        String queryString = split[1];
+        _request.setUri(new HttpURI("/old/context"));
+        _request.setQueryString(requestQueryString);
+
+        RewritePatternRule rewritePatternRule = new RewritePatternRule();
+        rewritePatternRule.setPattern("/old/context");
+        rewritePatternRule.setReplacement(replacement);
+
+        String result = rewritePatternRule.matchAndApply("/old/context", _request, _response);
+        assertThat(result, is(path));
+
+        rewritePatternRule.applyURI(_request, null, result);
+        assertThat("queryString matches expected", _request.getQueryString(),
+                is(requestQueryString + "&" + queryString));
+        assertThat("request URI matches expected", _request.getRequestURI(), is(path));
+    }
+
 }