diff --git a/CHANGELOG.md b/CHANGELOG.md index 33ce2fb91..70c53ede3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,31 @@ ## Release Notes +### 0.12.5 + +This patch release: + +* Ensures that builders' `NestedCollection` changes are applied to the collection immediately as mutation methods are called, no longer + requiring application developers to call `.and()` to 'commit' or apply a change. For example, prior to this release, + the following code did not apply changes: + ```java + JwtBuilder builder = Jwts.builder(); + builder.audience().add("an-audience"); // no .and() call + builder.compact(); // would not keep 'an-audience' + ``` + Now this code works as expected and all other `NestedCollection` instances like it apply changes immediately (e.g. when calling + `.add(value)`). + + However, standard fluent builder chains are still recommended for readability when feasible, e.g. + + ```java + Jwts.builder() + .audience().add("an-audience").and() // allows fluent chaining + .subject("Joe") + // etc... + .compact() + ``` + See [Issue 916](https://github.com/jwtk/jjwt/issues/916). + ### 0.12.4 This patch release includes various changes listed below. diff --git a/README.md b/README.md index 794f95b5c..797e307b3 100644 --- a/README.md +++ b/README.md @@ -993,7 +993,7 @@ String jws = Jwts.builder() .issuer("me") .subject("Bob") - .audience("you") + .audience().add("you").and() .expiration(expiration) //a java.util.Date .notBefore(notBefore) //a java.util.Date .issuedAt(new Date()) // for example, now diff --git a/api/src/main/java/io/jsonwebtoken/ClaimsMutator.java b/api/src/main/java/io/jsonwebtoken/ClaimsMutator.java index 4db0950a5..1fdca1e43 100644 --- a/api/src/main/java/io/jsonwebtoken/ClaimsMutator.java +++ b/api/src/main/java/io/jsonwebtoken/ClaimsMutator.java @@ -96,6 +96,17 @@ public interface ClaimsMutator> { * aud (audience) Claim * set, quietly ignoring any null, empty, whitespace-only, or existing value already in the set. * + *

When finished, the {@code audience} collection's {@link AudienceCollection#and() and()} method may be used + * to continue configuration. For example:

+ *
+     *  Jwts.builder() // or Jwts.claims()
+     *
+     *     .audience().add("anAudience").and() // return parent
+     *
+     *  .subject("Joe") // resume configuration...
+     *  // etc...
+     * 
+ * * @return the {@link AudienceCollection AudienceCollection} to use for {@code aud} configuration. * @see AudienceCollection AudienceCollection * @see AudienceCollection#single(String) AudienceCollection.single(String) @@ -221,6 +232,16 @@ public interface ClaimsMutator> { * A {@code NestedCollection} for setting {@link #audience()} values that also allows overriding the collection * to be a {@link #single(String) single string value} for legacy JWT recipients if necessary. * + *

Because this interface extends {@link NestedCollection}, the {@link #and()} method may be used to continue + * parent configuration. For example:

+ *
+     *  Jwts.builder() // or Jwts.claims()
+     *
+     *     .audience().add("anAudience").and() // return parent
+     *
+     *  .subject("Joe") // resume parent configuration...
+     *  // etc...
+ * * @param

the type of ClaimsMutator to return for method chaining. * @see #single(String) * @since 0.12.0 diff --git a/api/src/main/java/io/jsonwebtoken/JwtParserBuilder.java b/api/src/main/java/io/jsonwebtoken/JwtParserBuilder.java index d1a4755d5..79936695a 100644 --- a/api/src/main/java/io/jsonwebtoken/JwtParserBuilder.java +++ b/api/src/main/java/io/jsonwebtoken/JwtParserBuilder.java @@ -101,12 +101,25 @@ public interface JwtParserBuilder extends Builder { * the parser encounters a Protected JWT that {@link ProtectedHeader#getCritical() requires} extensions, and * those extensions' header names are not specified via this method, the parser will reject that JWT. * + *

The collection's {@link Conjunctor#and() and()} method returns to the builder for continued parser + * configuration, for example:

+ *
+     * parserBuilder.critical().add("headerName").{@link Conjunctor#and() and()} // etc...
+ * *

Extension Behavior

* *

The {@code critical} collection only identifies header parameter names that are used in extensions supported * by the application. Application developers, not JJWT, MUST perform the associated extension behavior * using the parsed JWT.

* + *

Continued Parser Configuration

+ *

When finished, use the collection's + * {@link Conjunctor#and() and()} method to continue parser configuration, for example: + *

+     * Jwts.parser()
+     *     .critical().add("headerName").{@link Conjunctor#and() and()} // return parent
+     * // resume parser configuration...
+ * * @return the {@link NestedCollection} to use for {@code crit} configuration. * @see ProtectedHeader#getCritical() * @since 0.12.0 @@ -557,7 +570,7 @@ public interface JwtParserBuilder extends Builder { *

The collection's {@link Conjunctor#and() and()} method returns to the builder for continued parser * configuration, for example:

*
-     * parserBuilder.enc().add(anAeadAlgorithm).{@link Conjunctor#and() and()} // etc...
+ * parserBuilder.enc().add(anAeadAlgorithm).{@link Conjunctor#and() and()} // etc... * *

Standard Algorithms and Overrides

* @@ -597,7 +610,7 @@ public interface JwtParserBuilder extends Builder { *

The collection's {@link Conjunctor#and() and()} method returns to the builder for continued parser * configuration, for example:

*
-     * parserBuilder.key().add(aKeyAlgorithm).{@link Conjunctor#and() and()} // etc...
+ * parserBuilder.key().add(aKeyAlgorithm).{@link Conjunctor#and() and()} // etc... * *

Standard Algorithms and Overrides

* @@ -639,7 +652,7 @@ public interface JwtParserBuilder extends Builder { *

The collection's {@link Conjunctor#and() and()} method returns to the builder for continued parser * configuration, for example:

*
-     * parserBuilder.sig().add(aSignatureAlgorithm).{@link Conjunctor#and() and()} // etc...
+ * parserBuilder.sig().add(aSignatureAlgorithm).{@link Conjunctor#and() and()} // etc... * *

Standard Algorithms and Overrides

* @@ -680,7 +693,7 @@ public interface JwtParserBuilder extends Builder { *

The collection's {@link Conjunctor#and() and()} method returns to the builder for continued parser * configuration, for example:

*
-     * parserBuilder.zip().add(aCompressionAlgorithm).{@link Conjunctor#and() and()} // etc...
+ * parserBuilder.zip().add(aCompressionAlgorithm).{@link Conjunctor#and() and()} // etc... * *

Standard Algorithms and Overrides

* diff --git a/api/src/main/java/io/jsonwebtoken/ProtectedHeaderMutator.java b/api/src/main/java/io/jsonwebtoken/ProtectedHeaderMutator.java index dd38bcaca..002250657 100644 --- a/api/src/main/java/io/jsonwebtoken/ProtectedHeaderMutator.java +++ b/api/src/main/java/io/jsonwebtoken/ProtectedHeaderMutator.java @@ -33,9 +33,11 @@ public interface ProtectedHeaderMutator> ext /** * Configures names of header parameters used by JWT or JWA specification extensions that MUST be * understood and supported by the JWT recipient. When finished, use the collection's - * {@link Conjunctor#and() and()} method to return to the header builder, for example: + * {@link Conjunctor#and() and()} method to continue header configuration, for example: *
-     * builder.critical().add("headerName").{@link Conjunctor#and() and()} // etc...
+ * headerBuilder + * .critical().add("headerName").{@link Conjunctor#and() and()} // return parent + * // resume header configuration... * * @return the {@link NestedCollection} to use for {@code crit} configuration. * @see JWS crit (Critical) Header Parameter diff --git a/api/src/main/java/io/jsonwebtoken/lang/NestedCollection.java b/api/src/main/java/io/jsonwebtoken/lang/NestedCollection.java index cf0118d4b..2fac66c94 100644 --- a/api/src/main/java/io/jsonwebtoken/lang/NestedCollection.java +++ b/api/src/main/java/io/jsonwebtoken/lang/NestedCollection.java @@ -17,7 +17,12 @@ /** * A {@link CollectionMutator} that can return access to its parent via the {@link Conjunctor#and() and()} method for - * continued configuration. + * continued configuration. For example: + *
+ * builder
+ *     .aNestedCollection()// etc...
+ *     .and() // return parent
+ * // resume parent configuration...
* * @param the type of elements in the collection * @param

the parent to return diff --git a/api/src/main/java/io/jsonwebtoken/security/JwkBuilder.java b/api/src/main/java/io/jsonwebtoken/security/JwkBuilder.java index 2133925c9..add7be044 100644 --- a/api/src/main/java/io/jsonwebtoken/security/JwkBuilder.java +++ b/api/src/main/java/io/jsonwebtoken/security/JwkBuilder.java @@ -109,9 +109,9 @@ public interface JwkBuilder, T extends JwkBuilde * the key is intended to be used. When finished, use the collection's {@link Conjunctor#and() and()} method to * return to the JWK builder, for example: *

-     * jwkBuilder.operations().add(aKeyOperation).{@link Conjunctor#and() and()} // etc...
+ * jwkBuilder.operations().add(aKeyOperation).{@link Conjunctor#and() and()} // etc... * - *

The {@code and()} method will throw an {@link IllegalArgumentException} if any of the specified + *

The {@code add()} method(s) will throw an {@link IllegalArgumentException} if any of the specified * {@code KeyOperation}s are not permitted by the JWK's * {@link #operationPolicy(KeyOperationPolicy) operationPolicy}. See that documentation for more * information on security vulnerabilities when using the same key with multiple algorithms.

diff --git a/impl/src/main/java/io/jsonwebtoken/impl/DefaultJweHeaderMutator.java b/impl/src/main/java/io/jsonwebtoken/impl/DefaultJweHeaderMutator.java index b35806f42..fc7c1f117 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/DefaultJweHeaderMutator.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/DefaultJweHeaderMutator.java @@ -107,9 +107,8 @@ public T setCompressionAlgorithm(String zip) { public NestedCollection critical() { return new DefaultNestedCollection(self(), this.DELEGATE.get(DefaultProtectedHeader.CRIT)) { @Override - public T and() { + protected void changed() { put(DefaultProtectedHeader.CRIT, Collections.asSet(getCollection())); - return super.and(); } }; } diff --git a/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParserBuilder.java b/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParserBuilder.java index 192904807..ddce12b22 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParserBuilder.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParserBuilder.java @@ -220,9 +220,8 @@ public JwtParserBuilder clock(Clock clock) { public NestedCollection critical() { return new DefaultNestedCollection(this, this.critical) { @Override - public JwtParserBuilder and() { + protected void changed() { critical = Collections.asSet(getCollection()); - return super.and(); } }; } @@ -304,9 +303,8 @@ private JwtParserBuilder decryptWith(final Key key) { public NestedCollection zip() { return new DefaultNestedCollection(this, this.zipAlgs.values()) { @Override - public JwtParserBuilder and() { + protected void changed() { zipAlgs = new IdRegistry<>(StandardCompressionAlgorithms.NAME, getCollection()); - return super.and(); } }; } @@ -315,9 +313,8 @@ public JwtParserBuilder and() { public NestedCollection enc() { return new DefaultNestedCollection(this, this.encAlgs.values()) { @Override - public JwtParserBuilder and() { + public void changed() { encAlgs = new IdRegistry<>(StandardEncryptionAlgorithms.NAME, getCollection()); - return super.and(); } }; } @@ -326,9 +323,8 @@ public JwtParserBuilder and() { public NestedCollection, JwtParserBuilder> sig() { return new DefaultNestedCollection, JwtParserBuilder>(this, this.sigAlgs.values()) { @Override - public JwtParserBuilder and() { + public void changed() { sigAlgs = new IdRegistry<>(StandardSecureDigestAlgorithms.NAME, getCollection()); - return super.and(); } }; } @@ -337,9 +333,8 @@ public JwtParserBuilder and() { public NestedCollection, JwtParserBuilder> key() { return new DefaultNestedCollection, JwtParserBuilder>(this, this.keyAlgs.values()) { @Override - public JwtParserBuilder and() { + public void changed() { keyAlgs = new IdRegistry<>(StandardKeyAlgorithms.NAME, getCollection()); - return super.and(); } }; } diff --git a/impl/src/main/java/io/jsonwebtoken/impl/DelegatingClaimsMutator.java b/impl/src/main/java/io/jsonwebtoken/impl/DelegatingClaimsMutator.java index 9c408a154..fd5c48330 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/DelegatingClaimsMutator.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/DelegatingClaimsMutator.java @@ -130,12 +130,12 @@ public AudienceCollection audience() { @Override public T single(String audience) { return audienceSingle(audience); + // DO NOT call changed() here - we don't want to replace the value with a collection } @Override - public T and() { + protected void changed() { put(DefaultClaims.AUDIENCE, Collections.asSet(getCollection())); - return super.and(); } }; } diff --git a/impl/src/main/java/io/jsonwebtoken/impl/lang/DefaultCollectionMutator.java b/impl/src/main/java/io/jsonwebtoken/impl/lang/DefaultCollectionMutator.java index 9a64d7e67..cdb7fd8fc 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/lang/DefaultCollectionMutator.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/lang/DefaultCollectionMutator.java @@ -37,38 +37,52 @@ protected final M self() { return (M) this; } - @Override - public M add(E e) { - if (Objects.isEmpty(e)) return self(); + private boolean doAdd(E e) { + if (Objects.isEmpty(e)) return false; if (e instanceof Identifiable && !Strings.hasText(((Identifiable) e).getId())) { String msg = e.getClass() + " getId() value cannot be null or empty."; throw new IllegalArgumentException(msg); } - this.collection.remove(e); - this.collection.add(e); + return this.collection.add(e); + } + + @Override + public M add(E e) { + if (doAdd(e)) changed(); return self(); } @Override public M remove(E e) { - this.collection.remove(e); + if (this.collection.remove(e)) changed(); return self(); } @Override public M add(Collection c) { + boolean changed = false; for (E element : Collections.nullSafe(c)) { - add(element); + changed = doAdd(element) || changed; } + if (changed) changed(); return self(); } @Override public M clear() { + boolean changed = !Collections.isEmpty(this.collection); this.collection.clear(); + if (changed) changed(); return self(); } + /** + * Callback for subclasses that wish to be notified if the internal collection has changed via builder mutation + * methods. + */ + protected void changed() { + } + protected Collection getCollection() { return Collections.immutable(this.collection); } diff --git a/impl/src/main/java/io/jsonwebtoken/impl/security/AbstractJwkBuilder.java b/impl/src/main/java/io/jsonwebtoken/impl/security/AbstractJwkBuilder.java index 5ac3b4e54..e580b349e 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/security/AbstractJwkBuilder.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/security/AbstractJwkBuilder.java @@ -111,11 +111,10 @@ public T idFromThumbprint(HashAlgorithm alg) { public NestedCollection operations() { return new DefaultNestedCollection(self(), this.DELEGATE.getOperations()) { @Override - public T and() { + protected void changed() { Collection c = getCollection(); opsPolicy.validate(c); DELEGATE.setOperations(c); - return super.and(); } }; } diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtBuilderTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtBuilderTest.groovy index 591b4b07d..c6bc8abca 100644 --- a/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtBuilderTest.groovy +++ b/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtBuilderTest.groovy @@ -791,4 +791,47 @@ class DefaultJwtBuilderTest { assertEquals three, claims.aud } + /** + * Asserts that if a .audience() builder is used, and its .and() method is not called, the change to the + * audience is still applied when building the JWT. + * @see JJWT Issue 916 + * @since JJWT_RELEASE_VERSION + */ + @Test + void testAudienceWithoutConjunction() { + def aud = 'my-web' + def builder = Jwts.builder() + builder.audience().add(aud) // no .and() call + def jwt = builder.compact() + + // assert that the resulting claims has the audience array set as expected: + def parsed = Jwts.parser().unsecured().build().parseUnsecuredClaims(jwt) + assertEquals aud, parsed.payload.getAudience()[0] + } + + /** + * Asserts that if a .header().critical() builder is used, and its .and() method is not called, the change to the + * crit collection is still applied when building the header. + * @see JJWT Issue 916 + * @since JJWT_RELEASE_VERSION + */ + @Test + void testCritWithoutConjunction() { + def crit = 'test' + def builder = Jwts.builder().issuer('me') + def headerBuilder = builder.header() + headerBuilder.critical().add(crit) // no .and() method + headerBuilder.add(crit, 'foo') // no .and() method + builder.signWith(TestKeys.HS256) + def jwt = builder.compact() + + def headerBytes = Decoders.BASE64URL.decode(jwt.split('\\.')[0]) + def headerMap = Services.get(Deserializer).deserialize(Streams.reader(headerBytes)) as Map + + def expected = [crit] as Set + def val = headerMap.get('crit') as Set + assertNotNull val + assertEquals expected, val + } + } diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtHeaderBuilderTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtHeaderBuilderTest.groovy index 295192830..5c9df8efd 100644 --- a/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtHeaderBuilderTest.groovy +++ b/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtHeaderBuilderTest.groovy @@ -507,6 +507,22 @@ class DefaultJwtHeaderBuilderTest { assertEquals expected, header.getCritical() } + /** + * Asserts that if a .critical() builder is used, and its .and() method is not called, the change to the + * crit collection is still applied when building the header. + * @see JJWT Issue 916 + * @since JJWT_RELEASE_VERSION + */ + @Test + void testCritWithoutConjunction() { + def crit = 'test' + def builder = jws() + builder.add(crit, 'foo').critical().add(crit) // no .and() method + def header = builder.build() as ProtectedHeader + def expected = [crit] as Set + assertEquals expected, header.getCritical() + } + @Test void testCritSingleNullIgnored() { def crit = 'test' diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtParserBuilderTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtParserBuilderTest.groovy index 71479b363..9cb7ef7ce 100644 --- a/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtParserBuilderTest.groovy +++ b/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtParserBuilderTest.groovy @@ -48,6 +48,22 @@ class DefaultJwtParserBuilderTest { assertTrue builder.@critical.isEmpty() } + /** + * Asserts that if a .critical() builder is used, and its .and() method is not called, the change to the + * crit collection is still applied when building the parser. + * @see JJWT Issue 916 + * @since JJWT_RELEASE_VERSION + */ + @Test + void testCriticalWithoutConjunction() { + builder.critical().add('foo') // no .and() call + assertFalse builder.@critical.isEmpty() + assertTrue builder.@critical.contains('foo') + def parser = builder.build() + assertFalse parser.@critical.isEmpty() + assertTrue parser.@critical.contains('foo') + } + @Test void testSetProvider() { Provider provider = createMock(Provider) @@ -173,6 +189,21 @@ class DefaultJwtParserBuilderTest { assertSame codec, parser.zipAlgs.locate(header) } + /** + * Asserts that if a .zip() builder is used, and its .and() method is not called, the change to the + * compression algorithm collection is still applied when building the parser. + * @see JJWT Issue 916 + * @since JJWT_RELEASE_VERSION + */ + @Test + void testAddCompressionAlgorithmWithoutConjunction() { + def codec = new TestCompressionCodec(id: 'test') + builder.zip().add(codec) // no .and() call + def parser = builder.build() + def header = Jwts.header().add('zip', codec.getId()).build() + assertSame codec, parser.zipAlgs.locate(header) + } + @Test void testAddCompressionAlgorithmsOverrideDefaults() { def header = Jwts.header().add('zip', 'DEF').build() @@ -211,6 +242,21 @@ class DefaultJwtParserBuilderTest { assertSame custom, parser.encAlgs.apply(header) // custom one, not standard impl } + /** + * Asserts that if an .enc() builder is used, and its .and() method is not called, the change to the + * encryption algorithm collection is still applied when building the parser. + * @see JJWT Issue 916 + * @since JJWT_RELEASE_VERSION + */ + @Test + void testAddEncryptionAlgorithmWithoutConjunction() { + def alg = new TestAeadAlgorithm(id: 'test') + builder.enc().add(alg) // no .and() call + def parser = builder.build() as DefaultJwtParser + def header = Jwts.header().add('alg', 'foo').add('enc', alg.getId()).build() as JweHeader + assertSame alg, parser.encAlgs.apply(header) + } + @Test void testCaseSensitiveEncryptionAlgorithm() { def alg = Jwts.ENC.A256GCM @@ -239,6 +285,23 @@ class DefaultJwtParserBuilderTest { assertSame custom, parser.keyAlgs.apply(header) // custom one, not standard impl } + /** + * Asserts that if an .key() builder is used, and its .and() method is not called, the change to the + * key algorithm collection is still applied when building the parser. + * @see JJWT Issue 916 + * @since JJWT_RELEASE_VERSION + */ + @Test + void testAddKeyAlgorithmWithoutConjunction() { + def alg = new TestKeyAlgorithm(id: 'test') + builder.key().add(alg) // no .and() call + def parser = builder.build() as DefaultJwtParser + def header = Jwts.header() + .add('enc', 'foo') + .add('alg', alg.getId()).build() as JweHeader + assertSame alg, parser.keyAlgs.apply(header) + } + @Test void testCaseSensitiveKeyAlgorithm() { def alg = Jwts.KEY.A256GCMKW @@ -268,6 +331,21 @@ class DefaultJwtParserBuilderTest { assertSame custom, parser.sigAlgs.apply(header) // custom one, not standard impl } + /** + * Asserts that if an .sig() builder is used, and its .and() method is not called, the change to the + * signature algorithm collection is still applied when building the parser. + * @see JJWT Issue 916 + * @since JJWT_RELEASE_VERSION + */ + @Test + void testAddSignatureAlgorithmWithoutConjunction() { + def alg = new TestMacAlgorithm(id: 'test') + builder.sig().add(alg) // no .and() call + def parser = builder.build() as DefaultJwtParser + def header = Jwts.header().add('alg', alg.getId()).build() as JwsHeader + assertSame alg, parser.sigAlgs.apply(header) + } + @Test void testCaseSensitiveSignatureAlgorithm() { def alg = Jwts.SIG.HS256 diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/lang/DefaultCollectionMutatorTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/lang/DefaultCollectionMutatorTest.groovy index 213a39fe0..e1b6d4021 100644 --- a/impl/src/test/groovy/io/jsonwebtoken/impl/lang/DefaultCollectionMutatorTest.groovy +++ b/impl/src/test/groovy/io/jsonwebtoken/impl/lang/DefaultCollectionMutatorTest.groovy @@ -27,11 +27,18 @@ import static org.junit.Assert.* */ class DefaultCollectionMutatorTest { + private int changeCount private DefaultCollectionMutator m @Before void setUp() { - m = new DefaultCollectionMutator(null) + changeCount = 0 + m = new DefaultCollectionMutator(null) { + @Override + protected void changed() { + changeCount++ + } + } } @Test @@ -51,9 +58,17 @@ class DefaultCollectionMutatorTest { void add() { def val = 'hello' m.add(val) + assertEquals 1, changeCount assertEquals Collections.singleton(val), m.getCollection() } + @Test + void addDuplicateDoesNotTriggerChange() { + m.add('hello') + m.add('hello') //already in the set, no change should be reflected + assertEquals 1, changeCount + } + @Test void addCollection() { def vals = ['hello', 'world'] @@ -67,6 +82,17 @@ class DefaultCollectionMutatorTest { assertFalse i.hasNext() } + /** + * Asserts that if a collection is added, each internal addition to the collection doesn't call changed(); instead + * changed() is only called once after they've all been added to the collection + */ + @Test + void addCollectionTriggersSingleChange() { + def c = ['hello', 'world'] + m.add(c) + assertEquals 1, changeCount // only one change triggered, not c.size() + } + @Test(expected = IllegalArgumentException) void addIdentifiableWithNullId() { def e = new Identifiable() { @@ -96,6 +122,12 @@ class DefaultCollectionMutatorTest { assertEquals Collections.singleton('world'), m.getCollection() } + @Test + void removeMissingDoesNotTriggerChange() { + m.remove('foo') // not in the collection, no change should be registered + assertEquals 0, changeCount + } + @Test void clear() { m.add('one').add('two').add(['three', 'four']) diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/security/AbstractJwkBuilderTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/security/AbstractJwkBuilderTest.groovy index ce73f8acf..ee7a75dab 100644 --- a/impl/src/test/groovy/io/jsonwebtoken/impl/security/AbstractJwkBuilderTest.groovy +++ b/impl/src/test/groovy/io/jsonwebtoken/impl/security/AbstractJwkBuilderTest.groovy @@ -239,7 +239,21 @@ class AbstractJwkBuilderTest { .related(Jwks.OP.VERIFY.id).build() def builder = builder().operationPolicy(Jwks.OP.policy().add(op).build()) def jwk = builder.operations().add(Collections.setOf(op, Jwks.OP.VERIFY)).and().build() as Jwk - assertSame op, jwk.getOperations().find({it.id == 'sign'}) + assertSame op, jwk.getOperations().find({ it.id == 'sign' }) + } + + /** + * Asserts that if a .operations() builder is used, and its .and() method is not called, the change to the + * operations collection is still applied when building the JWK. + * @see JJWT Issue 916 + * @since JJWT_RELEASE_VERSION + */ + @Test + void testOperationsWithoutConjunction() { + def builder = builder() + builder.operations().clear().add(Jwks.OP.DERIVE_BITS) // no .and() call + def jwk = builder.build() + assertEquals(Jwks.OP.DERIVE_BITS, jwk.getOperations()[0]) } @Test