Fix pr remarks

Author: arturobernalg

httpclient5/src/main/java/org/apache/hc/client5/http/impl/ScramException.java

@@ -36,7 +36,7 @@
  * an error or issue is encountered during the SCRAM authentication process.
  * </p>
  *
- * @since 5.5
+ * @since 5.6
  */
 public class ScramException extends AuthenticationException {
 

httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/ScramScheme.java

@@ -64,21 +64,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-/**
- * Strict HTTP SCRAM client implementing {@code SCRAM-SHA-256} per RFC 7804
- * with SCRAM core per RFC 5802/7677.
- * <p>HTTP SCRAM uses <em>no channel binding</em> (GS2 header {@code "n,,"}; {@code c=biws}).</p>
- *
- * @since 5.6
- */
-
-/**
- * Strict HTTP SCRAM client implementing {@code SCRAM-SHA-256} per RFC&nbsp;7804
- * with SCRAM core per RFC&nbsp;5802/7677.
- * <p>HTTP SCRAM uses <em>no channel binding</em> (GS2 header {@code "n,,"}; {@code c=biws}).</p>
- *
- * @since 5.6
- */
 
 /**
  * Strict HTTP SCRAM client implementing {@code SCRAM-SHA-256} per RFC&nbsp;7804
@@ -130,7 +115,8 @@ private enum State {
     private byte[] salt;
     private int iterations;
 
-    // Expected server signature (raw bytes) for constant-time check on 2xx Authentication-Info
+    // Expected server signature (raw bytes) for constant-time check on Authentication-Info
+    // (may appear on any final response status code)
     private byte[] expectedV;
 
     /**
@@ -177,7 +163,7 @@ public boolean isConnectionBased() {
     }
 
     /**
-     * SCRAM must inspect successful responses to verify {@code v=} in {@code Authentication-Info}.
+     * SCRAM must inspect final responses to verify {@code v=} in {@code Authentication-Info}.
      *
      * @since 5.6
      */
@@ -202,7 +188,8 @@ public void processChallenge(final AuthChallenge authChallenge, final HttpContex
     }
 
     /**
-     * Handles 401 challenges (with/without {@code data}) and 2xx {@code Authentication-Info}.
+     * Handles 401 challenges (with/without {@code data}) and final responses carrying
+     * {@code Authentication-Info} (any status code).
      *
      * @since 5.6
      */
@@ -217,7 +204,7 @@ public void processChallenge(
 
         if (authChallenge == null) {
             if (!challenged) {
-                // 2xx with no info: nothing to do
+                // Final response with no Authentication-Info: nothing to do
                 return;
             }
             throw new MalformedChallengeException("Null SCRAM challenge");
@@ -250,7 +237,7 @@ public void processChallenge(
             final String r = attrs.get("r");
             final String s = attrs.get("s");
             final String i = attrs.get("i");
-            if (r == null || s == null || i == null) {
+            if (r == null || r.isEmpty() || s == null || s.isEmpty() || i == null || i.isEmpty()) {
                 this.state = State.FAILED;
                 throw new MalformedChallengeException("SCRAM server-first missing r/s/i");
             }
@@ -262,12 +249,18 @@ public void processChallenge(
             this.sid = params.get("sid");
             try {
                 this.salt = B64D.decode(s);
+                if (this.salt.length == 0) {
+                    throw new IllegalArgumentException("empty salt");
+                }
             } catch (final IllegalArgumentException e) {
                 this.state = State.FAILED;
                 throw new MalformedChallengeException("Invalid base64 salt", e);
             }
             try {
                 this.iterations = Integer.parseInt(i);
+                if (this.iterations <= 0) {
+                    throw new NumberFormatException("i<=0");
+                }
             } catch (final NumberFormatException e) {
                 this.state = State.FAILED;
                 throw new MalformedChallengeException("Invalid iteration count", e);
@@ -289,7 +282,7 @@ public void processChallenge(
             return;
         }
 
-        // --- 2xx path (Authentication-Info) ---
+        // --- final-response path (Authentication-Info on any status) ---
         // For Authentication-Info, RFC 7804 does NOT mandate a scheme token; do NOT enforce scheme name here.
         final String data = params.get("data");
         if (data == null) {
@@ -361,9 +354,8 @@ public boolean isResponseReady(
             final CredentialsProvider credentialsProvider,
             final HttpContext context) throws AuthenticationException {
 
-        if (credentialsProvider == null) {
-            throw new IllegalArgumentException("CredentialsProvider");
-        }
+        Args.notNull(credentialsProvider, "Credentials provider");
+
         final HttpClientContext clientContext = HttpClientContext.cast(context);
         final AuthScope scope = new AuthScope(host, this.realm, getName());
         final Credentials creds = credentialsProvider.getCredentials(scope, clientContext);
@@ -682,4 +674,8 @@ public String toString() {
             return this.name;
         }
     }
+
+    private static boolean isBlank(final String s) {
+        return s == null || s.isEmpty();
+    }
 }

httpclient5/src/test/java/org/apache/hc/client5/http/impl/auth/TestScramScheme.java

@@ -416,9 +416,7 @@ void testNullCredentialsProvider() {
         final ScramScheme scheme = new ScramScheme();
         final HttpClientContext ctx = HttpClientContext.create();
 
-        assertThrows(IllegalArgumentException.class, () -> {
-            scheme.isResponseReady(HOST, null, ctx);
-        });
+        assertThrows(NullPointerException.class, () -> scheme.isResponseReady(HOST, null, ctx));
     }
 
     @Test