Replaced ProtectedHeaderMutator#critical* methods with critical() Nes…

…tedCollection

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

@@ -15,11 +15,12 @@
  */
 package io.jsonwebtoken;
 
+import io.jsonwebtoken.lang.Conjunctor;
+import io.jsonwebtoken.lang.NestedCollection;
 import io.jsonwebtoken.security.PublicJwk;
 import io.jsonwebtoken.security.X509Mutator;
 
 import java.net.URI;
-import java.util.Collection;
 
 /**
  * Mutation (modifications) to a {@link ProtectedHeader Header} instance.
@@ -30,29 +31,17 @@
 public interface ProtectedHeaderMutator<T extends ProtectedHeaderMutator<T>> extends HeaderMutator<T>, X509Mutator<T> {
 
     /**
-     * Adds the name of a header parameter used by a JWT or JWA specification extension that <em>MUST</em> be understood
-     * and supported by the JWT recipient.  A {@code null}, empty, whitespace-only or already existing value is ignored.
+     * 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:
+     * <blockquote><pre>
+     * builder.critical().add("headerName").{@link Conjunctor#and() and()} // etc...</pre></blockquote>
      *
-     * @param crit the name of a header parameter used by a JWT or JWA specification extension that <em>MUST</em> be
-     *             understood and supported by the JWT recipient.
-     * @return the header for method chaining.
+     * @return foo
      * @see <a href="https://www.rfc-editor.org/rfc/rfc7515.html#section-4.1.11">JWS <code>crit</code> (Critical) Header Parameter</a>
      * @see <a href="https://www.rfc-editor.org/rfc/rfc7516.html#section-4.1.13">JWS <code>crit</code> (Critical) Header Parameter</a>
      */
-    T critical(String crit);
-
-    /**
-     * Adds names of header parameters used by JWT or JWA specification extensions that <em>MUST</em> be
-     * understood and supported by the JWT recipient. {@code null}, empty, whitespace-only or already existing
-     * values are ignored.
-     *
-     * @param crit names of header parameters used by JWT or JWA specification extensions that <em>MUST</em> be
-     *             understood and supported by the JWT recipient
-     * @return the header for method chaining.
-     * @see <a href="https://www.rfc-editor.org/rfc/rfc7515.html#section-4.1.11">JWS <code>crit</code> (Critical) Header Parameter</a>
-     * @see <a href="https://www.rfc-editor.org/rfc/rfc7516.html#section-4.1.13">JWS <code>crit</code> (Critical) Header Parameter</a>
-     */
-    T critical(Collection<String> crit);
+    NestedCollection<String, T> critical();
 
     /**
      * Sets the {@code jwk} (JSON Web Key) associated with the JWT.  When set for a {@link JwsHeader}, the

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

@@ -105,15 +105,15 @@ public interface JwkBuilder<K extends Key, J extends Jwk<K>, T extends JwkBuilde
     T idFromThumbprint(HashAlgorithm alg);
 
     /**
-     * Configures the JWK's {@link KeyOperation}s that identify the operation(s) for which the key is intended to be
-     * used. Resume JWK modifications by using the nested collection's {@link Conjunctor#and() and()} method
-     * to return to the JWK builder, for example:
+     * Configures the <a href="https://www.rfc-editor.org/rfc/rfc7517.html#section-4.3">key operations</a> for which
+     * 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>
      *
      * <p>The {@code and()} method 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 method's documentation for more
+     * {@link #operationPolicy(KeyOperationPolicy) operationPolicy}. See that documentation for more
      * information on security vulnerabilities when using the same key with multiple algorithms.</p>
      *
      * <p><b>Standard {@code KeyOperation}s and Overrides</b></p>

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

@@ -16,19 +16,18 @@
 package io.jsonwebtoken.impl;
 
 import io.jsonwebtoken.JweHeaderMutator;
+import io.jsonwebtoken.impl.lang.DefaultNestedCollection;
 import io.jsonwebtoken.impl.lang.DelegatingMapMutator;
 import io.jsonwebtoken.impl.lang.Parameter;
 import io.jsonwebtoken.impl.security.X509BuilderSupport;
 import io.jsonwebtoken.lang.Collections;
+import io.jsonwebtoken.lang.NestedCollection;
 import io.jsonwebtoken.lang.Strings;
 import io.jsonwebtoken.security.PublicJwk;
 
 import java.net.URI;
 import java.security.cert.X509Certificate;
-import java.util.Collection;
-import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Set;
 
 /**
  * @param <T> return type for method chaining
@@ -105,29 +104,14 @@ public T setCompressionAlgorithm(String zip) {
     // =============================================================
 
     @Override
-    public T critical(String crit) {
-        crit = Strings.clean(crit);
-        if (Strings.hasText(crit)) {
-            critical(Collections.setOf(crit));
-        }
-        return self();
-    }
-
-    @Override
-    public T critical(Collection<String> crit) {
-        if (!Collections.isEmpty(crit)) {
-            Set<String> existing = Collections.nullSafe(this.DELEGATE.get(DefaultProtectedHeader.CRIT));
-            Set<String> set = new LinkedHashSet<>(existing.size() + crit.size());
-            set.addAll(existing);
-            for (String s : crit) {
-                s = Strings.clean(s);
-                if (s != null) {
-                    set.add(s);
-                }
+    public NestedCollection<String, T> critical() {
+        return new DefaultNestedCollection<String, T>(self(), this.DELEGATE.get(DefaultProtectedHeader.CRIT)) {
+            @Override
+            public T and() {
+                put(DefaultProtectedHeader.CRIT, Collections.asSet(getCollection()));
+                return super.and();
             }
-            put(DefaultProtectedHeader.CRIT, set);
-        }
-        return self();
+        };
     }
 
     @Override

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

@@ -586,7 +586,7 @@ private String sign(final Payload payload, final Key key, final Provider provide
         this.headerBuilder.add(DefaultHeader.ALGORITHM.getId(), sigAlg.getId());
         if (!this.encodePayload) { // b64 extension:
             String id = DefaultJwsHeader.B64.getId();
-            this.headerBuilder.critical(id).add(id, false);
+            this.headerBuilder.critical().add(id).and().add(id, false);
         }
         final JwsHeader header = Assert.isInstanceOf(JwsHeader.class, this.headerBuilder.build());
         encodeAndWrite("JWS Protected Header", header, jws);

impl/src/test/groovy/io/jsonwebtoken/impl/AbstractProtectedHeaderTest.groovy

@@ -214,7 +214,7 @@ class AbstractProtectedHeaderTest {
     void testCritical() {
         Set<String> crits = Collections.setOf('foo', 'bar')
         def header = Jwts.header().add('alg', 'HS256').add('foo', 'value1').add('bar', 'value2')
-                .critical(crits).build() as DefaultProtectedHeader
+                .critical().add(crits).and().build() as DefaultProtectedHeader
         assertEquals crits, header.getCritical()
     }
 

impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtHeaderBuilderTest.groovy

@@ -325,7 +325,7 @@ class DefaultJwtHeaderBuilderTest {
     @Test
     void testCritical() {
         def crit = ['foo'] as Set<String>
-        header = jws().add('foo', 'bar').critical(crit).build() as JwsHeader
+        header = jws().add('foo', 'bar').critical().add(crit).and().build() as JwsHeader
         assertTrue header instanceof JwsHeader
         assertFalse header instanceof JweHeader
         assertEquals crit, header.getCritical()
@@ -502,7 +502,7 @@ class DefaultJwtHeaderBuilderTest {
     @Test
     void testCritSingle() {
         def crit = 'test'
-        def header = jws().add(crit, 'foo').critical(crit).build() as ProtectedHeader
+        def header = jws().add(crit, 'foo').critical().add(crit).and().build() as ProtectedHeader
         def expected = [crit] as Set<String>
         assertEquals expected, header.getCritical()
     }
@@ -511,34 +511,34 @@ class DefaultJwtHeaderBuilderTest {
     void testCritSingleNullIgnored() {
         def crit = 'test'
         def expected = [crit] as Set<String>
-        def header = jws().add(crit, 'foo').critical(crit).build() as ProtectedHeader
+        def header = jws().add(crit, 'foo').critical().add(crit).and().build() as ProtectedHeader
         assertEquals expected, header.getCritical()
-        header = builder.critical((String) null).build() as ProtectedHeader // ignored
+        header = builder.critical().add((String) null).and().build() as ProtectedHeader // ignored
         assertEquals expected, header.getCritical() // nothing changed
     }
 
     @Test
     void testCritNullCollectionIgnored() {
         def crit = ['test'] as Set<String>
-        def header = jws().add('test', 'foo').critical(crit).build() as ProtectedHeader
+        def header = jws().add('test', 'foo').critical().add(crit).and().build() as ProtectedHeader
         assertEquals crit, header.getCritical()
-        header = builder.critical((Collection) null).build() as ProtectedHeader
+        header = builder.critical().add((Collection) null).and().build() as ProtectedHeader
         assertEquals crit, header.getCritical() // nothing changed
     }
 
     @Test
     void testCritCollectionWithNullElement() {
         def crit = [null] as Set<String>
-        def header = jws().add('test', 'foo').critical(crit).build() as ProtectedHeader
+        def header = jws().add('test', 'foo').critical().add(crit).and().build() as ProtectedHeader
         assertNull header.getCritical()
     }
 
     @Test
     void testCritEmptyIgnored() {
         def crit = ['test'] as Set<String>
-        def header = jws().add('test', 'foo').critical(crit).build() as ProtectedHeader
+        ProtectedHeader header = jws().add('test', 'foo').critical().add(crit).and().build() as ProtectedHeader
         assertEquals crit, header.getCritical()
-        header = builder.critical([] as Set<String>).build()
+        header = builder.critical().add([] as Set<String>).and().build() as ProtectedHeader
         assertEquals crit, header.getCritical() // ignored
     }
 
@@ -550,7 +550,7 @@ class DefaultJwtHeaderBuilderTest {
     void testCritRemovedForUnprotectedHeader() {
         def crit = Collections.setOf('foo', 'bar')
         // no JWS or JWE params specified:
-        def header = builder.add('test', 'value').critical(crit).build()
+        def header = builder.add('test', 'value').critical().add(crit).and().build()
         assertFalse header.containsKey(DefaultProtectedHeader.CRIT.getId())
     }
 
@@ -562,15 +562,15 @@ class DefaultJwtHeaderBuilderTest {
     void testCritNamesSanitizedWhenHeaderMissingCorrespondingParameter() {
         def critGiven = ['foo', 'bar'] as Set<String>
         def critExpected = ['foo'] as Set<String>
-        def header = jws().add('foo', 'fooVal').critical(critGiven).build() as ProtectedHeader
+        def header = jws().add('foo', 'fooVal').critical().add(critGiven).and().build() as ProtectedHeader
         // header didn't set the 'bar' parameter, so 'bar' should not be in the crit values:
         assertEquals critExpected, header.getCritical()
     }
 
     @Test
     void testCritNamesRemovedWhenHeaderMissingCorrespondingParameter() {
         def critGiven = ['foo'] as Set<String>
-        def header = jws().critical(critGiven).build() as ProtectedHeader
+        ProtectedHeader header = jws().critical().add(critGiven).and().build() as ProtectedHeader
         // header didn't set the 'foo' parameter, so crit would have been empty, and then removed from the header:
         assertNull header.getCritical()
     }

impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtParserTest.groovy

@@ -245,7 +245,7 @@ class DefaultJwtParserTest {
         def key = TestKeys.HS256
         def crit = Collections.setOf('whatever')
         String jws = Jwts.builder()
-                .header().critical(crit).add('whatever', 42).and()
+                .header().critical().add(crit).and().add('whatever', 42).and()
                 .subject('me')
                 .signWith(key).compact()
 

impl/src/test/groovy/io/jsonwebtoken/impl/DefaultMutableJweHeaderTest.groovy

@@ -45,6 +45,7 @@ class DefaultMutableJweHeaderTest {
         switch (propName) {
             case 'algorithm': header.add('alg', val); break // no setter
             case 'compressionAlgorithm': header.add('zip', val); break // no setter
+            case 'critical': header.critical().add(val).and(); break // no setter
             default: header."$propName"(val)
         }