This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 8992dbdd46f397905f378a2eb68c6ef15785911e Author: opencode <[email protected]> AuthorDate: Mon Oct 12 14:53:06 2026 +0200 Reject malformed rewrite rule flags instead of accepting typos parseRuleFlag() matched flags with startsWith(), so a single stray character in a flag was accepted with the full effect of the flag it prefixed: typeX set the response Content-Type to the literal string typeX, redirectX turned the rule into a 302 redirect, and forbiddenX, goneX, hostX, lastX, nocaseX, noescapeX, nextX, qsappendX, qsdiscardX, valveSkipX, F=x and Lx all silently enabled the corresponding flag. A typo that should fail configuration parsing changed the effective behaviour of the rule. Match valueless flags exactly, require the documented name= form for the valued flags skip=|S= and type=|T=, and keep the documented optional value of redirect|R and the value form of host|H. Unknown flags now fall through to the existing invalidFlags exception, as Apache mod_rewrite also refuses unknown flags. Add direct parser and end-to-end configuration tests. --- .../catalina/valves/rewrite/RewriteValve.java | 36 ++++++++++--------- .../catalina/valves/rewrite/TestRewriteValve.java | 42 ++++++++++++++++++++++ 2 files changed, 61 insertions(+), 17 deletions(-) diff --git a/java/org/apache/catalina/valves/rewrite/RewriteValve.java b/java/org/apache/catalina/valves/rewrite/RewriteValve.java index c83db5574f..79418b83e4 100644 --- a/java/org/apache/catalina/valves/rewrite/RewriteValve.java +++ b/java/org/apache/catalina/valves/rewrite/RewriteValve.java @@ -828,9 +828,9 @@ public class RewriteValve extends ValveBase { rule.setChain(true); } else if (flag.startsWith("cookie=") || flag.startsWith("CO=")) { rule.setCookie(true); - if (flag.startsWith("cookie")) { + if (flag.startsWith("cookie=")) { flag = flag.substring("cookie=".length()); - } else if (flag.startsWith("CO=")) { + } else { flag = flag.substring("CO=".length()); } StringTokenizer tokenizer = new StringTokenizer(flag, ":"); @@ -871,27 +871,29 @@ public class RewriteValve extends ValveBase { } rule.addEnvName(flag.substring(0, pos)); rule.addEnvValue(flag.substring(pos + 1)); - } else if (flag.startsWith("forbidden") || flag.startsWith("F")) { + } else if (flag.equals("forbidden") || flag.equals("F")) { rule.setForbidden(true); - } else if (flag.startsWith("gone") || flag.startsWith("G")) { + } else if (flag.equals("gone") || flag.equals("G")) { rule.setGone(true); - } else if (flag.startsWith("host") || flag.startsWith("H")) { + } else if (flag.equals("host") || flag.equals("H") || + flag.startsWith("host=") || flag.startsWith("H=")) { rule.setHost(true); - } else if (flag.startsWith("last") || flag.startsWith("L")) { + } else if (flag.equals("last") || flag.equals("L")) { rule.setLast(true); - } else if (flag.startsWith("nocase") || flag.startsWith("NC")) { + } else if (flag.equals("nocase") || flag.equals("NC")) { rule.setNocase(true); - } else if (flag.startsWith("noescape") || flag.startsWith("NE")) { + } else if (flag.equals("noescape") || flag.equals("NE")) { rule.setNoescape(true); - } else if (flag.startsWith("next") || flag.startsWith("N")) { + } else if (flag.equals("next") || flag.equals("N")) { rule.setNext(true); // Note: Proxy is not supported as Tomcat does not have proxy // capabilities - } else if (flag.startsWith("qsappend") || flag.startsWith("QSA")) { + } else if (flag.equals("qsappend") || flag.equals("QSA")) { rule.setQsappend(true); - } else if (flag.startsWith("qsdiscard") || flag.startsWith("QSD")) { + } else if (flag.equals("qsdiscard") || flag.equals("QSD")) { rule.setQsdiscard(true); - } else if (flag.startsWith("redirect") || flag.startsWith("R")) { + } else if (flag.equals("redirect") || flag.equals("R") || + flag.startsWith("redirect=") || flag.startsWith("R=")) { rule.setRedirect(true); int redirectCode = HttpServletResponse.SC_FOUND; if (flag.startsWith("redirect=") || flag.startsWith("R=")) { @@ -908,25 +910,25 @@ public class RewriteValve extends ValveBase { }; } rule.setRedirectCode(redirectCode); - } else if (flag.startsWith("skip") || flag.startsWith("S")) { + } else if (flag.startsWith("skip=") || flag.startsWith("S=")) { if (flag.startsWith("skip=")) { flag = flag.substring("skip=".length()); - } else if (flag.startsWith("S=")) { + } else { 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")) { + } else if (flag.startsWith("type=") || flag.startsWith("T=")) { if (flag.startsWith("type=")) { flag = flag.substring("type=".length()); - } else if (flag.startsWith("T=")) { + } else { flag = flag.substring("T=".length()); } rule.setType(true); rule.setTypeValue(flag); - } else if (flag.startsWith("valveSkip") || flag.startsWith("VS")) { + } else if (flag.equals("valveSkip") || flag.equals("VS")) { rule.setValveSkip(true); } else { throw new IllegalArgumentException(sm.getString("rewriteValve.invalidFlags", line, flag)); diff --git a/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java b/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java index dcd32c8139..c6a2b74201 100644 --- a/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java +++ b/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java @@ -406,6 +406,48 @@ public class TestRewriteValve extends TomcatBaseTest { requestDesc.getAttribute("X-Test")); } + @Test + public void testMalformedRuleFlagsRejected() throws Exception { + // Typos must fail configuration parsing instead of silently enabling + // the flag whose name the typo extends + String[] malformedFlags = { "forbiddenX", "goneX", "hostX", "Hfoo", "lastX", "nocaseX", "noescapeX", + "nextX", "qsappendX", "qsdiscardX", "redirectX", "R2", "skipX", "typeX", "valveSkipX", "F=x", "Lx", + "VSx" }; + for (String malformedFlag : malformedFlags) { + try { + RewriteValve.parseRuleFlag("RewriteRule ^/c/x$ - [" + malformedFlag + "]", + new RewriteRule(), malformedFlag); + Assert.fail("IAE expected for flag " + malformedFlag); + } catch (IllegalArgumentException e) { + // expected, the flag is unknown + } + } + + // Valid spellings, including the documented value forms, must work + RewriteRule rule = new RewriteRule(); + RewriteValve.parseRuleFlag("line", rule, "redirect"); + Assert.assertTrue(rule.isRedirect()); + RewriteValve.parseRuleFlag("line", rule, "R"); + Assert.assertEquals(HttpServletResponse.SC_FOUND, rule.getRedirectCode()); + RewriteValve.parseRuleFlag("line", rule, "R=302"); + Assert.assertEquals(302, rule.getRedirectCode()); + RewriteValve.parseRuleFlag("line", rule, "host=example.com"); + Assert.assertTrue(rule.isHost()); + RewriteValve.parseRuleFlag("line", rule, "H"); + Assert.assertTrue(rule.isHost()); + RewriteValve.parseRuleFlag("line", rule, "skip=2"); + Assert.assertEquals(2, rule.getSkip()); + RewriteValve.parseRuleFlag("line", rule, "T=text/plain"); + Assert.assertTrue(rule.isType()); + Assert.assertEquals("text/plain", rule.getTypeValue()); + } + + @Test(expected = IllegalArgumentException.class) + public void testMalformedRuleFlagFailsConfiguration() throws Exception { + // A typo in a flag must fail parsing of the whole configuration + doTestRewrite("RewriteRule /b/.* /c [redirectX]", "/b/x.html", "/c"); + } + @Test public void testRewriteMissingCurlyBraceOnVar() throws Exception { try { --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
