From 575b9ccf9e6f46ecea19e9c6ca4118bf7900706e Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Fri, 24 Jan 2025 14:05:44 -0600 Subject: [PATCH] Issue #12730 - regex rules have configs for query matching and merging (#12732) * Issue #12730 - regex rules have configs for query matching and merging --- .../rewrite/handler/RedirectRegexRule.java | 48 ++++++++++++++++++ .../jetty/rewrite/handler/RegexRule.java | 24 ++++++++- .../rewrite/handler/RewriteRegexRule.java | 34 +++++++++++++ .../handler/RedirectRegexRuleTest.java | 50 +++++++++++++++++-- .../rewrite/handler/RewriteRegexRuleTest.java | 45 +++++++++++++++-- 5 files changed, 192 insertions(+), 9 deletions(-) diff --git a/jetty-core/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RedirectRegexRule.java b/jetty-core/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RedirectRegexRule.java index ce552c5867fe..54f9fac9f609 100644 --- a/jetty-core/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RedirectRegexRule.java +++ b/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; } + /** + *

Is the input URI query added with replacement URI query

+ * + * @return true to add input query with replacement query. + */ + public boolean isAddQueries() + { + return _addQueries; + } + + /** + *

Set if input query should be preserved, and added together with replacement query

+ * + *

+ * This is especially useful when used in combination with a disabled {@link #setMatchQuery(boolean)} + *

+ * + * @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(); diff --git a/jetty-core/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RegexRule.java b/jetty-core/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RegexRule.java index 386da9afe8de..b07863bd73f1 100644 --- a/jetty-core/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RegexRule.java +++ b/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); } + /** + *

Is regex matching against URI path with query section present.

+ * + * @return true to match against URI path with query (default), false to match only against URI path. + */ + public boolean isMatchQuery() + { + return _matchQuery; + } + + /** + *

Enable or disable regex matching against URI path with query section present.

+ * + * @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); diff --git a/jetty-core/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RewriteRegexRule.java b/jetty-core/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RewriteRegexRule.java index e63275cdaa9b..185cdd62e10c 100644 --- a/jetty-core/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RewriteRegexRule.java +++ b/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); } + /** + *

Is the input URI query added with replacement URI query

+ * + * @return true to add input query with replacement query. + */ + public boolean isAddQueries() + { + return addQueries; + } + + /** + *

Set if input query should be preserved, and added together with replacement query

+ * + *

+ * This is especially useful when used in combination with a disabled {@link #setMatchQuery(boolean)} + *

+ * + * @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); } diff --git a/jetty-core/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/RedirectRegexRuleTest.java b/jetty-core/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/RedirectRegexRuleTest.java index 677f90567e4f..28fd01c82dff 100644 --- a/jetty-core/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/RedirectRegexRuleTest.java +++ b/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,7 +103,7 @@ 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)); @@ -111,6 +111,46 @@ public void testLocationWithReplacementGroupWithQuery() throws Exception 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)); diff --git a/jetty-core/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/RewriteRegexRuleTest.java b/jetty-core/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/RewriteRegexRuleTest.java index a583a6a81110..45f735e7f1f5 100644 --- a/jetty-core/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/RewriteRegexRuleTest.java +++ b/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 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) { } }