Author: jim
Date: Tue Dec 20 14:25:24 2011
New Revision: 1221276
URL: http://svn.apache.org/viewvc?rev=1221276&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.
Refactor value matching logic into separate method.
Expose the new method isAllowed and isAllowValid, isDenyValid
properties through JMX.
It is based on r1189256 and r1187027, r1198622
(r1189258, r1187029, r1198623 in TC7)
http://people.apache.org/~kkolinko/patches/2011-11-08_tc55_RequestFilterValve_v4.patch
+1: kkolinko,funkman,jim
-1:
kkolinko: It does its work and prevents app from starting
and working. Though
1. Autodeployment prints the same error every 10s. It
is OK, though a
bit annoying.
2. Application that failed to start responds
with 403. I do not
understand why. I would expect 404 or
503.
3. Application that failed to
start is not listed by the manager app.
It is expected, but does
not explain why error 403 and not 404 is observed.
Modified:
tomcat/tc5.5.x/trunk/STATUS.txt
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/LocalStrings.properties
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/RequestFilterValve.java
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/mbeans-descriptors.xml
Modified: tomcat/tc5.5.x/trunk/STATUS.txt
URL:
http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/STATUS.txt?rev=1221276&r1=1221275&r2=1221276&view=diff
==============================================================================
--- tomcat/tc5.5.x/trunk/STATUS.txt (original)
+++ tomcat/tc5.5.x/trunk/STATUS.txt Tue Dec 20 14:25:24 2011
@@ -24,25 +24,6 @@ $Id$
PATCHES ACCEPTED TO BACKPORT FROM TRUNK/OTHER:
[ start all new proposals below, under PATCHES PROPOSED. ]
-* 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.
- Refactor value matching logic into separate method.
- Expose the new method isAllowed and isAllowValid, isDenyValid properties
through JMX.
- It is based on r1189256 and r1187027, r1198622
- (r1189258, r1187029, r1198623 in TC7)
-
http://people.apache.org/~kkolinko/patches/2011-11-08_tc55_RequestFilterValve_v4.patch
- +1: kkolinko,funkman,jim
- -1:
-
- kkolinko: It does its work and prevents app from starting and working. Though
- 1. Autodeployment prints the same error every 10s. It is OK, though a
- bit annoying.
- 2. Application that failed to start responds with 403. I do not
- understand why. I would expect 404 or 503.
- 3. Application that failed to start is not listed by the manager app.
- It is expected, but does not explain why error 403 and not 404 is
observed.
-
* Improve performance of parameter processing
<add>
Improve performance of parameter processing for GET and POST requests.
Modified:
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/LocalStrings.properties
URL:
http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/LocalStrings.properties?rev=1221276&r1=1221275&r2=1221276&view=diff
==============================================================================
---
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/LocalStrings.properties
(original)
+++
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/LocalStrings.properties
Tue Dec 20 14:25:24 2011
@@ -21,12 +21,15 @@ 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
+# 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 which prevented the Valve and its
parent containers from starting
+
# Error report valve
errorReportValve.errorReport=Error report
errorReportValve.statusHeader=HTTP Status {0} - {1}
Modified:
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/RequestFilterValve.java
URL:
http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/RequestFilterValve.java?rev=1221276&r1=1221275&r2=1221276&view=diff
==============================================================================
---
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/RequestFilterValve.java
(original)
+++
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/RequestFilterValve.java
Tue Dec 20 14:25:24 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;
import org.apache.tomcat.util.compat.JdkCompat;
@@ -67,7 +71,7 @@ import org.apache.tomcat.util.compat.Jdk
*/
public abstract class RequestFilterValve
- extends ValveBase {
+ extends ValveBase implements Lifecycle {
// ----------------------------------------------------- Class Variables
@@ -99,26 +103,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
@@ -141,10 +170,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 {
+ this.allow = allow;
+ allows = precalculate(allow);
+ success = true;
+ } finally {
+ allowValid = success;
+ }
}
@@ -166,10 +199,34 @@ public abstract class RequestFilterValve
* @param deny The new set of deny expressions
*/
public void setDeny(String deny) {
+ boolean success = false;
+ try {
+ this.deny = deny;
+ denies = precalculate(deny);
+ success = true;
+ } finally {
+ denyValid = success;
+ }
+ }
- this.deny = deny;
- denies = precalculate(deny);
+ /**
+ * Returns <code>false</code> if the last change to the
+ * <code>allow</code> pattern did not apply successfully. E.g.
+ * if the pattern is syntactically invalid.
+ */
+ public final boolean isAllowValid() {
+ return allowValid;
+ }
+
+
+ /**
+ * Returns <code>false</code> if the last change to the
+ * <code>deny</code> pattern did not apply successfully. E.g.
+ * if the pattern is syntactically invalid.
+ */
+ public final boolean isDenyValid() {
+ return denyValid;
}
@@ -262,32 +319,125 @@ public abstract class RequestFilterValve
Request request, Response response)
throws IOException, ServletException {
+ if (isAllowed(property)) {
+ getNext().invoke(request, response);
+ return;
+ }
+
+ // Deny this request
+ response.sendError(HttpServletResponse.SC_FORBIDDEN);
+
+ }
+
+
+ /**
+ * Perform the test implemented by this Valve, matching against the
+ * specified request property value. This method is public so that it can
be
+ * called through JMX, e.g. to test whether certain IP address is allowed
or
+ * denied by the valve configuration.
+ *
+ * @param property
+ * The request property value on which to filter
+ */
+ public boolean isAllowed(String property) {
+ // 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()) {
- response.sendError(HttpServletResponse.SC_FORBIDDEN);
- return;
+ return false;
}
}
// Check the allow patterns, if any
for (int i = 0; i < allows.length; i++) {
if (allows[i].matcher(property).matches()) {
- getNext().invoke(request, response);
- return;
+ return true;
}
}
// Allow if denies specified but not allows
if ((denies.length > 0) && (allows.length == 0)) {
- getNext().invoke(request, response);
- return;
+ return true;
}
// Deny this request
- response.sendError(HttpServletResponse.SC_FORBIDDEN);
+ return false;
+ }
+
+
+ // ------------------------------------------------------ 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/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/mbeans-descriptors.xml
URL:
http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/mbeans-descriptors.xml?rev=1221276&r1=1221275&r2=1221276&view=diff
==============================================================================
---
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/mbeans-descriptors.xml
(original)
+++
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/mbeans-descriptors.xml
Tue Dec 20 14:25:24 2011
@@ -229,6 +229,12 @@
description="The comma-delimited set of allow expressions"
type="java.lang.String"/>
+ <attribute name="allowValid"
+ description="Becomes false if assigned value of allow
expression is not syntactically correct"
+ is="true"
+ type="boolean"
+ writeable="false"/>
+
<attribute name="containerName"
description="Object name of the container"
type="javax.management.ObjectName"/>
@@ -242,6 +248,20 @@
description="The comma-delimited set of deny expressions"
type="java.lang.String"/>
+ <attribute name="denyValid"
+ description="Becomes false if assigned value of deny expression
is not syntactically correct"
+ is="true"
+ type="boolean"
+ writeable="false"/>
+
+ <operation name="isAllowed"
+ description="Tests whether a client with this IP address value
is allowed access by the current valve configuration"
+ impact="INFO"
+ returnType="boolean">
+ <parameter name="ipAddress"
+ description="IP address to be tested"
+ type="java.lang.String"/>
+ </operation>
</mbean>
<mbean name="RemoteHostValve"
@@ -256,6 +276,12 @@
description="The comma-delimited set of allow expressions"
type="java.lang.String"/>
+ <attribute name="allowValid"
+ description="Becomes false if assigned value of allow
expression is not syntactically correct"
+ is="true"
+ type="boolean"
+ writeable="false"/>
+
<attribute name="containerName"
description="Object name of the container"
type="javax.management.ObjectName"/>
@@ -269,6 +295,21 @@
description="The comma-delimited set of deny expressions"
type="java.lang.String"/>
+ <attribute name="denyValid"
+ description="Becomes false if assigned value of deny expression
is not syntactically correct"
+ is="true"
+ type="boolean"
+ writeable="false"/>
+
+ <operation name="isAllowed"
+ description="Tests whether a client with this host name is
allowed access by the current valve configuration"
+ impact="INFO"
+ returnType="boolean">
+ <parameter name="hostName"
+ description="host name to be tested"
+ type="java.lang.String"/>
+ </operation>
+
</mbean>
<mbean name="RequestDumperValve"
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
