This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 455463a155ec747e6af293b004f1b061c22478af Author: opencode <[email protected]> AuthorDate: Mon Oct 12 14:17:42 2026 +0200 Bound the rewrite rule loop and reject negative skip values Rules with the next flag [N] restarted the rule loop without any limit. A configuration whose next rules did not converge, such as two rules rewriting back and forth, spun in the loop forever and the request never completed. Likewise, the skip flag accepted negative values, another way to move the rule index backwards. Bound the number of rounds of rules triggered by the next flag, stopping the rewriting with the URL of the last applied rewrite and logging a warning when the bound is reached, and reject negative skip values while parsing the flags. Add tests for both. --- .../valves/rewrite/LocalStrings.properties | 1 + .../catalina/valves/rewrite/RewriteValve.java | 17 ++++++++++ .../catalina/valves/rewrite/TestRewriteValve.java | 36 ++++++++++++++++++++++ 3 files changed, 54 insertions(+) diff --git a/java/org/apache/catalina/valves/rewrite/LocalStrings.properties b/java/org/apache/catalina/valves/rewrite/LocalStrings.properties index 8b09031857..94ca96636c 100644 --- a/java/org/apache/catalina/valves/rewrite/LocalStrings.properties +++ b/java/org/apache/catalina/valves/rewrite/LocalStrings.properties @@ -28,6 +28,7 @@ rewriteValve.closeError=Error closing configuration rewriteValve.invalidFlags=Invalid flag in [{0}] flags [{1}] rewriteValve.invalidLine=Invalid line [{0}] rewriteValve.invalidMapClassName=Invalid map class name [{0}] +rewriteValve.maxRounds=Too many rounds of rewrite rules [{0}] without the rewriting completing. Further rewriting was stopped. rewriteValve.noConfiguration=No configuration resource found [{0}] rewriteValve.readConfiguration=Read configuration from [/WEB-INF/{0}] rewriteValve.readError=Error reading configuration diff --git a/java/org/apache/catalina/valves/rewrite/RewriteValve.java b/java/org/apache/catalina/valves/rewrite/RewriteValve.java index 1f744ff49e..d395d9d119 100644 --- a/java/org/apache/catalina/valves/rewrite/RewriteValve.java +++ b/java/org/apache/catalina/valves/rewrite/RewriteValve.java @@ -69,6 +69,13 @@ public class RewriteValve extends ValveBase { private static final URLEncoder REWRITE_DEFAULT_ENCODER; private static final URLEncoder REWRITE_QUERY_ENCODER; + /** + * The maximum number of rounds of rewrite rules triggered by the next flag [N]. This bounds the processing of + * configurations whose next rules do not converge, which would otherwise spin in the rule loop without ever + * completing the request. + */ + private static final int MAX_REWRITE_ROUNDS = 1000; + static { /* * See the detailed explanation of encoding/decoding during URL re-writing in the invoke() method. @@ -401,6 +408,7 @@ public class RewriteValve extends ValveBase { boolean qsa = false; boolean qsd = false; boolean valveSkip = false; + int rounds = 0; // Step 2. Process the URL using the re-write rules. for (int i = 0; i < rules.length; i++) { @@ -552,6 +560,12 @@ public class RewriteValve extends ValveBase { } // - next (redo again) if (rule.isNext() && newtest != null) { + if (rounds >= MAX_REWRITE_ROUNDS) { + containerLog.warn(sm.getString("rewriteValve.maxRounds", + Integer.valueOf(MAX_REWRITE_ROUNDS))); + break; + } + rounds++; i = -1; continue; } @@ -911,6 +925,9 @@ public class RewriteValve extends ValveBase { flag = flag.substring("S=".length()); } rule.setSkip(Integer.parseInt(flag)); + if (rule.getSkip() < 0) { + throw new IllegalArgumentException(sm.getString("rewriteValve.invalidFlags", line, flag)); + } } else if (flag.startsWith("type") || flag.startsWith("T")) { if (flag.startsWith("type=")) { flag = flag.substring("type=".length()); diff --git a/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java b/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java index 011ea5348c..81eb6f9861 100644 --- a/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java +++ b/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java @@ -290,6 +290,42 @@ public class TestRewriteValve extends TomcatBaseTest { doTestRewrite("RewriteCond %{REQUEST_URI} ^/b.* [NC] extra\n" + "RewriteRule /b/.* /c", "/b/a.html", "/c"); } + @Test(expected = IllegalArgumentException.class) + public void testRewriteRuleNegativeSkip() throws Exception { + doTestRewrite("RewriteRule /b/.* /c [S=-1]", "/b/a.html", "/c"); + } + + @Test(timeout = 30000) + public void testRewriteRuleNextLoopIsBounded() throws Exception { + // Two next rules [N] rewriting back and forth, never reaching a + // state at which rewriting stops by itself + String config = "RewriteRule ^/c/x$ /c/y [N]\n" + "RewriteRule ^/c/y$ /c/x [N]\n"; + + Tomcat tomcat = getTomcatInstance(); + Context ctx = getProgrammaticRootContext(); + + RewriteValve rewriteValve = new RewriteValve(); + ctx.getPipeline().addValve(rewriteValve); + rewriteValve.setConfiguration(config); + + Tomcat.addServlet(ctx, "snoop", new SnoopServlet()); + ctx.addServletMapping("/c/*", "snoop"); + + tomcat.start(); + + ByteChunk res = new ByteChunk(); + int rc = methodUrl("http://localhost:"; + getPort() + "/c/x", res, DEFAULT_CLIENT_TIMEOUT_MS, null, null, + Method.GET, true); + res.setCharset(StandardCharsets.UTF_8); + + // Rewriting must stop after a bounded number of rounds and the + // request must complete, mapped to the servlet thanks to the last + // rewrite that was applied + Assert.assertEquals(200, rc); + String requestURI = SnoopResult.parse(res.toString()).getRequestInfo("REQUEST-URI"); + Assert.assertTrue(requestURI, "/c/x".equals(requestURI) || "/c/y".equals(requestURI)); + } + @Test public void testRewriteEnvVarAndServerVar() throws Exception { System.setProperty("some_variable", "something"); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
