geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kl...@apache.org
Subject [28/46] geode git commit: GEODE-2136: Don't duplicate cookies in the http response
Date Mon, 12 Dec 2016 22:36:31 GMT
GEODE-2136: Don't duplicate cookies in the http response

We had some code that copied cookies from the request to the response.
That caused us to include a potentially stale cookie value in the
response.

Adding a unit test that we don't screw up the users cookies. I had to
bring in a dependency on httpunit, because the HttpTester with jetty is
not correctly parsing multiple Set-Cookie headers.


Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/03715a63
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/03715a63
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/03715a63

Branch: refs/heads/feature/GEODE-1930
Commit: 03715a63eee5f20453f0dc0ec01311b11d7548af
Parents: 1fabe49
Author: Dan Smith <upthewaterspout@apache.org>
Authored: Tue Nov 22 10:53:11 2016 -0800
Committer: Dan Smith <upthewaterspout@apache.org>
Committed: Tue Nov 22 11:37:14 2016 -0800

----------------------------------------------------------------------
 extensions/geode-modules-session/build.gradle   |  3 ++
 .../session/filter/SessionCachingFilter.java    | 12 -----
 .../SessionReplicationIntegrationJUnitTest.java | 52 ++++++++++++++++++++
 3 files changed, 55 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/03715a63/extensions/geode-modules-session/build.gradle
----------------------------------------------------------------------
diff --git a/extensions/geode-modules-session/build.gradle b/extensions/geode-modules-session/build.gradle
index f068dde..3063772 100644
--- a/extensions/geode-modules-session/build.gradle
+++ b/extensions/geode-modules-session/build.gradle
@@ -27,6 +27,9 @@ dependencies {
   }
   testCompile(group: 'org.eclipse.jetty', name: 'jetty-http', version: project.'jetty.version',
classifier: 'tests')
   testCompile(group: 'org.eclipse.jetty', name: 'jetty-servlet', version: project.'jetty.version',
classifier: 'tests')
+  testCompile ('org.httpunit:httpunit:' + project.'httpunit.version') {
+    exclude group: 'javax.servlet'
+  }
 
   testCompile project(path: ':geode-junit')
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/03715a63/extensions/geode-modules-session/src/main/java/org/apache/geode/modules/session/filter/SessionCachingFilter.java
----------------------------------------------------------------------
diff --git a/extensions/geode-modules-session/src/main/java/org/apache/geode/modules/session/filter/SessionCachingFilter.java
b/extensions/geode-modules-session/src/main/java/org/apache/geode/modules/session/filter/SessionCachingFilter.java
index 71aa768..25e22bb 100644
--- a/extensions/geode-modules-session/src/main/java/org/apache/geode/modules/session/filter/SessionCachingFilter.java
+++ b/extensions/geode-modules-session/src/main/java/org/apache/geode/modules/session/filter/SessionCachingFilter.java
@@ -224,19 +224,7 @@ public class SessionCachingFilter implements Filter {
 
       Cookie cookie = new Cookie(manager.getSessionCookieName(), session.getId());
       cookie.setPath("".equals(getContextPath()) ? "/" : getContextPath());
-      // Clear out all old cookies and just set ours
       response.addCookie(cookie);
-
-      // Replace all other cookies which aren't JSESSIONIDs
-      if (cookies != null) {
-        for (Cookie c : cookies) {
-          if (manager.getSessionCookieName().equals(c.getName())) {
-            continue;
-          }
-          response.addCookie(c);
-        }
-      }
-
     }
 
     private String getCookieString(Cookie c) {

http://git-wip-us.apache.org/repos/asf/geode/blob/03715a63/extensions/geode-modules-session/src/test/java/org/apache/geode/modules/session/internal/filter/SessionReplicationIntegrationJUnitTest.java
----------------------------------------------------------------------
diff --git a/extensions/geode-modules-session/src/test/java/org/apache/geode/modules/session/internal/filter/SessionReplicationIntegrationJUnitTest.java
b/extensions/geode-modules-session/src/test/java/org/apache/geode/modules/session/internal/filter/SessionReplicationIntegrationJUnitTest.java
index 76ecab3..50bc0ad 100644
--- a/extensions/geode-modules-session/src/test/java/org/apache/geode/modules/session/internal/filter/SessionReplicationIntegrationJUnitTest.java
+++ b/extensions/geode-modules-session/src/test/java/org/apache/geode/modules/session/internal/filter/SessionReplicationIntegrationJUnitTest.java
@@ -47,6 +47,11 @@ import java.util.concurrent.TimeUnit;
 
 import static org.junit.Assert.*;
 
+import com.meterware.httpunit.GetMethodWebRequest;
+import com.meterware.httpunit.WebConversation;
+import com.meterware.httpunit.WebRequest;
+import com.meterware.httpunit.WebResponse;
+
 /**
  * In-container testing using Jetty. This allows us to test context listener events as well
as
  * dispatching actions.
@@ -386,6 +391,53 @@ public class SessionReplicationIntegrationJUnitTest {
     assertNull(((HttpSession) r.get(cookies.get(0).getValue())).getAttribute("foo"));
   }
 
+  /**
+   * Test that a servlet can modify cookies
+   */
+  @Test
+  public void testUserCanModifyTheirOwnCookie() throws Exception {
+    Callback c = new Callback() {
+      @Override
+      public void call(HttpServletRequest request, HttpServletResponse response)
+          throws IOException {
+        Cookie userCookie = findUserCookie(request.getCookies());
+        if (userCookie == null) {
+          userCookie = new Cookie("myCookie", "0");
+        } else {
+          userCookie =
+              new Cookie("myCookie", Integer.toString(Integer.valueOf(userCookie.getValue())
+ 1));
+        }
+
+        response.addCookie(userCookie);
+        request.getSession().setAttribute("dummy", "value");
+      }
+    };
+
+    tester.setAttribute("callback_1", c);
+    String url = tester.createConnector(true);
+    tester.start();
+
+    WebConversation wc = new WebConversation();
+    WebRequest req = new GetMethodWebRequest(url + "/test/hello");
+    req.setHeaderField("Cookie", "myCookie=" + 5);
+
+    final WebResponse webResponse = wc.getResponse(req);
+    assertEquals("6", webResponse.getNewCookieValue("myCookie"));
+  }
+
+  private Cookie findUserCookie(Cookie[] cookies) {
+    if (cookies == null) {
+      return null;
+    }
+    Cookie userCookie = null;
+    for (Cookie cookie : cookies) {
+      if (cookie.getName().equals("myCookie")) {
+        userCookie = cookie;
+      }
+    }
+    return userCookie;
+  }
+
   // Don't see how to do this currently as the SessionListener needs a full
   // web context to work in.
 


Mime
View raw message