2011/11/4  <ma...@apache.org>:
> Author: markt
> Date: Thu Nov  3 21:10:04 2011
> New Revision: 1197310
>
> URL: http://svn.apache.org/viewvc?rev=1197310&view=rev
> Log:
> Ensure that subsequent attempts to start the Valves will not succeed until 
> valid configuration is provided.
>
> Modified:
>    tomcat/tc7.0.x/trunk/   (props changed)
>    
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/valves/RequestFilterValve.java
>
> Modified: 
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/valves/RequestFilterValve.java
> URL: 
> http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/valves/RequestFilterValve.java?rev=1197310&r1=1197309&r2=1197310&view=diff
> ==============================================================================
> --- 
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/valves/RequestFilterValve.java 
> (original)
> +++ 
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/valves/RequestFilterValve.java 
> Thu Nov  3 21:10:04 2011
> @@ -83,6 +83,14 @@ public abstract class RequestFilterValve
>      */
>     protected volatile Pattern allow = null;
>
> +
> +    /**
> +     * The current allow configuration value that may or may not compile 
> into a
> +     * valid {@link Pattern}.
> +     */
> +    protected volatile String allowValue = null;
> +
> +
>     /**
>      * Helper variable to catch configuration errors.
>      * It is <code>true</code> by default, but becomes <code>false</code>
> @@ -97,6 +105,14 @@ public abstract class RequestFilterValve
>      */
>     protected volatile Pattern deny = null;
>
> +
> +    /**
> +     * The current deny configuration value that may or may not compile into 
> a
> +     * valid {@link Pattern}.
> +     */
> +    protected volatile String denyValue = null;
> +
> +
>     /**
>      * Helper variable to catch configuration errors.
>      * It is <code>true</code> by default, but becomes <code>false</code>
> @@ -114,12 +130,7 @@ public abstract class RequestFilterValve
>      * Valve, if any; otherwise, return <code>null</code>.
>      */
>     public String getAllow() {
> -        // Use local copies for thread safety
> -        Pattern allow = this.allow;
> -        if (allow == null) {
> -            return null;
> -        }
> -        return allow.toString();
> +        return allowValue;
>     }
>
>
> @@ -132,10 +143,12 @@ public abstract class RequestFilterValve
>     public void setAllow(String allow) {
>         if (allow == null || allow.length() == 0) {
>             this.allow = null;
> +            allowValue = null;
>             allowValid = true;
>         } else {
>             boolean success = false;
>             try {
> +                allowValue = allow;
>                 this.allow = Pattern.compile(allow);
>                 success = true;
>             } finally {
> @@ -150,12 +163,7 @@ public abstract class RequestFilterValve
>      * Valve, if any; otherwise, return <code>null</code>.
>      */
>     public String getDeny() {
> -        // Use local copies for thread safety
> -        Pattern deny = this.deny;
> -        if (deny == null) {
> -            return null;
> -        }
> -        return deny.toString();
> +        return denyValue;
>     }
>
>
> @@ -168,10 +176,12 @@ public abstract class RequestFilterValve
>     public void setDeny(String deny) {
>         if (deny == null || deny.length() == 0) {
>             this.deny = null;
> +            denyValue = null;
>             denyValid = true;
>         } else {
>             boolean success = false;
>             try {
> +                denyValue = deny;
>                 this.deny = Pattern.compile(deny);
>                 success = true;
>             } finally {
> @@ -225,6 +235,16 @@ public abstract class RequestFilterValve
>     }
>
>
> +    @Override
> +    protected synchronized void startInternal() throws LifecycleException {
> +        if (!allowValid || !denyValid) {
> +            throw new LifecycleException(
> +                    sm.getString("requestFilterValve.configInvalid"));
> +        }
> +        super.startInternal();
> +    }
> +
> +
>     /**
>      * Perform the filtering that has been configured for this Valve, matching
>      * against the specified request property.

Good.

The 5.5 and 6.0 patches should have this feature already, because they
do not have separate init method and do it in start() instead.

The only difference is that you assign the string values before
calling Pattern.compile(), while in 5.5/6.0 I do assignment only if
compilation is successful.

I think that this your assignment behaviour is better and I could
update the 5.5/6.0 patches to align with it.

Best regards,
Konstantin Kolinko

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

Reply via email to