* 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.
git-svn-id: https://svn.apache.org/repos/asf/tomcat/tc5.5.x/trunk@1221276 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/STATUS.txt b/STATUS.txt
index 59f5eef..1f3fa39 100644
--- a/STATUS.txt
+++ b/STATUS.txt
@@ -24,25 +24,6 @@
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.
diff --git a/container/catalina/src/share/org/apache/catalina/valves/LocalStrings.properties b/container/catalina/src/share/org/apache/catalina/valves/LocalStrings.properties
index 4886834..1b90c8e 100644
--- a/container/catalina/src/share/org/apache/catalina/valves/LocalStrings.properties
+++ b/container/catalina/src/share/org/apache/catalina/valves/LocalStrings.properties
@@ -21,12 +21,15 @@
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}
diff --git a/container/catalina/src/share/org/apache/catalina/valves/RequestFilterValve.java b/container/catalina/src/share/org/apache/catalina/valves/RequestFilterValve.java
index 8e8250a..f3a5da1 100644
--- a/container/catalina/src/share/org/apache/catalina/valves/RequestFilterValve.java
+++ b/container/catalina/src/share/org/apache/catalina/valves/RequestFilterValve.java
@@ -27,8 +27,12 @@
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 @@
*/
public abstract class RequestFilterValve
- extends ValveBase {
+ extends ValveBase implements Lifecycle {
// ----------------------------------------------------- Class Variables
@@ -99,26 +103,51 @@
/**
* 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 @@
* @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 @@
* @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,24 +319,7 @@
Request request, Response response)
throws IOException, ServletException {
- // 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;
- }
- }
-
- // 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;
- }
- }
-
- // Allow if denies specified but not allows
- if ((denies.length > 0) && (allows.length == 0)) {
+ if (isAllowed(property)) {
getNext().invoke(request, response);
return;
}
@@ -290,4 +330,114 @@
}
+ /**
+ * 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()) {
+ return false;
+ }
+ }
+
+ // Check the allow patterns, if any
+ for (int i = 0; i < allows.length; i++) {
+ if (allows[i].matcher(property).matches()) {
+ return true;
+ }
+ }
+
+ // Allow if denies specified but not allows
+ if ((denies.length > 0) && (allows.length == 0)) {
+ return true;
+ }
+
+ // Deny this request
+ 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;
+ }
+
}
diff --git a/container/catalina/src/share/org/apache/catalina/valves/mbeans-descriptors.xml b/container/catalina/src/share/org/apache/catalina/valves/mbeans-descriptors.xml
index 422536a..e292950 100644
--- a/container/catalina/src/share/org/apache/catalina/valves/mbeans-descriptors.xml
+++ b/container/catalina/src/share/org/apache/catalina/valves/mbeans-descriptors.xml
@@ -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"
