This is an automated email from the ASF dual-hosted git repository. remm pushed a commit to branch 8.5.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push: new 15a1d5b5d4 Add new valveSkip flag, also fix no rewrite rule match 15a1d5b5d4 is described below commit 15a1d5b5d4dc86ff99fc82fefc311c8c120ce439 Author: remm <r...@apache.org> AuthorDate: Fri Mar 8 15:46:04 2024 +0100 Add new valveSkip flag, also fix no rewrite rule match This allows skipping the next valve in the Catalina pipeline, for conditional execution. This was inspired by BZ 60997, which the rewrite valve can do (set the rewrite valve, then set the semaphore valve next; if match then the semaphore is skipped). Also notice major issue where rewrite is considered to have occurred when the output is identical but equals return false due to type differences. --- .../apache/catalina/valves/rewrite/RewriteRule.java | 14 ++++++++++++++ .../apache/catalina/valves/rewrite/RewriteValve.java | 20 ++++++++++++++++++-- .../catalina/valves/rewrite/TestRewriteValve.java | 18 ++++++++++++++++++ webapps/docs/changelog.xml | 9 +++++++++ webapps/docs/rewrite.xml | 11 +++++++++++ 5 files changed, 70 insertions(+), 2 deletions(-) diff --git a/java/org/apache/catalina/valves/rewrite/RewriteRule.java b/java/org/apache/catalina/valves/rewrite/RewriteRule.java index 66b3be839c..848bd3aecb 100644 --- a/java/org/apache/catalina/valves/rewrite/RewriteRule.java +++ b/java/org/apache/catalina/valves/rewrite/RewriteRule.java @@ -335,6 +335,11 @@ public class RewriteRule { protected boolean type = false; protected String typeValue = null; + /** + * Allows skipping the next valve in the Catalina pipeline. + */ + protected boolean valveSkip = false; + public boolean isEscapeBackReferences() { return escapeBackReferences; } @@ -560,4 +565,13 @@ public class RewriteRule { public void setCookieHttpOnly(boolean cookieHttpOnly) { this.cookieHttpOnly = cookieHttpOnly; } + + public boolean isValveSkip() { + return this.valveSkip; + } + + public void setValveSkip(boolean valveSkip) { + this.valveSkip = valveSkip; + } + } diff --git a/java/org/apache/catalina/valves/rewrite/RewriteValve.java b/java/org/apache/catalina/valves/rewrite/RewriteValve.java index 1277397483..39d09e48be 100644 --- a/java/org/apache/catalina/valves/rewrite/RewriteValve.java +++ b/java/org/apache/catalina/valves/rewrite/RewriteValve.java @@ -42,6 +42,7 @@ import org.apache.catalina.Host; import org.apache.catalina.Lifecycle; import org.apache.catalina.LifecycleException; import org.apache.catalina.Pipeline; +import org.apache.catalina.Valve; import org.apache.catalina.connector.Connector; import org.apache.catalina.connector.Request; import org.apache.catalina.connector.Response; @@ -335,11 +336,12 @@ public class RewriteValve extends ValveBase { boolean done = false; boolean qsa = false; boolean qsd = false; + boolean valveSkip = false; for (int i = 0; i < rules.length; i++) { RewriteRule rule = rules[i]; CharSequence test = (rule.isHost()) ? host : urlDecoded; CharSequence newtest = rule.evaluate(test, resolver); - if (newtest != null && !test.equals(newtest.toString())) { + if (newtest != null && !test.toString().equals(newtest.toString())) { if (containerLog.isTraceEnabled()) { containerLog.trace("Rewrote " + test + " as " + newtest + " with rule pattern " + rule.getPatternString()); @@ -361,6 +363,10 @@ public class RewriteValve extends ValveBase { qsd = true; } + if (!valveSkip && newtest != null && rule.isValveSkip()) { + valveSkip = true; + } + // Final reply // - forbidden @@ -559,7 +565,15 @@ public class RewriteValve extends ValveBase { pipeline.getFirst().invoke(request, response); } } else { - getNext().invoke(request, response); + Valve next = getNext(); + if (valveSkip) { + next = next.getNext(); + if (next == null) { + // Ignore and invoke the next valve normally + next = getNext(); + } + } + next.invoke(request, response); } } finally { @@ -851,6 +865,8 @@ public class RewriteValve extends ValveBase { } rule.setType(true); rule.setTypeValue(flag); + } else if (flag.startsWith("valveSkip") || flag.startsWith("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 ddc99a750e..7e4962770e 100644 --- a/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java +++ b/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java @@ -71,6 +71,11 @@ public class TestRewriteValve extends TomcatBaseTest { doTestRewrite("RewriteRule ^(.*) $1", "/a/%255A", "/a/%255A"); } + @Test + public void testNoopValveSkipRewrite() throws Exception { + doTestRewrite("RewriteRule ^(.*) $1 [VS]", "/a/%255A", "/a/%255A", null, null, true); + } + @Test public void testPathRewrite() throws Exception { doTestRewrite("RewriteRule ^/b(.*) /a$1", "/b/%255A", "/a/%255A"); @@ -725,6 +730,11 @@ public class TestRewriteValve extends TomcatBaseTest { private void doTestRewrite(String config, String request, String expectedURI, String expectedQueryString, String expectedAttributeValue) throws Exception { + doTestRewrite(config, request, expectedURI, expectedQueryString, expectedAttributeValue, false); + } + + private void doTestRewrite(String config, String request, String expectedURI, String expectedQueryString, + String expectedAttributeValue, boolean valveSkip) throws Exception { Tomcat tomcat = getTomcatInstance(); @@ -733,6 +743,14 @@ public class TestRewriteValve extends TomcatBaseTest { RewriteValve rewriteValve = new RewriteValve(); ctx.getPipeline().addValve(rewriteValve); + if (valveSkip) { + ctx.getPipeline().addValve(new ValveBase() { + @Override + public void invoke(Request request, Response response) throws IOException, ServletException { + throw new IllegalStateException(); + } + }); + } rewriteValve.setConfiguration(config); diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 5c2e64d663..f5ca224806 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -125,6 +125,15 @@ transformation of a class also triggers the loading of the same class. (markt) </fix> + <fix> + The rewrite valve should not do a rewrite if the output is identical + to the input. (remm) + </fix> + <update> + Add a new <code>valveSkip</code> (or <code>VS</code>) rule flag to the + rewrite valve to allow skipping over the next valve in the Catalina + pipeline. (remm) + </update> </changelog> </subsection> <subsection name="Coyote"> diff --git a/webapps/docs/rewrite.xml b/webapps/docs/rewrite.xml index 038e671eef..3f9905e467 100644 --- a/webapps/docs/rewrite.xml +++ b/webapps/docs/rewrite.xml @@ -750,6 +750,17 @@ cannot use <code>$N</code> in the substitution string! the <code>.phps</code> extension: <source>RewriteRule ^(.+\.php)s$ $1 [T=application/x-httpd-php-source]</source> </li> + + <li>'<strong><code>valveSkip|VS</code></strong>' + (skip valve)<br /> + This flag can be used to setup conditional execution of valves. + When the flag is set and the rule matches, the rewrite valve will skip + the next valve in the Catalina pipeline. If the rewrite valve is the + last of the pipeline, then the flag will be ignored and the container + basic valve will be invoked. If rewrite occurred, then the flag will + not have any effect. + </li> + </ul> </subsection> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org