SEC-539: Refactored populateSecurityContextFromSession() to reduce nested blocks and clarify logic.

core/src/main/java/org/acegisecurity/context/HttpSessionContextIntegrationFilter.java

@@ -246,66 +246,76 @@ public class HttpSessionContextIntegrationFilter implements InitializingBean, Fi
 
     /**
      * Extracts the security context from the session (if available) and sets it on SecurityContextHolder.
+     * <p/>
+     * If the session is null, the context object is null or the context object stored in the session
+     * is not an instance of SecurityContext it will generate a new empty context and store this.
      *
      * @param httpSession the session obtained from the request.
      */
     private void populateSecurityContextFromSession(HttpSession httpSession) throws ServletException {
-        if (httpSession != null) {
+        if (httpSession == null) {
-
-            Object contextFromSessionObject = httpSession.getAttribute(ACEGI_SECURITY_CONTEXT_KEY);
-
-            if (contextFromSessionObject != null) {
-                // Clone if required (see SEC-356)
-                if (cloneFromHttpSession) {
-                    Assert.isInstanceOf(Cloneable.class, contextFromSessionObject,
-                            "Context must implement Clonable and provide a Object.clone() method");
-                    try {
-                        Method m = contextFromSessionObject.getClass().getMethod("clone", new Class[]{});
-                        if (!m.isAccessible()) {
-                            m.setAccessible(true);
-                        }
-                        contextFromSessionObject = m.invoke(contextFromSessionObject, new Object[]{});
-                    }
-                    catch (Exception ex) {
-                        ReflectionUtils.handleReflectionException(ex);
-                    }
-                }
-
-                if (contextFromSessionObject instanceof SecurityContext) {
-                    if (logger.isDebugEnabled()) {
-                        logger.debug("Obtained from ACEGI_SECURITY_CONTEXT a valid SecurityContext and "
-                                + "set to SecurityContextHolder: '" + contextFromSessionObject + "'");
-                    }
-
-                    SecurityContextHolder.setContext((SecurityContext) contextFromSessionObject);
-                } else {
-                    if (logger.isWarnEnabled()) {
-                        logger
-                                .warn("ACEGI_SECURITY_CONTEXT did not contain a SecurityContext but contained: '"
-                                        + contextFromSessionObject
-                                        + "'; are you improperly modifying the HttpSession directly "
-                                        + "(you should always use SecurityContextHolder) or using the HttpSession attribute "
-                                        + "reserved for this class? - new SecurityContext instance associated with "
-                                        + "SecurityContextHolder");
-                    }
-
-                    SecurityContextHolder.setContext(generateNewContext());
-                }
-            } else {
-                if (logger.isDebugEnabled()) {
-                    logger.debug("HttpSession returned null object for ACEGI_SECURITY_CONTEXT - new "
-                            + "SecurityContext instance associated with SecurityContextHolder");
-                }
-
-                SecurityContextHolder.setContext(generateNewContext());
-            }
-        } else {
             if (logger.isDebugEnabled()) {
                 logger.debug("No HttpSession currently exists - new SecurityContext instance "
                         + "associated with SecurityContextHolder");
             }
 
             SecurityContextHolder.setContext(generateNewContext());
+
+            return;
+        }
+
+        // Session exists, so try to obtain a context from it.
+
+        Object contextFromSessionObject = httpSession.getAttribute(ACEGI_SECURITY_CONTEXT_KEY);
+
+        if (contextFromSessionObject == null) {
+            if (logger.isDebugEnabled()) {
+                logger.debug("HttpSession returned null object for ACEGI_SECURITY_CONTEXT - new "
+                        + "SecurityContext instance associated with SecurityContextHolder");
+            }
+
+            SecurityContextHolder.setContext(generateNewContext());
+
+            return;
+        }
+
+        // We now have the security context object from the session.
+
+        // Clone if required (see SEC-356)
+        if (cloneFromHttpSession) {
+            Assert.isInstanceOf(Cloneable.class, contextFromSessionObject,
+                    "Context must implement Clonable and provide a Object.clone() method");
+            try {
+                Method m = contextFromSessionObject.getClass().getMethod("clone", new Class[]{});
+                if (!m.isAccessible()) {
+                    m.setAccessible(true);
+                }
+                contextFromSessionObject = m.invoke(contextFromSessionObject, new Object[]{});
+            }
+            catch (Exception ex) {
+                ReflectionUtils.handleReflectionException(ex);
+            }
+        }
+
+        if (contextFromSessionObject instanceof SecurityContext) {
+            if (logger.isDebugEnabled()) {
+                logger.debug("Obtained from ACEGI_SECURITY_CONTEXT a valid SecurityContext and "
+                        + "set to SecurityContextHolder: '" + contextFromSessionObject + "'");
+            }
+
+            SecurityContextHolder.setContext((SecurityContext) contextFromSessionObject);
+        } else {
+            if (logger.isWarnEnabled()) {
+                logger
+                        .warn("ACEGI_SECURITY_CONTEXT did not contain a SecurityContext but contained: '"
+                                + contextFromSessionObject
+                                + "'; are you improperly modifying the HttpSession directly "
+                                + "(you should always use SecurityContextHolder) or using the HttpSession attribute "
+                                + "reserved for this class? - new SecurityContext instance associated with "
+                                + "SecurityContextHolder");
+            }
+
+            SecurityContextHolder.setContext(generateNewContext());
         }
     }