Rolled back unnecessary changes (whitespace, imports etc) for SEC-398 to make actual change from revision 1858 clearer.

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

@@ -32,7 +32,6 @@ import org.apache.commons.logging.LogFactory;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.util.Assert;
 import org.springframework.util.ReflectionUtils;
-import org.springframework.web.util.WebUtils;
 
 /**
  * Populates the {@link SecurityContextHolder} with information obtained from
@@ -97,13 +96,11 @@ import org.springframework.web.util.WebUtils;
  * @version $Id: HttpSessionContextIntegrationFilter.java 1784 2007-02-24
  * 21:00:24Z luke_t $
  */
-public class HttpSessionContextIntegrationFilter implements InitializingBean,
+public class HttpSessionContextIntegrationFilter implements InitializingBean, Filter {
-		Filter {
 	// ~ Static fields/initializers
 	// =====================================================================================
 
-	protected static final Log logger = LogFactory
+	protected static final Log logger = LogFactory.getLog(HttpSessionContextIntegrationFilter.class);
-			.getLog(HttpSessionContextIntegrationFilter.class);
 
 	static final String FILTER_APPLIED = "__acegi_session_integration_filter_applied";
 
@@ -175,16 +172,13 @@ public class HttpSessionContextIntegrationFilter implements InitializingBean,
 	// ========================================================================================================
 
 	public void afterPropertiesSet() throws Exception {
-		if ((this.context == null)
+		if ((this.context == null) || (!SecurityContext.class.isAssignableFrom(this.context))) {
-				|| (!SecurityContext.class.isAssignableFrom(this.context))) {
+			throw new IllegalArgumentException("context must be defined and implement SecurityContext "
-			throw new IllegalArgumentException(
+					+ "(typically use org.acegisecurity.context.SecurityContextImpl; existing class is " + this.context
-					"context must be defined and implement SecurityContext "
+					+ ")");
-							+ "(typically use org.acegisecurity.context.SecurityContextImpl; existing class is "
-							+ this.context + ")");
 		}
 
-		if ((forceEagerSessionCreation == true)
+		if ((forceEagerSessionCreation == true) && (allowSessionCreation == false)) {
-				&& (allowSessionCreation == false)) {
 			throw new IllegalArgumentException(
 					"If using forceEagerSessionCreation, you must set allowSessionCreation to also be true");
 		}
@@ -196,59 +190,54 @@ public class HttpSessionContextIntegrationFilter implements InitializingBean,
 	public void destroy() {
 	}
 
-	public void doFilter(ServletRequest request, ServletResponse response,
+	public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException,
-			FilterChain chain) throws IOException, ServletException {
+			ServletException {
 		boolean filterApplied = false;
 		if ((request != null) && (request.getAttribute(FILTER_APPLIED) != null)) {
 			// ensure that filter is only applied once per request
 			chain.doFilter(request, response);
-		} else {
+		}
+		else {
 			HttpSession httpSession = null;
 			boolean httpSessionExistedAtStartOfRequest = false;
 
 			try {
-				httpSession = ((HttpServletRequest) request)
+				httpSession = ((HttpServletRequest) request).getSession(forceEagerSessionCreation);
-						.getSession(forceEagerSessionCreation);
+			}
-			} catch (IllegalStateException ignored) {
+			catch (IllegalStateException ignored) {
 			}
 
 			if (httpSession != null) {
 				httpSessionExistedAtStartOfRequest = true;
 
-				Object contextFromSessionObject = httpSession
+				Object contextFromSessionObject = httpSession.getAttribute(ACEGI_SECURITY_CONTEXT_KEY);
-						.getAttribute(ACEGI_SECURITY_CONTEXT_KEY);
 
 				if (contextFromSessionObject != null) {
 					// Clone if required (see SEC-356)
 					if (cloneFromHttpSession) {
-						Assert
+						Assert.isInstanceOf(Cloneable.class, contextFromSessionObject,
-								.isInstanceOf(Cloneable.class,
-										contextFromSessionObject,
 								"Context must implement Clonable and provide a Object.clone() method");
 						try {
-							Method m = contextFromSessionObject.getClass()
+							Method m = contextFromSessionObject.getClass().getMethod("clone", new Class[] {});
-									.getMethod("clone", new Class[] {});
 							if (!m.isAccessible()) {
 								m.setAccessible(true);
 							}
-							contextFromSessionObject = m.invoke(
+							contextFromSessionObject = m.invoke(contextFromSessionObject, new Object[] {});
-									contextFromSessionObject, new Object[] {});
+						}
-						} catch (Exception ex) {
+						catch (Exception ex) {
 							ReflectionUtils.handleReflectionException(ex);
 						}
 					}
 
 					if (contextFromSessionObject instanceof SecurityContext) {
 						if (logger.isDebugEnabled()) {
-							logger
+							logger.debug("Obtained from ACEGI_SECURITY_CONTEXT a valid SecurityContext and "
-									.debug("Obtained from ACEGI_SECURITY_CONTEXT a valid SecurityContext and "
+									+ "set to SecurityContextHolder: '" + contextFromSessionObject + "'");
-											+ "set to SecurityContextHolder: '"
-											+ contextFromSessionObject + "'");
 						}
 
-						SecurityContextHolder
+						SecurityContextHolder.setContext((SecurityContext) contextFromSessionObject);
-								.setContext((SecurityContext) contextFromSessionObject);
+					}
-					} else {
+					else {
 						if (logger.isWarnEnabled()) {
 							logger
 									.warn("ACEGI_SECURITY_CONTEXT did not contain a SecurityContext but contained: '"
@@ -261,36 +250,32 @@ public class HttpSessionContextIntegrationFilter implements InitializingBean,
 
 						SecurityContextHolder.setContext(generateNewContext());
 					}
-				} else {
+				}
+				else {
 					if (logger.isDebugEnabled()) {
-						logger
+						logger.debug("HttpSession returned null object for ACEGI_SECURITY_CONTEXT - new "
-								.debug("HttpSession returned null object for ACEGI_SECURITY_CONTEXT - new "
 								+ "SecurityContext instance associated with SecurityContextHolder");
 					}
 
 					SecurityContextHolder.setContext(generateNewContext());
 				}
-
+			}
-			} else {
+			else {
 				if (logger.isDebugEnabled()) {
-					logger
+					logger.debug("No HttpSession currently exists - new SecurityContext instance "
-							.debug("No HttpSession currently exists - new SecurityContext instance "
 							+ "associated with SecurityContextHolder");
 				}
 
 				SecurityContextHolder.setContext(generateNewContext());
 			}
 
-			// end synch
-
 			// Make the HttpSession null, as we want to ensure we don't keep
 			// a reference to the HttpSession laying around in case the
 			// chain.doFilter() invalidates it.
 			httpSession = null;
 
 			// Proceed with chain
-			int contextWhenChainProceeded = SecurityContextHolder.getContext()
+			int contextWhenChainProceeded = SecurityContextHolder.getContext().hashCode();
-					.hashCode();
 
 			try {
 				filterApplied = true;
@@ -299,28 +284,26 @@ public class HttpSessionContextIntegrationFilter implements InitializingBean,
 			} catch (IOException ioe) {
 				throw ioe;
 			} catch (ServletException se) {
-
 				throw se;
-			} finally {
+            }
+            finally {
 				// do clean up, even if there was an exception
 				// Store context back to HttpSession
 				try {
-					httpSession = ((HttpServletRequest) request)
+					httpSession = ((HttpServletRequest) request).getSession(false);
-							.getSession(false);
+				}
-				} catch (IllegalStateException ignored) {
+				catch (IllegalStateException ignored) {
 				}
 
 				if ((httpSession == null) && httpSessionExistedAtStartOfRequest) {
 					if (logger.isDebugEnabled()) {
-						logger
+						logger.debug("HttpSession is now null, but was not null at start of request; "
-								.debug("HttpSession is now null, but was not null at start of request; "
 								+ "session was invalidated, so do not create a new session");
 					}
 				}
 
 				// Generate a HttpSession only if we need to
-				if ((httpSession == null)
+				if ((httpSession == null) && !httpSessionExistedAtStartOfRequest) {
-						&& !httpSessionExistedAtStartOfRequest) {
 					if (!allowSessionCreation) {
 						if (logger.isDebugEnabled()) {
 							logger
@@ -329,24 +312,23 @@ public class HttpSessionContextIntegrationFilter implements InitializingBean,
 											+ "(because the allowSessionCreation property is false) - SecurityContext thus not "
 											+ "stored for next request");
 						}
-					} else if (!contextObject.equals(SecurityContextHolder
+					}
-							.getContext())) {
+					else if (!contextObject.equals(SecurityContextHolder.getContext())) {
 						if (logger.isDebugEnabled()) {
-							logger
+							logger.debug("HttpSession being created as SecurityContextHolder contents are non-default");
-									.debug("HttpSession being created as SecurityContextHolder contents are non-default");
 						}
 
 						try {
-							httpSession = ((HttpServletRequest) request)
+							httpSession = ((HttpServletRequest) request).getSession(true);
-									.getSession(true);
-						} catch (IllegalStateException ignored) {
 						}
-					} else {
+						catch (IllegalStateException ignored) {
+						}
+					}
+					else {
 						if (logger.isDebugEnabled()) {
 							logger
 									.debug("HttpSession is null, but SecurityContextHolder has not changed from default: ' "
-											+ SecurityContextHolder
+											+ SecurityContextHolder.getContext()
-													.getContext()
 											+ "'; not creating HttpSession or storing SecurityContextHolder contents");
 						}
 					}
@@ -358,12 +340,11 @@ public class HttpSessionContextIntegrationFilter implements InitializingBean,
 				// actually changed (see JIRA SEC-37)
 				if ((httpSession != null)
 						&& (SecurityContextHolder.getContext().hashCode() != contextWhenChainProceeded)) {
-					httpSession.setAttribute(ACEGI_SECURITY_CONTEXT_KEY,
+					httpSession.setAttribute(ACEGI_SECURITY_CONTEXT_KEY, SecurityContextHolder.getContext());
-							SecurityContextHolder.getContext());
 
 					if (logger.isDebugEnabled()) {
-						logger.debug("SecurityContext stored to HttpSession: '"
+						logger.debug("SecurityContext stored to HttpSession: '" + SecurityContextHolder.getContext()
-								+ SecurityContextHolder.getContext() + "'");
+								+ "'");
 					}
 				}
 
@@ -375,22 +356,20 @@ public class HttpSessionContextIntegrationFilter implements InitializingBean,
 				SecurityContextHolder.clearContext();
 
 				if (logger.isDebugEnabled()) {
-					logger
+					logger.debug("SecurityContextHolder set to new context, as request processing completed");
-							.debug("SecurityContextHolder set to new context, as request processing completed");
 				}
-
 			}
-
 		}
-
 	}
 
 	public SecurityContext generateNewContext() throws ServletException {
 		try {
 			return (SecurityContext) this.context.newInstance();
-		} catch (InstantiationException ie) {
+		}
+		catch (InstantiationException ie) {
 			throw new ServletException(ie);
-		} catch (IllegalAccessException iae) {
+		}
+		catch (IllegalAccessException iae) {
 			throw new ServletException(iae);
 		}
 	}
@@ -402,11 +381,9 @@ public class HttpSessionContextIntegrationFilter implements InitializingBean,
 	/**
 	 * Does nothing. We use IoC container lifecycle services instead.
 	 *
-	 * @param filterConfig
+	 * @param filterConfig ignored
-	 *            ignored
 	 *
-	 * @throws ServletException
+	 * @throws ServletException ignored
-	 *             ignored
 	 */
 	public void init(FilterConfig filterConfig) throws ServletException {
 	}