Rémy,
On 3/8/24 09:46, r...@apache.org wrote:
This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 7a3bbc6e30 Add new valveSkip flag, also fix no rewrite rule match
7a3bbc6e30 is described below
commit 7a3bbc6e300ced35268fe1c46c90f6b5c752dc5c
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 e4466d525b..269eedab7f 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;
+
Maybe "skipNextValve"? "valveSkip" isn't super descriptive.
Same with accessor/mutator.
Same for ValveSkip/VS flag name? Maybe SNV? SkipNextValve?
-chris
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 492d45e26d..d15210621d 100644
--- a/java/org/apache/catalina/valves/rewrite/RewriteValve.java
+++ b/java/org/apache/catalina/valves/rewrite/RewriteValve.java
@@ -38,6 +38,7 @@ import org.apache.catalina.Context;
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;
@@ -319,11 +320,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());
@@ -345,6 +347,10 @@ public class RewriteValve extends ValveBase {
qsd = true;
}
+ if (!valveSkip && newtest != null && rule.isValveSkip()) {
+ valveSkip = true;
+ }
+
// Final reply
// - forbidden
@@ -543,7 +549,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 {
@@ -795,6 +809,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 8702891ac3..464a960384 100644
--- a/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
+++ b/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
@@ -70,6 +70,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");
@@ -730,6 +735,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();
@@ -738,6 +748,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 bba9fe5235..19aab1f4eb 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -150,6 +150,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 59c2d08624..72fc813c91 100644
--- a/webapps/docs/rewrite.xml
+++ b/webapps/docs/rewrite.xml
@@ -802,6 +802,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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
