Issue #12730 - regex rules have configs for query matching and merging (

#12732)

* Issue #12730 - regex rules have configs for query matching and merging

jetty-core/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RedirectRegexRule.java

@@ -20,6 +20,8 @@
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.server.Response;
 import org.eclipse.jetty.util.Callback;
+import org.eclipse.jetty.util.StringUtil;
+import org.eclipse.jetty.util.URIUtil;
 import org.eclipse.jetty.util.annotation.Name;
 
 /**
@@ -32,6 +34,7 @@ public class RedirectRegexRule extends RegexRule
 {
     protected String _location;
     private int _statusCode = HttpStatus.FOUND_302;
+    private boolean _addQueries = false;
 
     public RedirectRegexRule()
     {
@@ -63,6 +66,31 @@ public void setLocation(String location)
         _location = location;
     }
 
+    /**
+     * <p>Is the input URI query added with replacement URI query</p>
+     *
+     * @return true to add input query with replacement query.
+     */
+    public boolean isAddQueries()
+    {
+        return _addQueries;
+    }
+
+    /**
+     * <p>Set if input query should be preserved, and added together with replacement query</p>
+     *
+     * <p>
+     *     This is especially useful when used in combination with a disabled {@link #setMatchQuery(boolean)}
+     * </p>
+     *
+     * @param flag true to have input query added with replacement query, false (default) to have query
+     *    from input or output just be treated as a string, and not merged.
+     */
+    public void setAddQueries(boolean flag)
+    {
+        _addQueries = flag;
+    }
+
     public int getStatusCode()
     {
         return _statusCode;
@@ -84,6 +112,26 @@ protected Handler apply(Handler input, Matcher matcher) throws IOException
             protected boolean handle(Response response, Callback callback)
             {
                 String target = matcher.replaceAll(getLocation());
+
+                if (isAddQueries() && StringUtil.isNotBlank(input.getHttpURI().getQuery()))
+                {
+                    String inputQuery = input.getHttpURI().getQuery();
+                    String targetPath = null;
+                    String targetQuery = null;
+                    int targetQueryIdx = target.indexOf("?");
+                    if (targetQueryIdx != (-1))
+                    {
+                        targetPath = target.substring(0, targetQueryIdx);
+                        targetQuery = target.substring(targetQueryIdx + 1);
+                    }
+                    else
+                    {
+                        targetPath = target;
+                    }
+                    String resultingQuery = URIUtil.addQueries(inputQuery, targetQuery);
+                    target = targetPath + "?" + resultingQuery;
+                }
+
                 response.setStatus(_statusCode);
                 response.getHeaders().put(HttpHeader.LOCATION, Response.toRedirectURI(this, target));
                 callback.succeeded();

jetty-core/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RegexRule.java

@@ -23,6 +23,7 @@
 public abstract class RegexRule extends Rule
 {
     private Pattern _regex;
+    private boolean _matchQuery = true;
 
     public RegexRule()
     {
@@ -52,10 +53,31 @@ public void setRegex(String regex)
         _regex = regex == null ? null : Pattern.compile(regex);
     }
 
+    /**
+     * <p>Is regex matching against URI path with query section present.</p>
+     *
+     * @return true to match against URI path with query (default), false to match only against URI path.
+     */
+    public boolean isMatchQuery()
+    {
+        return _matchQuery;
+    }
+
+    /**
+     * <p>Enable or disable regex matching against URI path with query section present.</p>
+     *
+     * @param flag true to have regex match against URI path with query, false
+     *   to have match against only URI path.
+     */
+    public void setMatchQuery(boolean flag)
+    {
+        _matchQuery = flag;
+    }
+
     @Override
     public Handler matchAndApply(Handler input) throws IOException
     {
-        String target = input.getHttpURI().getPathQuery();
+        String target = isMatchQuery() ? input.getHttpURI().getPathQuery() : input.getHttpURI().getPath();
         Matcher matcher = _regex.matcher(target);
         if (matcher.matches())
             return apply(input, matcher);

jetty-core/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RewriteRegexRule.java

@@ -17,6 +17,7 @@
 import java.util.regex.Matcher;
 
 import org.eclipse.jetty.http.HttpURI;
+import org.eclipse.jetty.util.URIUtil;
 import org.eclipse.jetty.util.annotation.Name;
 
 /**
@@ -26,6 +27,7 @@
 public class RewriteRegexRule extends RegexRule
 {
     private String replacement;
+    private boolean addQueries = false;
 
     public RewriteRegexRule()
     {
@@ -37,6 +39,31 @@ public RewriteRegexRule(@Name("regex") String regex, @Name("replacement") String
         setReplacement(replacement);
     }
 
+    /**
+     * <p>Is the input URI query added with replacement URI query</p>
+     *
+     * @return true to add input query with replacement query.
+     */
+    public boolean isAddQueries()
+    {
+        return addQueries;
+    }
+
+    /**
+     * <p>Set if input query should be preserved, and added together with replacement query</p>
+     *
+     * <p>
+     *     This is especially useful when used in combination with a disabled {@link #setMatchQuery(boolean)}
+     * </p>
+     *
+     * @param flag true to have input query added with replacement query, false (default) to have query
+     *    from input or output just be treated as a string, and not merged.
+     */
+    public void setAddQueries(boolean flag)
+    {
+        this.addQueries = flag;
+    }
+
     /**
      * Whenever a match is found, it replaces with this value.
      *
@@ -54,6 +81,13 @@ public Handler apply(Handler input, Matcher matcher) throws IOException
         String replacedPath = matcher.replaceAll(replacement);
 
         HttpURI newURI = HttpURI.build(httpURI, replacedPath);
+        if (isAddQueries())
+        {
+            String inputQuery = input.getHttpURI().getQuery();
+            String targetQuery = newURI.getQuery();
+            String resultingQuery = URIUtil.addQueries(inputQuery, targetQuery);
+            newURI = HttpURI.build(newURI).query(resultingQuery);
+        }
         return new HttpURIHandler(input, newURI);
     }
 

jetty-core/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/RedirectRegexRuleTest.java

@@ -51,7 +51,7 @@ public void testLocationWithReplacementGroupEmpty() throws Exception
         String request = """
             GET /my/dir/file/ HTTP/1.1
             Host: localhost
-                        
+            
             """;
 
         HttpTester.Response response = HttpTester.parseResponse(_connector.getResponse(request));
@@ -68,7 +68,7 @@ public void testLocationWithPathReplacement() throws Exception
         String request = """
             GET /documentation/top.html HTTP/1.1
             Host: localhost
-                        
+            
             """;
 
         HttpTester.Response response = HttpTester.parseResponse(_connector.getResponse(request));
@@ -85,7 +85,7 @@ public void testLocationWithReplacementGroupSimple() throws Exception
         String request = """
             GET /my/dir/file/image.png HTTP/1.1
             Host: localhost
-                        
+            
             """;
 
         HttpTester.Response response = HttpTester.parseResponse(_connector.getResponse(request));
@@ -103,14 +103,54 @@ public void testLocationWithReplacementGroupWithQuery() throws Exception
         String request = """
             GET /my/dir/file/api/rest/foo?id=100&sort=date HTTP/1.1
             Host: localhost
-                        
+            
             """;
 
         HttpTester.Response response = HttpTester.parseResponse(_connector.getResponse(request));
         assertEquals(HttpStatus.MOVED_PERMANENTLY_301, response.getStatus());
         assertEquals("http://www.mortbay.org/api/rest/foo?id=100&sort=date", response.get(HttpHeader.LOCATION));
     }
 
+    @Test
+    public void testMatchOnlyPathLocationWithoutQuery() throws Exception
+    {
+        RedirectRegexRule rule = new RedirectRegexRule("/my/dir/file/(.*)/foo$", "http://www.mortbay.org/$1/foo");
+        rule.setMatchQuery(false);
+        rule.setStatusCode(HttpStatus.MOVED_PERMANENTLY_301);
+        start(rule);
+
+        String request = """
+            GET /my/dir/file/api/rest/foo?id=100&sort=date HTTP/1.1
+            Host: localhost
+            
+            """;
+
+        HttpTester.Response response = HttpTester.parseResponse(_connector.getResponse(request));
+        assertEquals(HttpStatus.MOVED_PERMANENTLY_301, response.getStatus());
+        // This configuration, with RedirectRegexRule.isAddQueries(false), will drop the input query section
+        assertEquals("http://www.mortbay.org/api/rest/foo", response.get(HttpHeader.LOCATION));
+    }
+
+    @Test
+    public void testMatchOnlyPathLocationWithQueryAddQueries() throws Exception
+    {
+        RedirectRegexRule rule = new RedirectRegexRule("/my/dir/file/(.*)/foo$", "http://www.mortbay.org/$1/foo?v=old");
+        rule.setMatchQuery(false);
+        rule.setAddQueries(true);
+        rule.setStatusCode(HttpStatus.MOVED_PERMANENTLY_301);
+        start(rule);
+
+        String request = """
+            GET /my/dir/file/api/rest/foo?id=100&sort=date HTTP/1.1
+            Host: localhost
+            
+            """;
+
+        HttpTester.Response response = HttpTester.parseResponse(_connector.getResponse(request));
+        assertEquals(HttpStatus.MOVED_PERMANENTLY_301, response.getStatus());
+        assertEquals("http://www.mortbay.org/api/rest/foo?id=100&sort=date&v=old", response.get(HttpHeader.LOCATION));
+    }
+
     @Test
     public void testEncodedNewLineInURI() throws Exception
     {
@@ -120,7 +160,7 @@ public void testEncodedNewLineInURI() throws Exception
         String request = """
             GET /%0A.evil.com HTTP/1.1
             Host: localhost
-                        
+            
             """;
 
         HttpTester.Response response = HttpTester.parseResponse(_connector.getResponse(request));

jetty-core/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/RewriteRegexRuleTest.java

@@ -95,7 +95,7 @@ public void testRequestUriEnabled(Scenario scenario) throws Exception
         String request = """
             GET $T HTTP/1.1
             Host: localhost
-                        
+            
             """.replace("$T", scenario.pathQuery);
 
         HttpTester.Response response = HttpTester.parseResponse(_connector.getResponse(request));
@@ -129,7 +129,7 @@ public void testRegexOptionalTargetQuery(String target, String expectedResult) t
         String request = """
             GET $T HTTP/1.1
             Host: localhost
-                        
+            
             """.replace("$T", target);
 
         HttpTester.Response response = HttpTester.parseResponse(_connector.getResponse(request));
@@ -142,7 +142,46 @@ public void testRegexOptionalTargetQuery(String target, String expectedResult) t
         assertThat(result, is(expectedResult));
     }
 
-    private record Scenario(String pathQuery, String regex, String replacement, String expectedPath, String expectedQuery)
+    public static Stream<Arguments> matchPathOnlyWithQueries()
+    {
+        return Stream.of(
+            Arguments.of("/foo/bar", "/test?p2=bar&p1=foo"),
+            Arguments.of("/foo/bar/", "/test?p2=bar&p1=foo"),
+            Arguments.of("/foo/bar?", "/test?p2=bar&p1=foo"),
+            Arguments.of("/foo/bar/?", "/test?p2=bar&p1=foo"),
+            Arguments.of("/foo/bar?a=b", "/test?a=b&p2=bar&p1=foo"),
+            Arguments.of("/foo/bar/?a=b", "/test?a=b&p2=bar&p1=foo")
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource("matchPathOnlyWithQueries")
+    public void testMatchOnlyOnPathAddQueries(String target, String expectedResult) throws Exception
+    {
+        String regex = "^/([^/]*)/([^/]*)/?.*$";
+        String replacement = "/test?p2=$2&p1=$1";
+        RewriteRegexRule rule = new RewriteRegexRule(regex, replacement);
+        rule.setMatchQuery(false);
+        rule.setAddQueries(true);
+        start(rule);
+
+        String request = """
+            GET $T HTTP/1.1
+            Host: localhost
+            
+            """.replace("$T", target);
+
+        HttpTester.Response response = HttpTester.parseResponse(_connector.getResponse(request));
+        assertEquals(HttpStatus.OK_200, response.getStatus(), "Response status code");
+        String result = response.get("X-Path");
+        String query = response.get("X-Query");
+        if (StringUtil.isNotBlank(query))
+            result = result + '?' + query;
+
+        assertThat(result, is(expectedResult));
+    }
+
+    public record Scenario(String pathQuery, String regex, String replacement, String expectedPath, String expectedQuery)
     {
     }
 }