From d5484e15ca5175e853c4dce0f8218d7583df82eb Mon Sep 17 00:00:00 2001
From: Rob Winch <rwinch@gopivotal.com>
Date: Tue, 28 Jul 2015 14:58:51 -0500
Subject: [PATCH] CookieHttpSessionStrategy only writes same Session once

If SessionRepositoryRequestWrapper.commitSession() is invoked twice
when a new session is created, then CookieHttpSessionStrategy will
add the same cookie twice. A couple examples of how this could happen:

* The response is committed and
  SessionRepositoryResponseWrapper.onResponseCommitted() invokes
  SessionRepositoryRequestWrapper.commitSession(). Then the finally
  block in SessionRepositoryFilter invokes
  SessionRepositoryRequestWrapper.commitSession() again.
* The new session is initialized and an Exception is thrown (i.e.
  gh-229). The SessionRepositoryFilter invokes
  SessionRepositoryRequestWrapper.commitSession() in the REQUEST
  dispatch. Then in the ERROR dispatch SessionRepositoryFilter invokes
  SessionRepositoryRequestWrapper.commitSession() invokes it again.

This commit ensures if the same Session is passed into
CookieHttpSessionStrategy multiple times within the same HttpServletRequest
it is only written once by keeping track of the sessions on a request
attribute.

Fixes gh-251
---
 .../web/http/CookieHttpSessionStrategy.java   | 19 ++++++++++++++++
 .../http/CookieHttpSessionStrategyTests.java  | 22 +++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/spring-session/src/main/java/org/springframework/session/web/http/CookieHttpSessionStrategy.java b/spring-session/src/main/java/org/springframework/session/web/http/CookieHttpSessionStrategy.java
index 439ad3b..f5d8d3c 100644
--- a/spring-session/src/main/java/org/springframework/session/web/http/CookieHttpSessionStrategy.java
+++ b/spring-session/src/main/java/org/springframework/session/web/http/CookieHttpSessionStrategy.java
@@ -17,6 +17,7 @@ package org.springframework.session.web.http;
 
 import java.io.UnsupportedEncodingException;
 import java.net.URLEncoder;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Set;
@@ -152,6 +153,8 @@ import org.springframework.session.Session;
  * @author Rob Winch
  */
 public final class CookieHttpSessionStrategy implements MultiHttpSessionStrategy, HttpSessionManager {
+	private static final String SESSION_IDS_WRITTEN_ATTR = CookieHttpSessionStrategy.class.getName().concat(".SESSIONS_WRITTEN_ATTR");
+
 	static final String DEFAULT_ALIAS = "0";
 
 	static final String DEFAULT_SESSION_ALIAS_PARAM_NAME = "_s";
@@ -208,6 +211,12 @@ public final class CookieHttpSessionStrategy implements MultiHttpSessionStrategy
 	}
 
 	public void onNewSession(Session session, HttpServletRequest request, HttpServletResponse response) {
+		Set<String> sessionIdsWritten = getSessionIdsWritten(request);
+		if(sessionIdsWritten.contains(session.getId())) {
+			return;
+		}
+		sessionIdsWritten.add(session.getId());
+
 		Map<String,String> sessionIds = getSessionIds(request);
 		String sessionAlias = getCurrentSessionAlias(request);
 		sessionIds.put(sessionAlias, session.getId());
@@ -215,6 +224,16 @@ public final class CookieHttpSessionStrategy implements MultiHttpSessionStrategy
 		response.addCookie(sessionCookie);
 	}
 
+	@SuppressWarnings("unchecked")
+	private Set<String> getSessionIdsWritten(HttpServletRequest request) {
+		Set<String> sessionsWritten = (Set<String>) request.getAttribute(SESSION_IDS_WRITTEN_ATTR);
+		if(sessionsWritten == null) {
+			sessionsWritten = new HashSet<String>();
+			request.setAttribute(SESSION_IDS_WRITTEN_ATTR, sessionsWritten);
+		}
+		return sessionsWritten;
+	}
+
 	private Cookie createSessionCookie(HttpServletRequest request,
 			Map<String, String> sessionIds) {
 		Cookie sessionCookie = new Cookie(cookieName,"");
diff --git a/spring-session/src/test/java/org/springframework/session/web/http/CookieHttpSessionStrategyTests.java b/spring-session/src/test/java/org/springframework/session/web/http/CookieHttpSessionStrategyTests.java
index 1cb33f3..1a0ab5d 100644
--- a/spring-session/src/test/java/org/springframework/session/web/http/CookieHttpSessionStrategyTests.java
+++ b/spring-session/src/test/java/org/springframework/session/web/http/CookieHttpSessionStrategyTests.java
@@ -68,6 +68,28 @@ public class CookieHttpSessionStrategyTests {
 		assertThat(getSessionId()).isEqualTo(session.getId());
 	}
 
+	@Test
+	public void onNewSessionTwiceSameId() throws Exception {
+		strategy.onNewSession(session, request, response);
+		strategy.onNewSession(session, request, response);
+
+		assertThat(response.getCookies()).hasSize(1);
+	}
+
+	@Test
+	public void onNewSessionTwiceNewId() throws Exception {
+		Session newSession = new MapSession();
+
+		strategy.onNewSession(session, request, response);
+		strategy.onNewSession(newSession, request, response);
+
+		Cookie[] cookies = response.getCookies();
+		assertThat(cookies).hasSize(2);
+
+		assertThat(cookies[0].getValue()).isEqualTo(session.getId());
+		assertThat(cookies[1].getValue()).isEqualTo(newSession.getId());
+	}
+
 	@Test
 	public void onNewSessionExistingSessionSameAlias() throws Exception {
 		Session existing = new MapSession();