[java-xmltooling COMMIT] in /branches/REL_1: doc/RELEASE-NOTES.txt src/main/java/org/opensaml/xml/util/IndexingObject...

[noreply at shibboleth.net]

Author: putmanb
Date: Wed Jun 11 19:25:40 2014
New Revision: 823

URL: http://svn.shibboleth.net/view/java-xmltooling?rev=823&view=rev
Log:
JXT-111: IndexingObjectStore uses hashCode() in an unsafe manner.
Also fix size() and isEmpty() public methods to use read lock to avoid race condition.

Modified:
    branches/REL_1/doc/RELEASE-NOTES.txt
    branches/REL_1/src/main/java/org/opensaml/xml/util/IndexingObjectStore.java
    branches/REL_1/src/test/java/org/opensaml/xml/util/IndexingObjectStoreTest.java

Modified: branches/REL_1/doc/RELEASE-NOTES.txt
URL: http://svn.shibboleth.net/view/java-xmltooling/branches/REL_1/doc/RELEASE-NOTES.txt?rev=823&r1=822&r2=823&view=diff
==============================================================================
--- branches/REL_1/doc/RELEASE-NOTES.txt (original)
+++ branches/REL_1/doc/RELEASE-NOTES.txt Wed Jun 11 19:25:40 2014
@@ -3,6 +3,7 @@
 [JXT-108] - Encrypter setKeyResolverCriteria() typo, should be getKeyResolverCriteria()
 [JXT-109] - SimpleKeyInfoReferenceEncryptedKeyResolver constructor missing 'public' modifier 
 [JXT-110] - SecurityHelper derivePublicKey incorrectly derives public key for DSAPrivateKey 
+[JXT-111] - IndexingObjectStore uses hashCode() in an unsafe manner
 
 Changes in Release 1.4.1
 =============================================

Modified: branches/REL_1/src/main/java/org/opensaml/xml/util/IndexingObjectStore.java
URL: http://svn.shibboleth.net/view/java-xmltooling/branches/REL_1/src/main/java/org/opensaml/xml/util/IndexingObjectStore.java?rev=823&r1=822&r2=823&view=diff
==============================================================================
--- branches/REL_1/src/main/java/org/opensaml/xml/util/IndexingObjectStore.java (original)
+++ branches/REL_1/src/main/java/org/opensaml/xml/util/IndexingObjectStore.java Wed Jun 11 19:25:40 2014
@@ -25,20 +25,29 @@
 import net.jcip.annotations.ThreadSafe;
 
 /**
- * This class is used to store instances of objects that may be created independently but are, in face, the same object.
- * For example, {@link org.opensaml.xml.signature.KeyInfo}s contain keys, certs, and CRLs. Multiple unique instances of
+ * <p>
+ * This class is used to store instances of objects that may be created independently but are, in fact, the same object.
+ * For example, KeyInfo XML structures contain keys, certs, and CRLs. Multiple unique instances of
  * a KeyInfo may contain, and separately construct, the exact same cert. KeyInfo could, therefore, create a class-level
  * instance of this object store and put certs within it. In this manner the cert is only sitting in memory once and
  * each KeyInfo simply stores a reference (index) to stored object.
+ * </p>
  * 
+ * <p>
  * This store uses basic reference counting to keep track of how many of the respective objects are pointing to an
- * entry. Adding an object that already exists, as determined by the objects <code>hashCode()</code> method, simply
+ * entry. Adding an object that already exists, as determined by the object's <code>equals()</code> method, simply
  * increments the reference counter. Removing an object decrements the counter. Only when the counter reaches zero is
  * the object actually freed for garbage collection.
+ * </p>
  * 
- * <strong>Note</strong> the instance of an object returned by {@link #get(String)} need not be the same object as 
- * stored via {@link #put(Object)}.  However, their hash codes will be equal.  Therefore this store should never be 
- * used to store objects that produce identical hash codes but are not functionally identical objects.
+ * <p>
+ * <strong>Note:</strong> the instance of an object returned by {@link #get(String)} need not be the same object as 
+ * stored via {@link #put(Object)}.  However, the instances will be equal according to their <code>equals()</code>.
+ * The indexing and storage is based on use of {@link Map}, so the normal caveats related to use of hash-based
+ * collection types apply: if the stored object's <code>hashCode()</code> and <code>equals()</code> methods are 
+ * implemented based on mutable properties of the object, then those object instance's properties should not 
+ * be mutated while the object is stored, otherwise unpredictable behavior will result.
+ * </p>
  * 
  * @param <T> type of object being stored
  */
@@ -50,11 +59,19 @@
 
     /** Backing object data store. */
     private Map<String, StoredObjectWrapper> objectStore;
+    
+    /** Map of object instances to the index value used to reference them externally. */
+    private Map<T, Integer> indexStore;
+    
+    /** The last index sequence used. */
+    private int lastIndex;
 
     /** Constructor. */
     public IndexingObjectStore() {
         rwLock = new ReentrantReadWriteLock();
         objectStore = new LazyMap<String, StoredObjectWrapper>();
+        indexStore = new LazyMap<T, Integer>();
+        lastIndex = 0;
     }
 
     /** Clears the object store. */
@@ -63,6 +80,7 @@
         writeLock.lock();
         try {
             objectStore.clear();
+            indexStore.clear();
         } finally {

[... 115 lines stripped ...]