Revision: 65f9d43bba9c
Author: Dhanji R. Prasanna <[email protected]>
Date: Wed May 21 03:26:50 2014 UTC
Log: Snapshot cookies in continuing request to prevent mutation side
effects
http://code.google.com/p/google-guice/source/detail?r=65f9d43bba9c
Modified:
/extensions/servlet/src/com/google/inject/servlet/ContinuingHttpServletRequest.java
/extensions/servlet/test/com/google/inject/servlet/ContinuingHttpServletRequestTest.java
=======================================
---
/extensions/servlet/src/com/google/inject/servlet/ContinuingHttpServletRequest.java
Thu May 16 01:59:34 2013 UTC
+++
/extensions/servlet/src/com/google/inject/servlet/ContinuingHttpServletRequest.java
Wed May 21 03:26:50 2014 UTC
@@ -38,9 +38,24 @@
// We clear out the attributes as they are mutable and not thread-safe.
private final Map<String, Object> attributes = Maps.newHashMap();
+ private final Cookie[] cookies;
public ContinuingHttpServletRequest(HttpServletRequest request) {
super(request);
+
+ Cookie[] originalCookies = request.getCookies();
+ if (originalCookies != null) {
+ int numberOfCookies = originalCookies.length;
+ cookies = new Cookie[numberOfCookies];
+ for (int i = 0; i < numberOfCookies; i++) {
+ Cookie originalCookie = originalCookies[i];
+
+ // Snapshot each cookie + freeze.
+ cookies[i] = new ImmutableCookie(originalCookie);
+ }
+ } else {
+ cookies = null;
+ }
}
@Override public HttpSession getSession() {
@@ -68,10 +83,60 @@
}
@Override public Cookie[] getCookies() {
- if (super.getCookies() == null) {
- return null;
+ // NOTE(dhanji): Cookies themselves are mutable. However a
ContinuingHttpServletRequest
+ // snapshots the original set of cookies it received and imprisons
them in immutable
+ // form. Unfortunately, the cookie array itself is mutable and there
is no way for us
+ // to avoid this. At worst, however, mutation effects are restricted
within the scope
+ // of a single request. Continued requests are not affected after
snapshot time.
+ return cookies;
+ }
+
+ private static final class ImmutableCookie extends Cookie {
+ public ImmutableCookie(Cookie original) {
+ super(original.getName(), original.getValue());
+
+ // No copy is required if this is a snapshot of a snapshot.
+ if (original instanceof ImmutableCookie) {
+ return;
+ }
+ super.setMaxAge(original.getMaxAge());
+ super.setPath(original.getPath());
+ super.setComment(original.getComment());
+ super.setSecure(original.getSecure());
+ super.setValue(original.getValue());
+ super.setVersion(original.getVersion());
+
+ if (original.getDomain() != null) {
+ super.setDomain(original.getDomain());
+ }
+ }
+
+ @Override public void setComment(String purpose) {
+ throw new UnsupportedOperationException("Cannot modify cookies on a
continued request");
+ }
+
+ @Override public void setDomain(String pattern) {
+ throw new UnsupportedOperationException("Cannot modify cookies on a
continued request");
+ }
+
+ @Override public void setMaxAge(int expiry) {
+ throw new UnsupportedOperationException("Cannot modify cookies on a
continued request");
+ }
+
+ @Override public void setPath(String uri) {
+ throw new UnsupportedOperationException("Cannot modify cookies on a
continued request");
+ }
+
+ @Override public void setSecure(boolean flag) {
+ throw new UnsupportedOperationException("Cannot modify cookies on a
continued request");
+ }
+
+ @Override public void setValue(String newValue) {
+ throw new UnsupportedOperationException("Cannot modify cookies on a
continued request");
+ }
+
+ @Override public void setVersion(int v) {
+ throw new UnsupportedOperationException("Cannot modify cookies on a
continued request");
}
- // TODO(dhanji): Cookies themselves are mutable. Is this a problem?
- return super.getCookies().clone();
}
}
=======================================
---
/extensions/servlet/test/com/google/inject/servlet/ContinuingHttpServletRequestTest.java
Thu May 16 01:59:34 2013 UTC
+++
/extensions/servlet/test/com/google/inject/servlet/ContinuingHttpServletRequestTest.java
Wed May 21 03:26:50 2014 UTC
@@ -15,6 +15,7 @@
*/
package com.google.inject.servlet;
+import junit.framework.AssertionFailedError;
import static org.easymock.EasyMock.createMock;
import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.replay;
@@ -50,9 +51,47 @@
replay(delegate);
- assertTrue(Arrays.equals(cookies,
- new ContinuingHttpServletRequest(delegate).getCookies()));
+ ContinuingHttpServletRequest continuingRequest = new
ContinuingHttpServletRequest(
+ delegate);
+
+ assertCookieArraysEqual(cookies, continuingRequest.getCookies());
+
+ // Now mutate the original cookies, this shouldnt be reflected in the
continued request.
+ cookies[0].setValue("INVALID");
+ cookies[1].setValue("INVALID");
+
+ try {
+ assertCookieArraysEqual(cookies, continuingRequest.getCookies());
+ fail();
+ } catch (AssertionFailedError e) {
+ // Expected.
+ }
+
+ // Perform a snapshot of the snapshot.
+ ContinuingHttpServletRequest furtherContinuingRequest = new
ContinuingHttpServletRequest(
+ continuingRequest);
+
+ // The cookies should be fixed.
+ assertCookieArraysEqual(continuingRequest.getCookies(),
furtherContinuingRequest.getCookies());
verify(delegate);
}
+
+ private static void assertCookieArraysEqual(Cookie[] one, Cookie[] two) {
+ assertEquals(one.length, two.length);
+ for (int i = 0; i < one.length; i++) {
+ Cookie cookie = one[i];
+ assertCookiequality(cookie, two[i]);
+ }
+ }
+
+ private static void assertCookiequality(Cookie one, Cookie two) {
+ assertEquals(one.getName(), two.getName());
+ assertEquals(one.getComment(), two.getComment());
+ assertEquals(one.getDomain(), two.getDomain());
+ assertEquals(one.getPath(), two.getPath());
+ assertEquals(one.getValue(), two.getValue());
+ assertEquals(one.getMaxAge(), two.getMaxAge());
+ assertEquals(one.getSecure(), two.getSecure());
+ }
}
--
You received this message because you are subscribed to the Google Groups
"google-guice-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/google-guice-dev.
For more options, visit https://groups.google.com/d/optout.
