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