tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Konstantin Kolinko <knst.koli...@gmail.com>
Subject Re: svn commit: r1197310 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/valves/RequestFilterValve.java
Date Thu, 03 Nov 2011 21:51:16 GMT
2011/11/4  <markt@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message