Author: markt
Date: Thu Jun 20 12:27:08 2013
New Revision: 1494951

URL: http://svn.apache.org/r1494951
Log:
Refactor with a view to adding some unit tests

Modified:
    tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties
    tomcat/trunk/java/org/apache/catalina/core/StandardContext.java
    tomcat/trunk/java/org/apache/catalina/deploy/LocalStrings.properties
    tomcat/trunk/java/org/apache/catalina/deploy/SecurityConstraint.java

Modified: tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties?rev=1494951&r1=1494950&r2=1494951&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties 
(original)
+++ tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties Thu Jun 
20 12:27:08 2013
@@ -139,10 +139,6 @@ standardContext.servletMap.pattern=Inval
 standardContext.startFailed=Context [{0}] startup failed due to previous errors
 standardContext.startingContext=Exception starting Context with name [{0}]
 standardContext.stoppingContext=Exception stopping Context with name [{0}]
-standardContext.uncoveredHttpMethod=For security constraints with URL pattern 
[{0}] only the HTTP methods [{1}] are covered. All other methods are uncovered.
-standardContext.uncoveredHttpMethodFix=Adding security constraints with URL 
pattern [{0}] to deny access with the uncovered HTTP methods that are not one 
of the following [{1}]
-standardContext.uncoveredHttpOmittedMethod=For security constraints with URL 
pattern [{0}] the HTTP methods [{1}] are uncovered.
-standardContext.uncoveredHttpOmittedMethodFix=Adding security constraints with 
URL pattern [{0}] to deny access with the uncovered HTTP methods [{1}]
 standardContext.urlPattern.patternWarning=WARNING: URL pattern {0} must start 
with a ''/'' in Servlet 2.4
 standardContext.webappClassLoader.missingProperty=Unable to set the web 
application class loader property [{0}] to [{1}] as the property does not exist.
 standardContext.workPath=Exception obtaining work path for context [{0}]

Modified: tomcat/trunk/java/org/apache/catalina/core/StandardContext.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/StandardContext.java?rev=1494951&r1=1494950&r2=1494951&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/StandardContext.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/StandardContext.java Thu Jun 20 
12:27:08 2013
@@ -5359,172 +5359,11 @@ public class StandardContext extends Con
 
 
     private void checkConstraintsForUncoveredMethods() {
-        // TODO - Add an option to lower the log level of any uncovered method
-        //        warnings to debug
-        Set<String> coveredPatterns = new HashSet<>();
-        Map<String,Set<String>> urlMethodMap = new HashMap<>();
-        Map<String,Set<String>> urlOmittedMethodMap = new HashMap<>();
-
-        // First build the lists of covered patterns and those patterns that
-        // might be uncovered
-        for (SecurityConstraint constraint : constraints) {
-            SecurityCollection[] collections = constraint.findCollections();
-            for (SecurityCollection collection : collections) {
-                String[] patterns = collection.findPatterns();
-                String[] methods = collection.findMethods();
-                String[] omittedMethods = collection.findOmittedMethods();
-                // Simple case: no methods
-                if (methods.length == 0 && omittedMethods.length == 0) {
-                    for (String pattern : patterns) {
-                        coveredPatterns.add(pattern);
-                    }
-                    continue;
-                }
-
-                // Pre-calculate so we don't do this for every iteration of the
-                // following loop
-                List<String> omNew = null;
-                if (omittedMethods.length != 0) {
-                    omNew = Arrays.asList(omittedMethods);
-                }
-
-                // Only need to process uncovered patterns
-                for (String pattern : patterns) {
-                    if (!coveredPatterns.contains(pattern)) {
-                        if (methods.length == 0) {
-                            // Build the interset of omitted methods for this
-                            // pattern
-                            Set<String> om = urlOmittedMethodMap.get(pattern);
-                            if (om == null) {
-                                om = new HashSet<>();
-                                urlOmittedMethodMap.put(pattern, om);
-                                om.addAll(omNew);
-                            } else {
-                                om.retainAll(omNew);
-                            }
-                        } else {
-                            // Build the union of methods for this pattern
-                            Set<String> m = urlMethodMap.get(pattern);
-                            if (m == null) {
-                                m = new HashSet<>();
-                                urlMethodMap.put(pattern, m);
-                            }
-                            for (String method : methods) {
-                                m.add(method);
-                            }
-                        }
-                    }
-                }
-            }
-        }
-
-        // Now check the potentially uncovered patterns
-        for (Map.Entry<String, Set<String>> entry : urlMethodMap.entrySet()) {
-            String pattern = entry.getKey();
-            if (coveredPatterns.contains(pattern)) {
-                // Fully covered. Ignore any partial coverage
-                urlOmittedMethodMap.remove(pattern);
-                continue;
-            }
-
-            Set<String> omittedMethods = urlOmittedMethodMap.get(pattern);
-            Set<String> methods = entry.getValue();
-
-            if (omittedMethods == null) {
-                StringBuilder msg = new StringBuilder();
-                for (String method : methods) {
-                    msg.append(method);
-                    msg.append(' ');
-                }
-                if (getDenyUncoveredHttpMethods()) {
-                    log.info(sm.getString(
-                            "standardContext.uncoveredHttpMethodFix",
-                            pattern, msg.toString().trim()));
-                    SecurityCollection collection = new SecurityCollection();
-                    for (String method : methods) {
-                        collection.addOmittedMethod(method);
-                    }
-                    collection.addPattern(pattern);
-                    collection.setName("deny-uncovered-http-methods");
-                    SecurityConstraint constraint = new SecurityConstraint();
-                    constraint.setAuthConstraint(true);
-                    constraint.addCollection(collection);
-                    addConstraint(constraint);
-                } else {
-                    log.error(sm.getString(
-                            "standardContext.uncoveredHttpMethod",
-                            pattern, msg.toString().trim()));
-                }
-                continue;
-            }
-
-            // As long as every omitted method as a corresponding method the
-            // pattern is fully covered.
-            omittedMethods.removeAll(methods);
-
-            if (omittedMethods.size() > 0) {
-                StringBuilder msg = new StringBuilder();
-                for (String method : omittedMethods) {
-                    msg.append(method);
-                    msg.append(' ');
-                }
-                if (getDenyUncoveredHttpMethods()) {
-                    log.info(sm.getString(
-                            "standardContext.uncoveredHttpOmittedMethodFix",
-                            pattern, msg.toString().trim()));
-                    SecurityCollection collection = new SecurityCollection();
-                    for (String method : omittedMethods) {
-                        collection.addMethod(method);
-                    }
-                    collection.addPattern(pattern);
-                    collection.setName("deny-uncovered-http-methods");
-                    SecurityConstraint constraint = new SecurityConstraint();
-                    constraint.setAuthConstraint(true);
-                    constraint.addCollection(collection);
-                    addConstraint(constraint);
-                } else {
-                    log.error(sm.getString(
-                            "standardContext.uncoveredHttpOmittedMethod",
-                            pattern, msg.toString().trim()));
-                }
-            }
-        }
-        for (Map.Entry<String, Set<String>> entry :
-                urlOmittedMethodMap.entrySet()) {
-            String pattern = entry.getKey();
-            if (coveredPatterns.contains(pattern)) {
-                // Fully covered. Ignore any partial coverage
-                continue;
-            }
-
-            Set<String> omittedMethods = entry.getValue();
-
-            if (omittedMethods.size() > 0) {
-                StringBuilder msg = new StringBuilder();
-                for (String method : omittedMethods) {
-                    msg.append(method);
-                    msg.append(' ');
-                }
-                if (getDenyUncoveredHttpMethods()) {
-                    log.info(sm.getString(
-                            "standardContext.uncoveredHttpOmittedMethodFix",
-                            pattern, msg.toString().trim()));
-                    SecurityCollection collection = new SecurityCollection();
-                    for (String method : omittedMethods) {
-                        collection.addMethod(method);
-                    }
-                    collection.addPattern(pattern);
-                    collection.setName("deny-uncovered-http-methods");
-                    SecurityConstraint constraint = new SecurityConstraint();
-                    constraint.setAuthConstraint(true);
-                    constraint.addCollection(collection);
-                    addConstraint(constraint);
-                } else {
-                    log.error(sm.getString(
-                            "standardContext.uncoveredHttpOmittedMethod",
-                            pattern, msg.toString().trim()));
-                }
-            }
+        SecurityConstraint[] newConstraints =
+                SecurityConstraint.findUncoveredHttpMethods(findConstraints(),
+                        getDenyUncoveredHttpMethods(), getLogger());
+        for (SecurityConstraint constraint : newConstraints) {
+            addConstraint(constraint);
         }
     }
 

Modified: tomcat/trunk/java/org/apache/catalina/deploy/LocalStrings.properties
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/deploy/LocalStrings.properties?rev=1494951&r1=1494950&r2=1494951&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/deploy/LocalStrings.properties 
(original)
+++ tomcat/trunk/java/org/apache/catalina/deploy/LocalStrings.properties Thu 
Jun 20 12:27:08 2013
@@ -53,4 +53,9 @@ namingResources.cleanupNoContext=Failed 
 namingResources.cleanupNoResource=Failed to retrieve JNDI resource [{0}] for 
container [{1}] so no cleanup was performed for that resource
 namingResources.mbeanCreateFail=Failed to create MBean for naming resource 
[{0}]
 namingResources.mbeanDestroyFail=Failed to destroy MBean for naming resource 
[{0}]
-namingResources.resourceTypeFail=The JNDI resource named [{0}] is of type 
[{1}] but the type is inconsistent with the type(s) of the injection target(s) 
configured for that resource
\ No newline at end of file
+namingResources.resourceTypeFail=The JNDI resource named [{0}] is of type 
[{1}] but the type is inconsistent with the type(s) of the injection target(s) 
configured for that resource
+
+securityConstraint.uncoveredHttpMethod=For security constraints with URL 
pattern [{0}] only the HTTP methods [{1}] are covered. All other methods are 
uncovered.
+securityConstraint.uncoveredHttpMethodFix=Adding security constraints with URL 
pattern [{0}] to deny access with the uncovered HTTP methods that are not one 
of the following [{1}]
+securityConstraint.uncoveredHttpOmittedMethod=For security constraints with 
URL pattern [{0}] the HTTP methods [{1}] are uncovered.
+securityConstraint.uncoveredHttpOmittedMethodFix=Adding security constraints 
with URL pattern [{0}] to deny access with the uncovered HTTP methods [{1}]

Modified: tomcat/trunk/java/org/apache/catalina/deploy/SecurityConstraint.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/deploy/SecurityConstraint.java?rev=1494951&r1=1494950&r2=1494951&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/deploy/SecurityConstraint.java 
(original)
+++ tomcat/trunk/java/org/apache/catalina/deploy/SecurityConstraint.java Thu 
Jun 20 12:27:08 2013
@@ -19,9 +19,14 @@
 package org.apache.catalina.deploy;
 
 import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 import javax.servlet.HttpConstraintElement;
@@ -30,6 +35,9 @@ import javax.servlet.ServletSecurityElem
 import javax.servlet.annotation.ServletSecurity;
 import javax.servlet.annotation.ServletSecurity.EmptyRoleSemantic;
 
+import org.apache.juli.logging.Log;
+import org.apache.tomcat.util.res.StringManager;
+
 
 /**
  * Representation of a security constraint element for a web application,
@@ -53,16 +61,17 @@ public class SecurityConstraint implemen
     public static final String ROLE_ALL_ROLES = "*";
     public static final String ROLE_ALL_AUTHENTICATED_USERS = "**";
 
-    // ----------------------------------------------------------- Constructors
+    private static final StringManager sm =
+            StringManager.getManager(Constants.Package);
 
 
+    // ----------------------------------------------------------- Constructors
+
     /**
      * Construct a new security constraint instance with default values.
      */
     public SecurityConstraint() {
-
         super();
-
     }
 
 
@@ -600,4 +609,181 @@ public class SecurityConstraint implemen
 
         return null;
     }
+
+
+    public static SecurityConstraint[] findUncoveredHttpMethods(
+            SecurityConstraint[] constraints,
+            boolean denyUncoveredHttpMethods, Log log) {
+
+        Set<String> coveredPatterns = new HashSet<>();
+        Map<String,Set<String>> urlMethodMap = new HashMap<>();
+        Map<String,Set<String>> urlOmittedMethodMap = new HashMap<>();
+
+        List<SecurityConstraint> newConstraints = new ArrayList<>();
+
+        // First build the lists of covered patterns and those patterns that
+        // might be uncovered
+        for (SecurityConstraint constraint : constraints) {
+            SecurityCollection[] collections = constraint.findCollections();
+            for (SecurityCollection collection : collections) {
+                String[] patterns = collection.findPatterns();
+                String[] methods = collection.findMethods();
+                String[] omittedMethods = collection.findOmittedMethods();
+                // Simple case: no methods
+                if (methods.length == 0 && omittedMethods.length == 0) {
+                    for (String pattern : patterns) {
+                        coveredPatterns.add(pattern);
+                    }
+                    continue;
+                }
+
+                // Pre-calculate so we don't do this for every iteration of the
+                // following loop
+                List<String> omNew = null;
+                if (omittedMethods.length != 0) {
+                    omNew = Arrays.asList(omittedMethods);
+                }
+
+                // Only need to process uncovered patterns
+                for (String pattern : patterns) {
+                    if (!coveredPatterns.contains(pattern)) {
+                        if (methods.length == 0) {
+                            // Build the interset of omitted methods for this
+                            // pattern
+                            Set<String> om = urlOmittedMethodMap.get(pattern);
+                            if (om == null) {
+                                om = new HashSet<>();
+                                urlOmittedMethodMap.put(pattern, om);
+                                om.addAll(omNew);
+                            } else {
+                                om.retainAll(omNew);
+                            }
+                        } else {
+                            // Build the union of methods for this pattern
+                            Set<String> m = urlMethodMap.get(pattern);
+                            if (m == null) {
+                                m = new HashSet<>();
+                                urlMethodMap.put(pattern, m);
+                            }
+                            for (String method : methods) {
+                                m.add(method);
+                            }
+                        }
+                    }
+                }
+            }
+        }
+
+        // Now check the potentially uncovered patterns
+        for (Map.Entry<String, Set<String>> entry : urlMethodMap.entrySet()) {
+            String pattern = entry.getKey();
+            if (coveredPatterns.contains(pattern)) {
+                // Fully covered. Ignore any partial coverage
+                urlOmittedMethodMap.remove(pattern);
+                continue;
+            }
+
+            Set<String> omittedMethods = urlOmittedMethodMap.get(pattern);
+            Set<String> methods = entry.getValue();
+
+            if (omittedMethods == null) {
+                StringBuilder msg = new StringBuilder();
+                for (String method : methods) {
+                    msg.append(method);
+                    msg.append(' ');
+                }
+                if (denyUncoveredHttpMethods) {
+                    log.info(sm.getString(
+                            "securityConstraint.uncoveredHttpMethodFix",
+                            pattern, msg.toString().trim()));
+                    SecurityCollection collection = new SecurityCollection();
+                    for (String method : methods) {
+                        collection.addOmittedMethod(method);
+                    }
+                    collection.addPattern(pattern);
+                    collection.setName("deny-uncovered-http-methods");
+                    SecurityConstraint constraint = new SecurityConstraint();
+                    constraint.setAuthConstraint(true);
+                    constraint.addCollection(collection);
+                    newConstraints.add(constraint);
+                } else {
+                    log.error(sm.getString(
+                            "securityConstraint.uncoveredHttpMethod",
+                            pattern, msg.toString().trim()));
+                }
+                continue;
+            }
+
+            // As long as every omitted method as a corresponding method the
+            // pattern is fully covered.
+            omittedMethods.removeAll(methods);
+
+            if (omittedMethods.size() > 0) {
+                StringBuilder msg = new StringBuilder();
+                for (String method : omittedMethods) {
+                    msg.append(method);
+                    msg.append(' ');
+                }
+                if (denyUncoveredHttpMethods) {
+                    log.info(sm.getString(
+                            "securityConstraint.uncoveredHttpOmittedMethodFix",
+                            pattern, msg.toString().trim()));
+                    SecurityCollection collection = new SecurityCollection();
+                    for (String method : omittedMethods) {
+                        collection.addMethod(method);
+                    }
+                    collection.addPattern(pattern);
+                    collection.setName("deny-uncovered-http-methods");
+                    SecurityConstraint constraint = new SecurityConstraint();
+                    constraint.setAuthConstraint(true);
+                    constraint.addCollection(collection);
+                    newConstraints.add(constraint);
+                } else {
+                    log.error(sm.getString(
+                            "securityConstraint.uncoveredHttpOmittedMethod",
+                            pattern, msg.toString().trim()));
+                }
+            }
+        }
+        for (Map.Entry<String, Set<String>> entry :
+                urlOmittedMethodMap.entrySet()) {
+            String pattern = entry.getKey();
+            if (coveredPatterns.contains(pattern)) {
+                // Fully covered. Ignore any partial coverage
+                continue;
+            }
+
+            Set<String> omittedMethods = entry.getValue();
+
+            if (omittedMethods.size() > 0) {
+                StringBuilder msg = new StringBuilder();
+                for (String method : omittedMethods) {
+                    msg.append(method);
+                    msg.append(' ');
+                }
+                if (denyUncoveredHttpMethods) {
+                    log.info(sm.getString(
+                            "securityConstraint.uncoveredHttpOmittedMethodFix",
+                            pattern, msg.toString().trim()));
+                    SecurityCollection collection = new SecurityCollection();
+                    for (String method : omittedMethods) {
+                        collection.addMethod(method);
+                    }
+                    collection.addPattern(pattern);
+                    collection.setName("deny-uncovered-http-methods");
+                    SecurityConstraint constraint = new SecurityConstraint();
+                    constraint.setAuthConstraint(true);
+                    constraint.addCollection(collection);
+                    newConstraints.add(constraint);
+                } else {
+                    log.error(sm.getString(
+                            "securityConstraint.uncoveredHttpOmittedMethod",
+                            pattern, msg.toString().trim()));
+                }
+            }
+        }
+
+        return newConstraints.toArray(
+                new SecurityConstraint[newConstraints.size()]);
+    }
 }



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

Reply via email to