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 2024/03/08 14:46:26 UTC

(tomcat) branch main updated: Add new valveSkip flag, also fix no rewrite rule match

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 <re...@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;
+
     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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: (tomcat) branch main updated: Add new valveSkip flag, also fix no rewrite rule match

Posted by Rémy Maucherat <re...@apache.org>.
On Fri, Mar 8, 2024 at 8:10 PM Christopher Schultz
<ch...@christopherschultz.net> wrote:
>
> Rémy,
>
> On 3/8/24 09:46, remm@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 <re...@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?

Maybe, but I'm not super motivated, the parser and params are a bit
the same philosophy as the mod_rewrite stuff so it's "startsWith" for
everything. It would need more changes because "skip" and
"skipNextValve" override, same for the abbreviated form.

Rémy

> -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-unsubscribe@tomcat.apache.org
> > For additional commands, e-mail: dev-help@tomcat.apache.org
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: (tomcat) branch main updated: Add new valveSkip flag, also fix no rewrite rule match

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Rémy,

On 3/8/24 09:46, remm@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 <re...@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-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org