Selaa lähdekoodia

SEC-641: Replaced use of Assert with more tooling friendly calls to parserContext.getReaderContext().error()

Luke Taylor 17 vuotta sitten
vanhempi
commit
717ab0b3cc

+ 11 - 5
core/src/main/java/org/springframework/security/config/FilterChainMapBeanDefinitionDecorator.java

@@ -8,7 +8,6 @@ import org.springframework.beans.factory.support.ManagedMap;
 import org.springframework.beans.factory.xml.BeanDefinitionDecorator;
 import org.springframework.beans.factory.xml.ParserContext;
 import org.springframework.security.util.RegexUrlPathMatcher;
-import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
 import org.springframework.util.xml.DomUtils;
 import org.w3c.dom.Element;
@@ -40,11 +39,18 @@ class FilterChainMapBeanDefinitionDecorator implements BeanDefinitionDecorator {
         while (filterChainElts.hasNext()) {
             Element chain = (Element) filterChainElts.next();
             String path = chain.getAttribute(HttpSecurityBeanDefinitionParser.ATT_PATH_PATTERN);
-            Assert.hasText(path, "The attribute '" + HttpSecurityBeanDefinitionParser.ATT_PATH_PATTERN +
-                    "' must not be empty");
+
+            if(!StringUtils.hasText(path)) {
+                parserContext.getReaderContext().error("The attribute '" + HttpSecurityBeanDefinitionParser.ATT_PATH_PATTERN +
+                    "' must not be empty", elt);
+            }
+
             String filters = chain.getAttribute(HttpSecurityBeanDefinitionParser.ATT_FILTERS);
-            Assert.hasText(filters, "The attribute '" + HttpSecurityBeanDefinitionParser.ATT_FILTERS +
-                    "'must not be empty");
+
+            if(!StringUtils.hasText(filters)) {
+                parserContext.getReaderContext().error("The attribute '" + HttpSecurityBeanDefinitionParser.ATT_FILTERS +
+                    "'must not be empty", elt);    
+            }
 
             if (filters.equals(HttpSecurityBeanDefinitionParser.OPT_FILTERS_NONE)) {
                 filterChainMap.put(path, Collections.EMPTY_LIST);

+ 5 - 3
core/src/main/java/org/springframework/security/config/HttpSecurityBeanDefinitionParser.java

@@ -31,7 +31,6 @@ import org.springframework.security.util.FilterChainProxy;
 import org.springframework.security.util.RegexUrlPathMatcher;
 import org.springframework.security.util.AntUrlPathMatcher;
 import org.springframework.security.util.UrlMatcher;
-import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
 import org.springframework.util.xml.DomUtils;
 import org.w3c.dom.Element;
@@ -290,6 +289,11 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser {
             Element urlElt = (Element) urlEltsIterator.next();
 
             String path = urlElt.getAttribute(ATT_PATH_PATTERN);
+
+            if(!StringUtils.hasText(path)) {
+                parserContext.getReaderContext().error("path attribute cannot be empty or null", urlElt);
+            }
+
             if (useLowerCasePaths) {
                 path = path.toLowerCase();
             }
@@ -299,8 +303,6 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser {
                 method = null;
             }
 
-            Assert.hasText(path, "path attribute cannot be empty or null");
-
             String access = urlElt.getAttribute(ATT_ACCESS_CONFIG);
 
             // Convert the comma-separated list of access attributes to a ConfigAttributeDefinition

+ 4 - 3
core/src/main/java/org/springframework/security/config/LdapServerBeanDefinitionParser.java

@@ -8,7 +8,6 @@ import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.support.RootBeanDefinition;
 import org.springframework.ldap.core.DirContextAdapter;
 import org.springframework.util.StringUtils;
-import org.springframework.util.Assert;
 
 import org.w3c.dom.Element;
 import org.apache.directory.server.configuration.MutableServerStartupConfiguration;
@@ -65,8 +64,10 @@ public class LdapServerBeanDefinitionParser implements BeanDefinitionParser {
         String managerPassword = elt.getAttribute(ATT_PASSWORD);
 
         if (StringUtils.hasText(managerDn)) {
-            Assert.hasText(managerPassword, "You must specify the " + ATT_PASSWORD +
-                    " if you supply a " + managerDn);
+            if(!StringUtils.hasText(managerPassword)) {
+                parserContext.getReaderContext().error("You must specify the " + ATT_PASSWORD +
+                        " if you supply a " + managerDn, elt);
+            }
 
             contextSource.getPropertyValues().addPropertyValue("userDn", managerDn);
             contextSource.getPropertyValues().addPropertyValue("password", managerPassword);

+ 5 - 2
core/src/main/java/org/springframework/security/config/LdapUserServiceBeanDefinitionParser.java

@@ -9,7 +9,6 @@ import org.springframework.beans.factory.support.RootBeanDefinition;
 import org.springframework.beans.factory.config.RuntimeBeanReference;
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.util.StringUtils;
-import org.springframework.util.Assert;
 
 import org.w3c.dom.Element;
 
@@ -41,7 +40,11 @@ public class LdapUserServiceBeanDefinitionParser extends AbstractUserDetailsServ
         }
 
         String userSearchFilter = elt.getAttribute(ATT_USER_SEARCH_FILTER);
-        Assert.hasText(userSearchFilter, "User search filter must be supplied");
+        
+        if (!StringUtils.hasText(userSearchFilter)) {
+            parserContext.getReaderContext().error("User search filter must be supplied", elt);
+        }
+
         String userSearchBase = elt.getAttribute(ATT_USER_SEARCH_BASE);
 
         if (!StringUtils.hasText(userSearchBase)) {

+ 14 - 5
core/src/main/java/org/springframework/security/config/PortMappingsBeanDefinitionParser.java

@@ -5,7 +5,7 @@ import org.springframework.beans.factory.xml.BeanDefinitionParser;
 import org.springframework.beans.factory.xml.ParserContext;
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.support.RootBeanDefinition;
-import org.springframework.util.Assert;
+import org.springframework.util.StringUtils;
 import org.springframework.util.xml.DomUtils;
 
 import org.w3c.dom.Element;
@@ -30,8 +30,11 @@ public class PortMappingsBeanDefinitionParser implements BeanDefinitionParser {
         BeanDefinition portMapper = new RootBeanDefinition(PortMapperImpl.class);
 
         if (element != null) {
-            List mappingElts = DomUtils.getChildElementsByTagName(element, Elements.PORT_MAPPING);    
-            Assert.notEmpty(mappingElts, "No port-mapping child elements!");
+            List mappingElts = DomUtils.getChildElementsByTagName(element, Elements.PORT_MAPPING);
+            if(mappingElts.isEmpty()) {
+                parserContext.getReaderContext().error("No port-mapping child elements specified", element);
+            }
+
             Map mappings = new HashMap();
 
             Iterator iterator = mappingElts.iterator();
@@ -39,8 +42,14 @@ public class PortMappingsBeanDefinitionParser implements BeanDefinitionParser {
                 Element elt = (Element) iterator.next();
                 String httpPort = elt.getAttribute(ATT_HTTP_PORT);
                 String httpsPort = elt.getAttribute(ATT_HTTPS_PORT);
-                Assert.notNull(httpPort, "No http port supplied in mapping");
-                Assert.notNull(httpsPort, "No https port supplied in mapping");
+
+                if (!StringUtils.hasText(httpPort)) {
+                    parserContext.getReaderContext().error("No http port supplied in port mapping", elt);
+                }
+
+                if (!StringUtils.hasText(httpsPort)) {
+                    parserContext.getReaderContext().error("No https port supplied in port mapping", elt);
+                }
 
                 mappings.put(httpPort, httpsPort);
             }

+ 11 - 6
core/src/main/java/org/springframework/security/config/UserServiceBeanDefinitionParser.java

@@ -4,12 +4,13 @@ import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.config.PropertiesFactoryBean;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.beans.factory.support.RootBeanDefinition;
+import org.springframework.beans.factory.BeanDefinitionStoreException;
 import org.springframework.security.userdetails.memory.InMemoryDaoImpl;
 import org.springframework.security.userdetails.memory.UserMap;
 import org.springframework.security.userdetails.User;
 import org.springframework.security.util.AuthorityUtils;
 import org.springframework.util.StringUtils;
-import org.springframework.util.Assert;
+import org.springframework.util.CollectionUtils;
 import org.springframework.util.xml.DomUtils;
 import org.w3c.dom.Element;
 
@@ -40,8 +41,10 @@ public class UserServiceBeanDefinitionParser extends AbstractUserDetailsServiceB
         List userElts = DomUtils.getChildElementsByTagName(element, ELT_USER);
 
         if (StringUtils.hasText(userProperties)) {
-            Assert.isTrue(userElts.isEmpty(), "Use of a properties file ('" + ATT_PROPERTIES + "' attribute) and <" +
-                    ELT_USER + "> elements are mutually exclusive.");
+
+            if(!CollectionUtils.isEmpty(userElts)) {
+                throw new BeanDefinitionStoreException("Use of a properties file and user elements are mutually exclusive");
+            }
 
             BeanDefinition bd = new RootBeanDefinition(PropertiesFactoryBean.class);
             bd.getPropertyValues().addPropertyValue("location", userProperties);
@@ -50,9 +53,11 @@ public class UserServiceBeanDefinitionParser extends AbstractUserDetailsServiceB
             return;
         }
 
-        Assert.notEmpty(userElts, "You must supply user definitions, either with <" + ELT_USER + "> child elements or a " +
-                "properties file (specified with the '" + ATT_PROPERTIES + "' attribute)" );
-
+        if(CollectionUtils.isEmpty(userElts)) {
+            throw new BeanDefinitionStoreException("You must supply user definitions, either with <" + ELT_USER + "> child elements or a " +
+                "properties file (using the '" + ATT_PROPERTIES + "' attribute)" );
+        }
+        
         UserMap users = new UserMap();
 
         for (Iterator i = userElts.iterator(); i.hasNext();) {