You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by kk...@apache.org on 2011/11/08 08:43:58 UTC

svn commit: r1199139 - in /tomcat/tc6.0.x/trunk: STATUS.txt java/org/apache/catalina/valves/LocalStrings.properties java/org/apache/catalina/valves/RequestFilterValve.java webapps/docs/changelog.xml

Author: kkolinko
Date: Tue Nov  8 07:43:58 2011
New Revision: 1199139

URL: http://svn.apache.org/viewvc?rev=1199139&view=rev
Log:
Make configuration issue for RemoteAddrValve, RemoteHostValve result
in the failure of the valve rather than just a warning message.
Ensure changes to the configuration of these valves via JMX are thread-safe.

Modified:
    tomcat/tc6.0.x/trunk/STATUS.txt
    tomcat/tc6.0.x/trunk/java/org/apache/catalina/valves/LocalStrings.properties
    tomcat/tc6.0.x/trunk/java/org/apache/catalina/valves/RequestFilterValve.java
    tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1199139&r1=1199138&r2=1199139&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Tue Nov  8 07:43:58 2011
@@ -64,15 +64,6 @@ PATCHES PROPOSED TO BACKPORT:
       - getStuckThreadIds() returns a list of ids. It might be useful to
         have a similar method that returns Thread.getName() names.
 
-* Make configuration issue for RemoteAddrValve, RemoteHostValve result
-  in the failure of the valve rather than just a warning message.
-  Ensure changes to the configuration of the RemoteHostValve and the
-  RemoteIpValve via JMX are thread-safe.
-  http://people.apache.org/~kkolinko/patches/2011-10-26_tc6_RequestFilterValve_v3.patch
-  It is based on valves part of r1189256 and r1187027. (r1189258, r1187029 in TC7)
-  +1: kkolinko, kfujino, markt
-  -1:
-
 * Improve performance of parameter processing.
   http://people.apache.org/~markt/patches/2011-10-29-param-perf-tc6-v2.patch
   http://svn.apache.org/viewvc?rev=1195222&view=rev - performance tweaks

Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/valves/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/valves/LocalStrings.properties?rev=1199139&r1=1199138&r2=1199139&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/valves/LocalStrings.properties (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/valves/LocalStrings.properties Tue Nov  8 07:43:58 2011
@@ -19,14 +19,17 @@ certificatesValve.alreadyStarted=Certifi
 certificatesValve.notStarted=Certificates Valve has not yet been started
 interceptorValve.alreadyStarted=Interceptor Valve has already been started
 interceptorValve.notStarted=Interceptor Valve has not yet been started
-requestFilterValve.next=No ''next'' valve has been configured
-requestFilterValve.syntax=Syntax error in request filter pattern {0}
 valveBase.noNext=Configuration error: No ''next'' valve configured
 jdbcAccessLogValve.exception=Exception performing insert access entry
 jdbcAccessLogValve.close=Exception closing database connection
 cometConnectionManagerValve.event=Exception processing event
 cometConnectionManagerValve.listenerEvent=Exception processing session listener event
 
+# Request filter valve - RemoteAddrValve, RemoteHostValve
+requestFilterValve.alreadyStarted=Valve has already been started
+requestFilterValve.syntax=Syntax error in request filter pattern {0}
+requestFilterValve.configInvalid=One or more invalid configuration settings were provided for the Remote[Addr|Host]Valve. The valve will deny all requests.
+
 # Access log valve
 accessLogValve.alreadyStarted=Access Logger has already been started
 accessLogValve.notStarted=Access Logger has not yet been started

Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/valves/RequestFilterValve.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/valves/RequestFilterValve.java?rev=1199139&r1=1199138&r2=1199139&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/valves/RequestFilterValve.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/valves/RequestFilterValve.java Tue Nov  8 07:43:58 2011
@@ -27,8 +27,12 @@ import java.util.regex.PatternSyntaxExce
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletResponse;
 
+import org.apache.catalina.Lifecycle;
+import org.apache.catalina.LifecycleException;
+import org.apache.catalina.LifecycleListener;
 import org.apache.catalina.connector.Request;
 import org.apache.catalina.connector.Response;
+import org.apache.catalina.util.LifecycleSupport;
 import org.apache.catalina.util.StringManager;
 
 /**
@@ -66,7 +70,7 @@ import org.apache.catalina.util.StringMa
  */
 
 public abstract class RequestFilterValve
-    extends ValveBase {
+    extends ValveBase implements Lifecycle {
 
 
     // ----------------------------------------------------- Class Variables
@@ -92,26 +96,51 @@ public abstract class RequestFilterValve
     /**
      * The comma-delimited set of <code>allow</code> expressions.
      */
-    protected String allow = null;
+    protected volatile String allow = null;
+
+    /**
+     * Helper variable to catch configuration errors.
+     * It is <code>true</code> by default, but becomes <code>false</code>
+     * if there was an attempt to assign an invalid value to the
+     * <code>allow</code> pattern.
+     */
+    protected volatile boolean allowValid = true;
 
 
     /**
      * The set of <code>allow</code> regular expressions we will evaluate.
      */
-    protected Pattern allows[] = new Pattern[0];
+    protected volatile Pattern allows[] = new Pattern[0];
 
 
     /**
      * The set of <code>deny</code> regular expressions we will evaluate.
      */
-    protected Pattern denies[] = new Pattern[0];
+    protected volatile Pattern denies[] = new Pattern[0];
 
 
     /**
      * The comma-delimited set of <code>deny</code> expressions.
      */
-    protected String deny = null;
+    protected volatile String deny = null;
 
+    /**
+     * Helper variable to catch configuration errors.
+     * It is <code>true</code> by default, but becomes <code>false</code>
+     * if there was an attempt to assign an invalid value to the
+     * <code>deny</code> pattern.
+     */
+    protected volatile boolean denyValid = true;
+
+    /**
+     * The lifecycle event support for this component.
+     */
+    protected LifecycleSupport lifecycle = new LifecycleSupport(this);
+
+    /**
+     * Has this component been started yet?
+     */
+    protected boolean started = false;
 
     // ------------------------------------------------------------- Properties
 
@@ -134,10 +163,14 @@ public abstract class RequestFilterValve
      * @param allow The new set of allow expressions
      */
     public void setAllow(String allow) {
-
-        this.allow = allow;
-        allows = precalculate(allow);
-
+        boolean success = false;
+        try {
+            allows = precalculate(allow);
+            success = true;
+            this.allow = allow;
+        } finally {
+            allowValid = success;
+        }
     }
 
 
@@ -159,10 +192,14 @@ public abstract class RequestFilterValve
      * @param deny The new set of deny expressions
      */
     public void setDeny(String deny) {
-
-        this.deny = deny;
-        denies = precalculate(deny);
-
+        boolean success = false;
+        try {
+            denies = precalculate(deny);
+            success = true;
+            this.deny = deny;
+        } finally {
+            denyValid = success;
+        }
     }
 
 
@@ -255,6 +292,10 @@ public abstract class RequestFilterValve
                            Request request, Response response)
         throws IOException, ServletException {
 
+        // Use local copies for thread safety
+        Pattern[] denies = this.denies;
+        Pattern[] allows = this.allows;
+
         // Check the deny patterns, if any
         for (int i = 0; i < denies.length; i++) {
             if (denies[i].matcher(property).matches()) {
@@ -283,4 +324,76 @@ public abstract class RequestFilterValve
     }
 
 
+    // ------------------------------------------------------ Lifecycle Methods
+
+
+    /**
+     * Add a lifecycle event listener to this component.
+     *
+     * @param listener The listener to add
+     */
+    public void addLifecycleListener(LifecycleListener listener) {
+        lifecycle.addLifecycleListener(listener);
+    }
+
+
+    /**
+     * Get the lifecycle listeners associated with this lifecycle. If this
+     * Lifecycle has no listeners registered, a zero-length array is returned.
+     */
+    public LifecycleListener[] findLifecycleListeners() {
+        return lifecycle.findLifecycleListeners();
+    }
+
+
+    /**
+     * Remove a lifecycle event listener from this component.
+     *
+     * @param listener The listener to add
+     */
+    public void removeLifecycleListener(LifecycleListener listener) {
+        lifecycle.removeLifecycleListener(listener);
+    }
+
+
+    /**
+     * Prepare for the beginning of active use of the public methods of this
+     * component.  This method should be called after <code>configure()</code>,
+     * and before any of the public methods of the component are utilized.
+     *
+     * @exception LifecycleException if this component detects a fatal error
+     *  that prevents this component from being used
+     */
+    public void start() throws LifecycleException {
+
+        // Validate and update our current component state
+        if (started) {
+            throw new LifecycleException(
+                    sm.getString("requestFilterValve.alreadyStarted"));
+        }
+        if (!allowValid || !denyValid) {
+            throw new LifecycleException(
+                    sm.getString("requestFilterValve.configInvalid"));
+        }
+        lifecycle.fireLifecycleEvent(START_EVENT, null);
+        started = true;
+    }
+
+    /**
+     * Gracefully terminate the active use of the public methods of this
+     * component.  This method should be the last one called on a given
+     * instance of this component.
+     *
+     * @exception LifecycleException if this component detects a fatal error
+     *  that needs to be reported
+     */
+    public void stop() throws LifecycleException {
+        // Validate and update our current component state
+        if (!started) {
+            return;
+        }
+        lifecycle.fireLifecycleEvent(STOP_EVENT, null);
+        started = false;
+    }
+
 }

Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=1199139&r1=1199138&r2=1199139&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Tue Nov  8 07:43:58 2011
@@ -102,6 +102,15 @@
         Make configuration issue for CsrfPreventionFilter result in the
         failure of the filter rather than just a warning message. (kkolinko)
       </add>
+      <fix>
+        Ensure changes to the configuration of RemoteAddrValve and
+        RemoteHostValve via JMX are thread-safe. (kkolinko)
+      </fix>
+      <add>
+        Make configuration issue for RemoteAddrValve and
+        RemoteHostValve result in the failure of the valve rather than
+        just a warning message. (kkolinko)
+      </add>
     </changelog>
   </subsection>
   <subsection name="Coyote">



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