You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2026/10/12 12:59:07 UTC
(tomcat) 20/20: Reject malformed rewrite rule flags instead of accepting typos
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 <re...@apache.org>
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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org