From ea56a9888329a8706d716d5251a73fcccf4e3a20 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Thu, 29 Dec 2011 22:26:08 -0600 Subject: [PATCH] SEC-1868: Remove error level logs from SecurityNamespaceHandler when the web classes are not available and not required To get the detailed errors the FilterChainProxy is loaded again in reportMissingWebClasses and included in the readerContext fatal log. --- config/config.gradle | 8 +- .../config/SecurityNamespaceHandler.java | 19 ++-- .../config/SecurityNamespaceHandlerTests.java | 89 +++++++++++++++++++ gradle/javaprojects.gradle | 1 + 4 files changed, 110 insertions(+), 7 deletions(-) diff --git a/config/config.gradle b/config/config.gradle index fd9a9f0f4d..48f7698fd9 100644 --- a/config/config.gradle +++ b/config/config.gradle @@ -34,7 +34,13 @@ dependencies { "org.springframework:spring-jdbc:$springVersion", "org.springframework:spring-tx:$springVersion", 'org.spockframework:spock-core:0.5-groovy-1.7', - "org.slf4j:jcl-over-slf4j:$slf4jVersion" + "org.slf4j:jcl-over-slf4j:$slf4jVersion", + "org.powermock:powermock-core:$powerMockVersion", + "org.powermock:powermock-api-support:$powerMockVersion", + "org.powermock:powermock-module-junit4-common:$powerMockVersion", + "org.powermock:powermock-module-junit4:$powerMockVersion", + "org.powermock:powermock-api-mockito:$powerMockVersion", + "org.powermock:powermock-reflect:$powerMockVersion" testCompile('org.openid4java:openid4java-nodeps:0.9.6') { exclude group: 'com.google.code.guice', module: 'guice' } diff --git a/config/src/main/java/org/springframework/security/config/SecurityNamespaceHandler.java b/config/src/main/java/org/springframework/security/config/SecurityNamespaceHandler.java index e2f295d7c5..41950fa219 100644 --- a/config/src/main/java/org/springframework/security/config/SecurityNamespaceHandler.java +++ b/config/src/main/java/org/springframework/security/config/SecurityNamespaceHandler.java @@ -35,9 +35,11 @@ import java.util.*; * * @author Luke Taylor * @author Ben Alex + * @author Rob Winch * @since 2.0 */ public final class SecurityNamespaceHandler implements NamespaceHandler { + private static final String FILTER_CHAIN_PROXY_CLASSNAME = "org.springframework.security.web.FilterChainProxy"; private final Log logger = LogFactory.getLog(getClass()); private final Map parsers = new HashMap(); private final BeanDefinitionDecorator interceptMethodsBDD = new InterceptMethodsBeanDefinitionDecorator(); @@ -119,8 +121,16 @@ public final class SecurityNamespaceHandler implements NamespaceHandler { } private void reportMissingWebClasses(String nodeName, ParserContext pc, Node node) { - pc.getReaderContext().fatal("The classes from the spring-security-web jar " + - "(or one of its dependencies) are not available. You need these to use <" + nodeName + ">", node); + String errorMessage = "The classes from the spring-security-web jar " + + "(or one of its dependencies) are not available. You need these to use <" + nodeName + ">"; + try { + ClassUtils.forName(FILTER_CHAIN_PROXY_CLASSNAME, getClass().getClassLoader()); + // no details available + pc.getReaderContext().fatal(errorMessage, node); + } catch (Throwable cause) { + // provide details on why it could not be loaded + pc.getReaderContext().fatal(errorMessage, node, cause); + } } public void init() { @@ -141,8 +151,7 @@ public final class SecurityNamespaceHandler implements NamespaceHandler { parsers.put(Elements.METHOD_SECURITY_METADATA_SOURCE, new MethodSecurityMetadataSourceBeanDefinitionParser()); // Only load the web-namespace parsers if the web classes are available - try { - ClassUtils.forName("org.springframework.security.web.FilterChainProxy", getClass().getClassLoader()); + if(ClassUtils.isPresent(FILTER_CHAIN_PROXY_CLASSNAME, getClass().getClassLoader())) { parsers.put(Elements.DEBUG, new DebugBeanDefinitionParser()); parsers.put(Elements.HTTP, new HttpSecurityBeanDefinitionParser()); parsers.put(Elements.HTTP_FIREWALL, new HttpFirewallBeanDefinitionParser()); @@ -150,8 +159,6 @@ public final class SecurityNamespaceHandler implements NamespaceHandler { parsers.put(Elements.FILTER_SECURITY_METADATA_SOURCE, new FilterInvocationSecurityMetadataSourceParser()); parsers.put(Elements.FILTER_CHAIN, new FilterChainBeanDefinitionParser()); filterChainMapBDD = new FilterChainMapBeanDefinitionDecorator(); - } catch(Throwable t) { - logger.error("Failed to load required web classes", t); } } diff --git a/config/src/test/java/org/springframework/security/config/SecurityNamespaceHandlerTests.java b/config/src/test/java/org/springframework/security/config/SecurityNamespaceHandlerTests.java index 57189758aa..566e3803e2 100644 --- a/config/src/test/java/org/springframework/security/config/SecurityNamespaceHandlerTests.java +++ b/config/src/test/java/org/springframework/security/config/SecurityNamespaceHandlerTests.java @@ -1,17 +1,44 @@ package org.springframework.security.config; import static org.junit.Assert.*; +import static org.mockito.Matchers.*; +import static org.powermock.api.mockito.PowerMockito.*; +import org.apache.commons.logging.Log; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.junit.runner.RunWith; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; +import org.powermock.reflect.internal.WhiteboxImpl; import org.springframework.beans.factory.parsing.BeanDefinitionParsingException; import org.springframework.security.config.util.InMemoryXmlApplicationContext; +import org.springframework.util.ClassUtils; /** * * @author Luke Taylor + * @author Rob Winch * @since 3.0 */ +@RunWith(PowerMockRunner.class) +@PrepareForTest({ClassUtils.class}) public class SecurityNamespaceHandlerTests { + @Rule + public ExpectedException thrown = ExpectedException.none(); + + private static final String XML_AUTHENTICATION_MANAGER = + ""+ + " "+ + " "+ + " " + + " "+ + " "+ + ""; + private static final String XML_HTTP_BLOCK = ""; + private static final String FILTER_CHAIN_PROXY_CLASSNAME = "org.springframework.security.web.FilterChainProxy"; + @Test public void constructionSucceeds() { new SecurityNamespaceHandler(); @@ -33,4 +60,66 @@ public class SecurityNamespaceHandlerTests { assertTrue(expected.getMessage().contains("You cannot use a spring-security-2.0.xsd or")); } } + + // SEC-1868 + @Test + public void initDoesNotLogErrorWhenFilterChainProxyFailsToLoad() throws Exception { + String className = "javax.servlet.Filter"; + spy(ClassUtils.class); + doThrow(new NoClassDefFoundError(className)).when(ClassUtils.class,"forName",eq(FILTER_CHAIN_PROXY_CLASSNAME),any(ClassLoader.class)); + + Log logger = mock(Log.class); + SecurityNamespaceHandler handler = new SecurityNamespaceHandler(); + WhiteboxImpl.setInternalState(handler, Log.class, logger); + + handler.init(); + + verifyStatic(); + ClassUtils.forName(eq(FILTER_CHAIN_PROXY_CLASSNAME),any(ClassLoader.class)); + verifyZeroInteractions(logger); + } + + @Test + public void filterNoClassDefFoundError() throws Exception { + String className = "javax.servlet.Filter"; + thrown.expect(BeanDefinitionParsingException.class); + thrown.expectMessage("NoClassDefFoundError: "+className); + spy(ClassUtils.class); + doThrow(new NoClassDefFoundError(className)).when(ClassUtils.class,"forName",eq(FILTER_CHAIN_PROXY_CLASSNAME),any(ClassLoader.class)); + new InMemoryXmlApplicationContext( + XML_AUTHENTICATION_MANAGER + + XML_HTTP_BLOCK); + } + + @Test + public void filterNoClassDefFoundErrorNoHttpBlock() throws Exception { + String className = "javax.servlet.Filter"; + spy(ClassUtils.class); + doThrow(new NoClassDefFoundError(className)).when(ClassUtils.class,"forName",eq(FILTER_CHAIN_PROXY_CLASSNAME),any(ClassLoader.class)); + new InMemoryXmlApplicationContext( + XML_AUTHENTICATION_MANAGER); + // should load just fine since no http block + } + + @Test + public void filterChainProxyClassNotFoundException() throws Exception { + String className = FILTER_CHAIN_PROXY_CLASSNAME; + thrown.expect(BeanDefinitionParsingException.class); + thrown.expectMessage("ClassNotFoundException: "+className); + spy(ClassUtils.class); + doThrow(new ClassNotFoundException(className)).when(ClassUtils.class,"forName",eq(FILTER_CHAIN_PROXY_CLASSNAME),any(ClassLoader.class)); + new InMemoryXmlApplicationContext( + XML_AUTHENTICATION_MANAGER + + XML_HTTP_BLOCK); + } + + @Test + public void filterChainProxyClassNotFoundExceptionNoHttpBlock() throws Exception { + String className = FILTER_CHAIN_PROXY_CLASSNAME; + spy(ClassUtils.class); + doThrow(new ClassNotFoundException(className)).when(ClassUtils.class,"forName",eq(FILTER_CHAIN_PROXY_CLASSNAME),any(ClassLoader.class)); + new InMemoryXmlApplicationContext( + XML_AUTHENTICATION_MANAGER); + // should load just fine since no http block + } } diff --git a/gradle/javaprojects.gradle b/gradle/javaprojects.gradle index 30db251d9c..4822c65f1a 100644 --- a/gradle/javaprojects.gradle +++ b/gradle/javaprojects.gradle @@ -12,6 +12,7 @@ hsqlVersion = '1.8.0.10' slf4jVersion = '1.6.1' logbackVersion = '0.9.29' cglibVersion = '2.2' +powerMockVersion = '1.4.10' bundlorProperties = [ version: version,