Issue 116: qsappend fix

paultuckey · Jul 31, 2012 · 8047d4f · 8047d4f
1 parent 0dc4fd1
commit 8047d4f
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 3 deletions.
diff --git a/src/doc/manual/4.0/introduction.html b/src/doc/manual/4.0/introduction.html
@@ -189,6 +189,19 @@ <h2>License</h2>
  <h2>Changelog</h2>
 
     <ul>
+        <li>4.0.5
+            <ul>
+                <li>Bug fix for appending query strings breaking URL
+                    (<a href="http://code.google.com/p/urlrewritefilter/issues/detail?id=116">issue 116</a>)</li>
+            </ul>
+        </li>
+
+        <li>4.0.4
+            <ul>
+                <li>Changes to build process (not formally released)</li>
+            </ul>
+        </li>
+
         <li>4.0.3
             <ul>
                 <li>Minor fix for identifying versions</li>

diff --git a/src/main/java/org/tuckey/web/filters/urlrewrite/NormalRule.java b/src/main/java/org/tuckey/web/filters/urlrewrite/NormalRule.java
@@ -94,9 +94,13 @@ public RewrittenUrl matches(final String url, final HttpServletRequest hsRequest
             // no match, or run/set only match
             return null;
         }
-        if ( queryStringAppend && hsRequest.getQueryString() != null ) {
+        if ( queryStringAppend && hsRequest.getQueryString() != null && hsRequest.getQueryString().length() > 0) {
             String target = ruleExecutionOutput.getReplacedUrl();
-            ruleExecutionOutput.setReplacedUrl(target + "&" + hsRequest.getQueryString());
+            if (target.contains("?")) {
+                ruleExecutionOutput.setReplacedUrl(target + "&" + hsRequest.getQueryString());
+            } else {
+                ruleExecutionOutput.setReplacedUrl(target + "?" + hsRequest.getQueryString());
+            }
         }
         if ( toServletContext != null ) ruleExecutionOutput.setReplacedUrlContext(toServletContext);
         return RuleExecutionOutput.getRewritenUrl(toType, encodeToUrl, ruleExecutionOutput);

diff --git a/src/test/java/org/tuckey/web/filters/urlrewrite/RuleTest.java b/src/test/java/org/tuckey/web/filters/urlrewrite/RuleTest.java
@@ -78,14 +78,30 @@ public void testRule01a() throws IOException, ServletException, InvocationTarget
         rule.setTo("$1simple");
         rule.setQueryStringAppend("true");
         rule.initialise(null);
-        MockRequest request = new MockRequest("simpleass?aaa=bbb");
+        MockRequest request = new MockRequest("simpleass");
+        request.setQueryString("aaa=bbb");
         NormalRewrittenUrl rewrittenUrl = (NormalRewrittenUrl) rule.matches(request.getRequestURI(), request, response);
 
         assertEquals("forward should be default type", "forward", rule.getToType());
         assertEquals("asssimple?aaa=bbb", rewrittenUrl.getTarget());
         assertTrue("Should be a forward", rewrittenUrl.isForward());
     }
 
+    public void testRule01b() throws IOException, ServletException, InvocationTargetException {
+        NormalRule rule = new NormalRule();
+        rule.setFrom("simple(ass)");
+        rule.setTo("$1simple?d=f");
+        rule.setQueryStringAppend("true");
+        rule.initialise(null);
+        MockRequest request = new MockRequest("simpleass");
+        request.setQueryString("aaa=bbb");
+        NormalRewrittenUrl rewrittenUrl = (NormalRewrittenUrl) rule.matches(request.getRequestURI(), request, response);
+
+        assertEquals("forward should be default type", "forward", rule.getToType());
+        assertEquals("asssimple?d=f&aaa=bbb", rewrittenUrl.getTarget());
+        assertTrue("Should be a forward", rewrittenUrl.isForward());
+    }
+
     public void testRule03() throws IOException, ServletException, InvocationTargetException {
         NormalRule rule = new NormalRule();
         rule.setFrom("^/visa.jsp/([0-9]+)$");
@@ -813,4 +829,18 @@ public void testNotDirectory() throws InvocationTargetException, IOException, Se
         NormalRewrittenUrl rewrittenUrl = (NormalRewrittenUrl) rule.matches(request.getRequestURI(), request, response);
         assertEquals("/not-found-the-dir", rewrittenUrl.getTarget());
     }
+
+    public void testIssue116() throws InvocationTargetException, IOException, ServletException {
+        NormalRule rule = new NormalRule();
+        rule.setFrom("^/robots\\.txt$");
+        rule.setTo("/en/robots.txt");
+        rule.setToType("permanent-redirect");
+        rule.setToLast("true");
+        rule.setQueryStringAppend("true");
+        rule.initialise(new MockServletContext());
+        MockRequest request = new MockRequest("/robots.txt");
+        request.setQueryString("param1=value1&param2=value2");
+        NormalRewrittenUrl rewrittenUrl = (NormalRewrittenUrl) rule.matches(request.getRequestURI(), request, response);
+        assertEquals("/en/robots.txt?param1=value1&param2=value2", rewrittenUrl.getTarget());
+    }
 }