From 717ab0b3cc88a181906abd17be3bdd4edbd730c3 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Tue, 5 Feb 2008 11:29:52 +0000 Subject: [PATCH] SEC-641: Replaced use of Assert with more tooling friendly calls to parserContext.getReaderContext().error() --- ...FilterChainMapBeanDefinitionDecorator.java | 16 +++++++++++----- .../HttpSecurityBeanDefinitionParser.java | 8 +++++--- .../LdapServerBeanDefinitionParser.java | 7 ++++--- .../LdapUserServiceBeanDefinitionParser.java | 7 +++++-- .../PortMappingsBeanDefinitionParser.java | 19 ++++++++++++++----- .../UserServiceBeanDefinitionParser.java | 17 +++++++++++------ 6 files changed, 50 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/org/springframework/security/config/FilterChainMapBeanDefinitionDecorator.java b/core/src/main/java/org/springframework/security/config/FilterChainMapBeanDefinitionDecorator.java index 044967eb1a..7ca3e44ef1 100644 --- a/core/src/main/java/org/springframework/security/config/FilterChainMapBeanDefinitionDecorator.java +++ b/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); diff --git a/core/src/main/java/org/springframework/security/config/HttpSecurityBeanDefinitionParser.java b/core/src/main/java/org/springframework/security/config/HttpSecurityBeanDefinitionParser.java index 24855d7923..047224a962 100644 --- a/core/src/main/java/org/springframework/security/config/HttpSecurityBeanDefinitionParser.java +++ b/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 diff --git a/core/src/main/java/org/springframework/security/config/LdapServerBeanDefinitionParser.java b/core/src/main/java/org/springframework/security/config/LdapServerBeanDefinitionParser.java index 6e7b86366f..7d0c65efff 100644 --- a/core/src/main/java/org/springframework/security/config/LdapServerBeanDefinitionParser.java +++ b/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); diff --git a/core/src/main/java/org/springframework/security/config/LdapUserServiceBeanDefinitionParser.java b/core/src/main/java/org/springframework/security/config/LdapUserServiceBeanDefinitionParser.java index aa7157843e..43c4005655 100644 --- a/core/src/main/java/org/springframework/security/config/LdapUserServiceBeanDefinitionParser.java +++ b/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)) { diff --git a/core/src/main/java/org/springframework/security/config/PortMappingsBeanDefinitionParser.java b/core/src/main/java/org/springframework/security/config/PortMappingsBeanDefinitionParser.java index 9578ec7e1f..245f14e414 100644 --- a/core/src/main/java/org/springframework/security/config/PortMappingsBeanDefinitionParser.java +++ b/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); } diff --git a/core/src/main/java/org/springframework/security/config/UserServiceBeanDefinitionParser.java b/core/src/main/java/org/springframework/security/config/UserServiceBeanDefinitionParser.java index b932407336..fc1198db45 100644 --- a/core/src/main/java/org/springframework/security/config/UserServiceBeanDefinitionParser.java +++ b/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();) {