From 3fb77f3b59b781e26b9ae249810ecdfba6306ee8 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Thu, 1 Sep 2016 14:59:35 -0500 Subject: [PATCH] Polish SecurityJacksonModules Issue gh-3736 * ClassLoader argument - this is required because we do not want to assume the ClassLoader that should be used * Clean up logging - logging is now at debug level because we don't expect all of the modules are loaded (they are quite possibly off the ClassPath) * Remove ObjectUtils as it was being used on methods that expect a Collection or Array with non collection based objects * Polish Javadoc warnings --- .../CasAuthenticationTokenMixinTests.java | 3 +- .../jackson2/SecurityJacksonModules.java | 34 +++++++++---------- .../security/jackson2/AbstractMixinTests.java | 3 +- .../web/jackson2/WebJackson2Module.java | 2 +- .../web/jackson2/AbstractMixinTests.java | 3 +- .../web/jackson2/CookieMixinTests.java | 3 +- .../jackson2/DefaultCsrfTokenMixinTests.java | 3 +- .../WebAuthenticationDetailsMixinTests.java | 3 +- 8 files changed, 30 insertions(+), 24 deletions(-) diff --git a/cas/src/test/java/org/springframework/security/cas/jackson2/CasAuthenticationTokenMixinTests.java b/cas/src/test/java/org/springframework/security/cas/jackson2/CasAuthenticationTokenMixinTests.java index 7b1eae8cd7..5dbbd7e9ed 100644 --- a/cas/src/test/java/org/springframework/security/cas/jackson2/CasAuthenticationTokenMixinTests.java +++ b/cas/src/test/java/org/springframework/security/cas/jackson2/CasAuthenticationTokenMixinTests.java @@ -71,8 +71,9 @@ public class CasAuthenticationTokenMixinTests { } ObjectMapper buildObjectMapper() { + ClassLoader loader = getClass().getClassLoader(); ObjectMapper mapper = new ObjectMapper(); - mapper.registerModules(SecurityJacksonModules.getModules()); + mapper.registerModules(SecurityJacksonModules.getModules(loader)); return mapper; } diff --git a/core/src/main/java/org/springframework/security/jackson2/SecurityJacksonModules.java b/core/src/main/java/org/springframework/security/jackson2/SecurityJacksonModules.java index 27603fe97a..9a6f1be73e 100644 --- a/core/src/main/java/org/springframework/security/jackson2/SecurityJacksonModules.java +++ b/core/src/main/java/org/springframework/security/jackson2/SecurityJacksonModules.java @@ -23,7 +23,6 @@ import com.fasterxml.jackson.databind.jsontype.TypeResolverBuilder; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.util.ClassUtils; -import org.springframework.util.ObjectUtils; import java.util.ArrayList; import java.util.Arrays; @@ -63,41 +62,42 @@ public final class SecurityJacksonModules { } public static void enableDefaultTyping(ObjectMapper mapper) { - if(!ObjectUtils.isEmpty(mapper)) { + if(mapper != null) { TypeResolverBuilder typeBuilder = mapper.getDeserializationConfig().getDefaultTyper(null); - if (ObjectUtils.isEmpty(typeBuilder)) { + if (typeBuilder == null) { mapper.enableDefaultTyping(ObjectMapper.DefaultTyping.NON_FINAL, JsonTypeInfo.As.PROPERTY); } } } - private static Module loadAndGetInstance(String className) { + @SuppressWarnings("unchecked") + private static Module loadAndGetInstance(String className, ClassLoader loader) { Module instance = null; try { - logger.debug("Loading module " + className); - Class securityModule = (Class) ClassUtils.forName(className, ClassUtils.getDefaultClassLoader()); - if (!ObjectUtils.isEmpty(securityModule)) { - logger.debug("Loaded module " + className + ", now registering"); + Class securityModule = (Class) ClassUtils.forName(className, loader); + if (securityModule != null) { + if(logger.isDebugEnabled()) { + logger.debug("Loaded module " + className + ", now registering"); + } instance = securityModule.newInstance(); } - } catch (ClassNotFoundException e) { - logger.warn("Module class not found : " + e.getMessage()); - } catch (InstantiationException e) { - logger.error(e.getMessage()); - } catch (IllegalAccessException e) { - logger.error(e.getMessage()); + } catch (Exception e) { + if(logger.isDebugEnabled()) { + logger.debug("Cannot load module " + className, e); + } } return instance; } /** + * @param loader the ClassLoader to use * @return List of available security modules in classpath. */ - public static List getModules() { + public static List getModules(ClassLoader loader) { List modules = new ArrayList(); for (String className : securityJackson2ModuleClasses) { - Module module = loadAndGetInstance(className); - if (!ObjectUtils.isEmpty(module)) { + Module module = loadAndGetInstance(className, loader); + if (module != null) { modules.add(module); } } diff --git a/core/src/test/java/org/springframework/security/jackson2/AbstractMixinTests.java b/core/src/test/java/org/springframework/security/jackson2/AbstractMixinTests.java index 8b78118d3e..6b4c53e40b 100644 --- a/core/src/test/java/org/springframework/security/jackson2/AbstractMixinTests.java +++ b/core/src/test/java/org/springframework/security/jackson2/AbstractMixinTests.java @@ -37,7 +37,8 @@ public abstract class AbstractMixinTests { protected ObjectMapper buildObjectMapper() { if (ObjectUtils.isEmpty(mapper)) { mapper = new ObjectMapper(); - mapper.registerModules(SecurityJacksonModules.getModules()); + ClassLoader loader = getClass().getClassLoader(); + mapper.registerModules(SecurityJacksonModules.getModules(loader)); } return mapper; } diff --git a/web/src/main/java/org/springframework/security/web/jackson2/WebJackson2Module.java b/web/src/main/java/org/springframework/security/web/jackson2/WebJackson2Module.java index 197537e865..3d8dbe6b12 100644 --- a/web/src/main/java/org/springframework/security/web/jackson2/WebJackson2Module.java +++ b/web/src/main/java/org/springframework/security/web/jackson2/WebJackson2Module.java @@ -37,7 +37,7 @@ import javax.servlet.http.Cookie; * ObjectMapper mapper = new ObjectMapper(); * mapper.registerModule(new WebJackson2Module()); * - * Note: use {@link SecurityJacksonModules#getModules()} to get list of all security modules. + * Note: use {@link SecurityJacksonModules#getModules(ClassLoader)} to get list of all security modules. * * @author Jitendra Singh * @see SecurityJacksonModules diff --git a/web/src/test/java/org/springframework/security/web/jackson2/AbstractMixinTests.java b/web/src/test/java/org/springframework/security/web/jackson2/AbstractMixinTests.java index 7523385a3c..7ae4aa249e 100644 --- a/web/src/test/java/org/springframework/security/web/jackson2/AbstractMixinTests.java +++ b/web/src/test/java/org/springframework/security/web/jackson2/AbstractMixinTests.java @@ -34,7 +34,8 @@ public abstract class AbstractMixinTests { protected ObjectMapper buildObjectMapper() { if (ObjectUtils.isEmpty(mapper)) { mapper = new ObjectMapper(); - mapper.registerModules(SecurityJacksonModules.getModules()); + ClassLoader loader = getClass().getClassLoader(); + mapper.registerModules(SecurityJacksonModules.getModules(loader)); } return mapper; } diff --git a/web/src/test/java/org/springframework/security/web/jackson2/CookieMixinTests.java b/web/src/test/java/org/springframework/security/web/jackson2/CookieMixinTests.java index 251f26e3f3..1be162aeb4 100644 --- a/web/src/test/java/org/springframework/security/web/jackson2/CookieMixinTests.java +++ b/web/src/test/java/org/springframework/security/web/jackson2/CookieMixinTests.java @@ -39,7 +39,8 @@ public class CookieMixinTests { ObjectMapper buildObjectMapper() { ObjectMapper mapper = new ObjectMapper(); - mapper.registerModules(SecurityJacksonModules.getModules()); + ClassLoader loader = getClass().getClassLoader(); + mapper.registerModules(SecurityJacksonModules.getModules(loader)); return mapper; } diff --git a/web/src/test/java/org/springframework/security/web/jackson2/DefaultCsrfTokenMixinTests.java b/web/src/test/java/org/springframework/security/web/jackson2/DefaultCsrfTokenMixinTests.java index 4944173bc5..22f8dec580 100644 --- a/web/src/test/java/org/springframework/security/web/jackson2/DefaultCsrfTokenMixinTests.java +++ b/web/src/test/java/org/springframework/security/web/jackson2/DefaultCsrfTokenMixinTests.java @@ -45,7 +45,8 @@ public class DefaultCsrfTokenMixinTests { @Before public void setup() { objectMapper = new ObjectMapper(); - objectMapper.registerModules(SecurityJacksonModules.getModules()); + ClassLoader loader = getClass().getClassLoader(); + objectMapper.registerModules(SecurityJacksonModules.getModules(loader)); defaultCsrfTokenJson = "{\"@class\": \"org.springframework.security.web.csrf.DefaultCsrfToken\", " + "\"headerName\": \"csrf-header\", \"parameterName\": \"_csrf\", \"token\": \"1\"}"; } diff --git a/web/src/test/java/org/springframework/security/web/jackson2/WebAuthenticationDetailsMixinTests.java b/web/src/test/java/org/springframework/security/web/jackson2/WebAuthenticationDetailsMixinTests.java index 9cc8d54b44..0e6a011ffc 100644 --- a/web/src/test/java/org/springframework/security/web/jackson2/WebAuthenticationDetailsMixinTests.java +++ b/web/src/test/java/org/springframework/security/web/jackson2/WebAuthenticationDetailsMixinTests.java @@ -48,7 +48,8 @@ public class WebAuthenticationDetailsMixinTests { @Before public void setup() { this.mapper = new ObjectMapper(); - this.mapper.registerModules(SecurityJacksonModules.getModules()); + ClassLoader loader = getClass().getClassLoader(); + this.mapper.registerModules(SecurityJacksonModules.getModules(loader)); } @Test