[java-opensaml COMMIT] in /trunk: opensaml-storage-api/src/main/java/org/opensaml/storage/RequestScopedStorageService...

[noreply at shibboleth.net]

Author: scantor
Date: Tue Oct 29 16:19:46 2013
New Revision: 3491

URL: http://svn.shibboleth.net/view/java-opensaml?rev=3491&view=rev
Log:
Fix bugs and race conditions in load/save flow.

Modified:
    trunk/opensaml-storage-api/src/main/java/org/opensaml/storage/RequestScopedStorageService.java
    trunk/opensaml-storage-impl/src/main/java/org/opensaml/storage/impl/JSONRequestScopedStorageService.java
    trunk/opensaml-storage-impl/src/test/java/org/opensaml/storage/impl/JSONRequestScopedStorageServiceTest.java

Modified: trunk/opensaml-storage-api/src/main/java/org/opensaml/storage/RequestScopedStorageService.java
URL: http://svn.shibboleth.net/view/java-opensaml/trunk/opensaml-storage-api/src/main/java/org/opensaml/storage/RequestScopedStorageService.java?rev=3491&r1=3490&r2=3491&view=diff
==============================================================================
--- trunk/opensaml-storage-api/src/main/java/org/opensaml/storage/RequestScopedStorageService.java (original)
+++ trunk/opensaml-storage-api/src/main/java/org/opensaml/storage/RequestScopedStorageService.java Tue Oct 29 16:19:46 2013
@@ -38,6 +38,8 @@
     /**
      * Reconstitute stored data.
      * 
+     * <p>This method must be idempotent with respect to existing state when called more than once.</p> 
+     * 
      * @throws IOException  if an error occurs reconstituting the data
      */
     public void load() throws IOException;

Modified: trunk/opensaml-storage-impl/src/main/java/org/opensaml/storage/impl/JSONRequestScopedStorageService.java
URL: http://svn.shibboleth.net/view/java-opensaml/trunk/opensaml-storage-impl/src/main/java/org/opensaml/storage/impl/JSONRequestScopedStorageService.java?rev=3491&r1=3490&r2=3491&view=diff
==============================================================================
--- trunk/opensaml-storage-impl/src/main/java/org/opensaml/storage/impl/JSONRequestScopedStorageService.java (original)
+++ trunk/opensaml-storage-impl/src/main/java/org/opensaml/storage/impl/JSONRequestScopedStorageService.java Tue Oct 29 16:19:46 2013
@@ -18,6 +18,7 @@
 package org.opensaml.storage.impl;
 
 import java.io.IOException;
+import java.io.PrintWriter;
 import java.io.StringReader;
 import java.io.StringWriter;
 import java.util.Arrays;
@@ -41,11 +42,13 @@
 import javax.servlet.FilterChain;
 import javax.servlet.FilterConfig;
 import javax.servlet.ServletException;
+import javax.servlet.ServletOutputStream;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpServletResponseWrapper;
 
 import net.shibboleth.utilities.java.support.annotation.constraint.Live;
 import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
@@ -82,11 +85,11 @@
     implements RequestScopedStorageService, Filter {
 
     /** Name of request attribute for context map. */
-    @Nonnull private static final String CONTEXT_MAP_ATTRIBUTE = 
+    @Nonnull protected static final String CONTEXT_MAP_ATTRIBUTE = 
             "org.opensaml.storage.impl.JSONRequestScopedStorageService.contextMap";
 
     /** Name of request attribute used as a dirty bit. */
-    @Nonnull private static final String DIRTY_BIT_ATTRIBUTE =
+    @Nonnull protected static final String DIRTY_BIT_ATTRIBUTE =
             "org.opensaml.storage.impl.JSONRequestScopedStorageService.dirty";
 
     /** Default cookie name for storage tracking. */
@@ -228,9 +231,18 @@
 
     /** {@inheritDoc} */
     public void load() throws IOException {
+        
+        Map<String,Map<String,MutableStorageRecord>> contextMap = getContextMap();
+        
+        // Check for recursion. If load() is called directly, the above getter will
+        // call us, which means we need to short-circuit the "outer" load call by
+        // detecting that data has been loaded already.
+        if (!contextMap.isEmpty()) {
+            return;
+        }
+        
         log.trace("Loading storage state from cookie in current request");
         
-        getContextMap().clear();
         setDirty(false);
         
         // Search for our cookie.
@@ -263,7 +275,7 @@
             
             for (Map.Entry<String,JsonValue> context : obj.entrySet()) {
                 if (context.getValue().getValueType() != JsonValue.ValueType.OBJECT) {
-                    getContextMap().clear();
+                    contextMap.clear();
                     throw new IOException("Found invalid data structure while parsing context map");
                 }
                 
@@ -281,7 +293,7 @@
             }
             setDirty(false);
         } catch (NullPointerException | ClassCastException | ArithmeticException | JsonException e) {
-            getContextMap().clear();
+            contextMap.clear();
             setDirty(false);
             log.error("Exception while parsing context map", e);
             throw new IOException("Found invalid data structure while parsing context map", e);
@@ -375,10 +387,12 @@

[... 127 lines stripped ...]