Bugfix: Nested collection replacment

.github/workflows/ci.yml

@@ -20,9 +20,10 @@ jobs:
     steps:
       - uses: actions/checkout@v4
       - name: Set up JDK
-        uses: oracle-actions/setup-java@v1
+        uses: actions/setup-java@v4.7.0
         with:
-          release: ${{ matrix.java }}
+          distribution: oracle
+          java-version: ${{ matrix.java }}
       - name: Install softhsm2
         run: sudo apt-get install -y softhsm2
       - name: Install opensc

api/src/main/java/io/jsonwebtoken/lang/Assert.java

@@ -347,6 +347,33 @@ public static void notEmpty(Map map) {
         notEmpty(map, "[Assertion failed] - this map must not be empty; it must contain at least one entry");
     }
 
+    /**
+     * Assert that an Object is not <code>null</code> or empty.
+     * <pre class="code">Assert.notEmpty(object, "Object cannot be null or empty");</pre>
+     *
+     * @param object  the object to check
+     * @param message the exception message to use if the assertion fails
+     * @return the non-null, non-empty object
+     * @throws IllegalArgumentException if the object is <code>null</code> or empty
+     */
+    public static Object notEmpty(Object object, String message) {
+        if (Objects.isEmpty(object)) {
+            throw new IllegalArgumentException(message);
+        }
+        return object;
+    }
+
+    /**
+     * Assert that an Object is not <code>null</code> or empty.
+     * <pre class="code">Assert.notEmpty(object);</pre>
+     *
+     * @param object the object to check
+     * @throws IllegalArgumentException if the object is <code>null</code> or empty
+     */
+    public static void notEmpty(Object object) {
+        notEmpty(object, "[Assertion failed] - this object must not be null or empty");
+    }
+
 
     /**
      * Assert that the provided object is an instance of the provided class.

impl/src/main/java/io/jsonwebtoken/impl/lang/DefaultCollectionMutator.java

@@ -22,6 +22,7 @@
 import io.jsonwebtoken.lang.Strings;
 
 import java.util.Collection;
+import java.util.Iterator;
 import java.util.LinkedHashSet;
 
 public class DefaultCollectionMutator<E, M extends CollectionMutator<E, M>> implements CollectionMutator<E, M> {
@@ -48,7 +49,45 @@ private boolean doAdd(E e) {
 
     @Override
     public M add(E e) {
-        if (doAdd(e)) changed();
+        boolean doReplace = false;
+
+        // Replacement step 1: iterate until element to replace (if any)
+        E item;
+        Iterator<E> it = this.collection.iterator();
+        while (it.hasNext()) {
+            item = it.next();
+
+            // Same item, nothing to do
+            if (item.equals(e))
+                return self();
+
+            boolean bothIdentifiable = e instanceof Identifiable && item instanceof Identifiable;
+            boolean sameId = bothIdentifiable && ((Identifiable) item).getId().equals(((Identifiable) e).getId());
+            if (sameId) {
+                it.remove(); // step 2: remove existing item
+                doReplace = true;
+                break;
+            }
+        }
+
+        if (doReplace) {
+            // Replacement step 3: collect and remove elements after element to replace
+            Collection<E> elementsAfterExisting = new LinkedHashSet<>();
+            while (it.hasNext()) {
+                elementsAfterExisting.add(it.next());
+                it.remove();
+            }
+
+            this.doAdd(e); // step 4: add replacer element (position will be at the existing item)
+            this.collection.addAll(elementsAfterExisting); // step 5: add back the elements found after existing item
+
+            changed(); // trigger changed()
+        }
+        else {
+            // No replacement, do add instead
+            if (doAdd(e)) changed();
+        }
+
         return self();
     }
 

impl/src/main/java/io/jsonwebtoken/impl/security/DefaultMacAlgorithm.java

@@ -222,4 +222,16 @@ protected boolean doVerify(VerifySecureDigestRequest<SecretKey> request) {
         byte[] computedSignature = digest(request);
         return MessageDigest.isEqual(providedSignature, computedSignature);
     }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (obj instanceof DefaultMacAlgorithm) {
+            DefaultMacAlgorithm other = (DefaultMacAlgorithm) obj;
+            return super.equals(obj) && this.minKeyBitLength == other.minKeyBitLength;
+        }
+        return false;
+    }
 }

impl/src/test/groovy/io/jsonwebtoken/impl/lang/DefaultCollectionMutatorTest.groovy

@@ -16,10 +16,14 @@
 package io.jsonwebtoken.impl.lang
 
 import io.jsonwebtoken.Identifiable
+import io.jsonwebtoken.Jwts
 import io.jsonwebtoken.lang.Strings
+import io.jsonwebtoken.security.MacAlgorithm
 import org.junit.Before
 import org.junit.Test
 
+import java.lang.reflect.Constructor
+
 import static org.junit.Assert.*
 
 /**
@@ -48,9 +52,17 @@ class DefaultCollectionMutatorTest {
         assertTrue c.isEmpty()
     }
 
+    @Test
+    void addNull() {
+        m.add(null)
+        assertEquals 0, changeCount
+        assertTrue m.getCollection().isEmpty() // wasn't added
+    }
+
     @Test
     void addEmpty() {
         m.add(Strings.EMPTY)
+        assertEquals 0, changeCount
         assertTrue m.getCollection().isEmpty() // wasn't added
     }
 
@@ -95,24 +107,48 @@ class DefaultCollectionMutatorTest {
 
     @Test(expected = IllegalArgumentException)
     void addIdentifiableWithNullId() {
-        def e = new Identifiable() {
-            @Override
-            String getId() {
-                return null
-            }
-        }
-        m.add(e)
+        m.add(new IdentifiableObject(null, null))
     }
 
     @Test(expected = IllegalArgumentException)
     void addIdentifiableWithEmptyId() {
-        def e = new Identifiable() {
-            @Override
-            String getId() {
-                return '  '
-            }
-        }
-        m.add(e)
+        m.add(new IdentifiableObject('  ', null))
+    }
+
+    @Test
+    void addIdentifiableWithSameIdEvictsExisting() {
+        m.add(new IdentifiableObject('sameId', 'foo'))
+        m.add(new IdentifiableObject('sameId', 'bar'))
+        assertEquals 2, changeCount
+        assertEquals 1, m.collection.size() // second 'add' should evict first
+        assertEquals 'bar', ((IdentifiableObject) m.collection.toArray()[0]).obj
+    }
+
+    @Test
+    void addIdentifiableWithSameIdMaintainsOrder() {
+        IdentifiableObject e1 = new IdentifiableObject('1', 'e1')
+        IdentifiableObject e2 = new IdentifiableObject('sameId', 'e2')
+        IdentifiableObject e3 = new IdentifiableObject('3', 'e3')
+        IdentifiableObject eB = new IdentifiableObject('sameId', 'eB')
+
+        m.add([e1, e2, e3])
+        m.add(eB) // replace e2 with eB
+        assertEquals 2, changeCount
+        assertArrayEquals(new Object[] {e1, eB, e3}, m.collection.toArray())
+    }
+
+    @Test
+    void addSecureDigestAlgorithmWithSameIdReplacesExisting() {
+        Class<?> c = Class.forName("io.jsonwebtoken.impl.security.DefaultMacAlgorithm")
+        Constructor<?> ctor = c.getDeclaredConstructor(String.class, String.class, int.class)
+        ctor.setAccessible(true)
+        MacAlgorithm custom = (MacAlgorithm) ctor.newInstance('HS512', 'HmacSHA512', 80)
+
+        m.add(Jwts.SIG.HS512)
+        m.add(custom)
+        assertEquals 2, changeCount // replace is count as one change
+        assertEquals 1, m.getCollection().size() // existing is removed as part of replacement
+        assertEquals 80, ((MacAlgorithm) m.getCollection().toArray()[0]).getKeyBitLength()
     }
 
     @Test
@@ -135,4 +171,19 @@ class DefaultCollectionMutatorTest {
         m.clear()
         assertTrue m.getCollection().isEmpty()
     }
+
+    private class IdentifiableObject implements Identifiable {
+        String id
+        Object obj
+
+        IdentifiableObject(String id, Object obj) {
+            this.id = id
+            this.obj = obj
+        }
+
+        @Override
+        String getId() {
+            return id
+        }
+    }
 }

pom.xml

@@ -33,7 +33,7 @@
 
     <licenses>
         <license>
-            <name>Apache License, Version 2.0</name>
+            <name>Apache-2.0</name>
             <url>https://www.apache.org/licenses/LICENSE-2.0</url>
             <distribution>repo</distribution>
         </license>