You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by kl...@apache.org on 2016/12/12 22:36:31 UTC
[28/46] geode git commit: GEODE-2136: Don't duplicate cookies in the
http response
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 <up...@apache.org>
Authored: Tue Nov 22 10:53:11 2016 -0800
Committer: Dan Smith <up...@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.