NestedCollection changes without .and()

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.

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

api/src/main/java/io/jsonwebtoken/ClaimsMutator.java

@@ -96,6 +96,17 @@ public interface ClaimsMutator<T extends ClaimsMutator<T>> {
  * <a href="https://www.rfc-editor.org/rfc/rfc7519.html#section-4.1.3"><code>aud</code></a> (audience) Claim
  * set, quietly ignoring any null, empty, whitespace-only, or existing value already in the set.
  *
+ * <p>When finished, the {@code audience} collection's {@link AudienceCollection#and() and()} method may be used
+ * to continue configuration. For example:</p>
+ * <blockquote><pre>
+ * Jwts.builder() // or Jwts.claims()
+ *
+ * .audience().add("anAudience")<b>.and() // return parent</b>
+ *
+ * .subject("Joe") // resume configuration...
+ * // etc...
+ * </pre></blockquote>
+ *
  * @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<T extends ClaimsMutator<T>> {
  * 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.
  *
+ * <p>Because this interface extends {@link NestedCollection}, the {@link #and()} method may be used to continue
+ * parent configuration. For example:</p>
+ * <blockquote><pre>
+ * Jwts.builder() // or Jwts.claims()
+ *
+ * .audience().add("anAudience")<b>.and() // return parent</b>
+ *
+ * .subject("Joe") // resume parent configuration...
+ * // etc...</pre></blockquote>
+ *
  * @param <P> the type of ClaimsMutator to return for method chaining.
  * @see #single(String)
  * @since 0.12.0

api/src/main/java/io/jsonwebtoken/JwtParserBuilder.java

@@ -101,12 +101,25 @@ public interface JwtParserBuilder extends Builder<JwtParser> {
  * 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.
  *
+ * <p>The collection's {@link Conjunctor#and() and()} method returns to the builder for continued parser
+ * configuration, for example:</p>
+ * <blockquote><pre>
+ * parserBuilder.critical().add("headerName")<b>.{@link Conjunctor#and() and()} // etc...</b></pre></blockquote>
+ *
  * <p><b>Extension Behavior</b></p>
  *
  * <p>The {@code critical} collection only identifies header parameter names that are used in extensions supported
  * by the application. <b>Application developers, <em>not JJWT</em>, MUST perform the associated extension behavior
  * using the parsed JWT</b>.</p>
  *
+ * <p><b>Continued Parser Configuration</b></p>
+ * <p>When finished, use the collection's
+ * {@link Conjunctor#and() and()} method to continue parser configuration, for example:
+ * <blockquote><pre>
+ * Jwts.parser()
+ * .critical().add("headerName").<b>{@link Conjunctor#and() and()} // return parent</b>
+ * // resume parser configuration...</pre></blockquote>
+ *
  * @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<JwtParser> {
  * <p>The collection's {@link Conjunctor#and() and()} method returns to the builder for continued parser
  * configuration, for example:</p>
  * <blockquote><pre>
- * parserBuilder.enc().add(anAeadAlgorithm).{@link Conjunctor#and() and()} // etc...</pre></blockquote>
+ * parserBuilder.enc().add(anAeadAlgorithm)<b>.{@link Conjunctor#and() and()} // etc...</b></pre></blockquote>
  *
  * <p><b>Standard Algorithms and Overrides</b></p>
  *
@@ -597,7 +610,7 @@ public interface JwtParserBuilder extends Builder<JwtParser> {
  * <p>The collection's {@link Conjunctor#and() and()} method returns to the builder for continued parser
  * configuration, for example:</p>
  * <blockquote><pre>
- * parserBuilder.key().add(aKeyAlgorithm).{@link Conjunctor#and() and()} // etc...</pre></blockquote>
+ * parserBuilder.key().add(aKeyAlgorithm)<b>.{@link Conjunctor#and() and()} // etc...</b></pre></blockquote>
  *
  * <p><b>Standard Algorithms and Overrides</b></p>
  *
@@ -639,7 +652,7 @@ public interface JwtParserBuilder extends Builder<JwtParser> {
  * <p>The collection's {@link Conjunctor#and() and()} method returns to the builder for continued parser
  * configuration, for example:</p>
  * <blockquote><pre>
- * parserBuilder.sig().add(aSignatureAlgorithm).{@link Conjunctor#and() and()} // etc...</pre></blockquote>
+ * parserBuilder.sig().add(aSignatureAlgorithm)<b>.{@link Conjunctor#and() and()} // etc...</b></pre></blockquote>
  *
  * <p><b>Standard Algorithms and Overrides</b></p>
  *
@@ -680,7 +693,7 @@ public interface JwtParserBuilder extends Builder<JwtParser> {
  * <p>The collection's {@link Conjunctor#and() and()} method returns to the builder for continued parser
  * configuration, for example:</p>
  * <blockquote><pre>
- * parserBuilder.zip().add(aCompressionAlgorithm).{@link Conjunctor#and() and()} // etc...</pre></blockquote>
+ * parserBuilder.zip().add(aCompressionAlgorithm)<b>.{@link Conjunctor#and() and()} // etc...</b></pre></blockquote>
  *
  * <p><b>Standard Algorithms and Overrides</b></p>
  *

api/src/main/java/io/jsonwebtoken/ProtectedHeaderMutator.java

@@ -33,9 +33,11 @@ public interface ProtectedHeaderMutator<T extends ProtectedHeaderMutator<T>> ext
  /**
  * Configures names of header parameters used by JWT or JWA specification extensions that <em>MUST</em> 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:
  * <blockquote><pre>
- * builder.critical().add("headerName").{@link Conjunctor#and() and()} // etc...</pre></blockquote>
+ * headerBuilder
+ * .critical().add("headerName").<b>{@link Conjunctor#and() and()} // return parent</b>
+ * // resume header configuration...</pre></blockquote>
  *
  * @return the {@link NestedCollection} to use for {@code crit} configuration.
  * @see <a href="https://www.rfc-editor.org/rfc/rfc7515.html#section-4.1.11">JWS <code>crit</code> (Critical) Header Parameter</a>

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:
+ * <blockquote><pre>
+ * builder
+ * .aNestedCollection()// etc...
+ * <b>.and() // return parent</b>
+ * // resume parent configuration...</pre></blockquote>
  *
  * @param <E> the type of elements in the collection
  * @param <P> the parent to return

api/src/main/java/io/jsonwebtoken/security/JwkBuilder.java

@@ -109,9 +109,9 @@ public interface JwkBuilder<K extends Key, J extends Jwk<K>, 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:
  * <blockquote><pre>
- * jwkBuilder.operations().add(aKeyOperation).{@link Conjunctor#and() and()} // etc...</pre></blockquote>
+ * jwkBuilder.operations().add(aKeyOperation)<b>.{@link Conjunctor#and() and()} // etc...</b></pre></blockquote>
  *
- * <p>The {@code and()} method will throw an {@link IllegalArgumentException} if any of the specified
+ * <p>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.</p>

impl/src/main/java/io/jsonwebtoken/impl/DefaultJweHeaderMutator.java

@@ -107,9 +107,8 @@ public T setCompressionAlgorithm(String zip) {
  public NestedCollection<String, T> critical() {
  return new DefaultNestedCollection<String, T>(self(), this.DELEGATE.get(DefaultProtectedHeader.CRIT)) {
  @Override
- public T and() {
+ protected void changed() {
  put(DefaultProtectedHeader.CRIT, Collections.asSet(getCollection()));
- return super.and();
  }
  };
  }

impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParserBuilder.java

@@ -220,9 +220,8 @@ public JwtParserBuilder clock(Clock clock) {
  public NestedCollection<String, JwtParserBuilder> critical() {
  return new DefaultNestedCollection<String, JwtParserBuilder>(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<CompressionAlgorithm, JwtParserBuilder> zip() {
  return new DefaultNestedCollection<CompressionAlgorithm, JwtParserBuilder>(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<AeadAlgorithm, JwtParserBuilder> enc() {
  return new DefaultNestedCollection<AeadAlgorithm, JwtParserBuilder>(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<SecureDigestAlgorithm<?, ?>, JwtParserBuilder> sig() {
  return new DefaultNestedCollection<SecureDigestAlgorithm<?, ?>, 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<KeyAlgorithm<?, ?>, JwtParserBuilder> key() {
  return new DefaultNestedCollection<KeyAlgorithm<?, ?>, JwtParserBuilder>(this, this.keyAlgs.values()) {
  @Override
- public JwtParserBuilder and() {
+ public void changed() {
  keyAlgs = new IdRegistry<>(StandardKeyAlgorithms.NAME, getCollection());
- return super.and();
  }
  };
  }

impl/src/main/java/io/jsonwebtoken/impl/DelegatingClaimsMutator.java

@@ -130,12 +130,12 @@ public AudienceCollection<T> 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();
  }
  };
  }

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<? extends E> 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<E> getCollection() {
  return Collections.immutable(this.collection);
  }

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

@@ -111,11 +111,10 @@ public T idFromThumbprint(HashAlgorithm alg) {
  public NestedCollection<KeyOperation, T> operations() {
  return new DefaultNestedCollection<KeyOperation, T>(self(), this.DELEGATE.getOperations()) {
  @Override
- public T and() {
+ protected void changed() {
  Collection<? extends KeyOperation> c = getCollection();
  opsPolicy.validate(c);
  DELEGATE.setOperations(c);
- return super.and();
  }
  };
  }

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 <a href="https://github.com/jwtk/jjwt/issues/916">JJWT Issue 916</a>
+ * @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 <a href="https://github.com/jwtk/jjwt/issues/916">JJWT Issue 916</a>
+ * @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<String, ?>
+
+ def expected = [crit] as Set<String>
+ def val = headerMap.get('crit') as Set<String>
+ assertNotNull val
+ assertEquals expected, val
+ }
+
 }