chore: Update PMD rules

Signed-off-by: Jorge Solórzano <[email protected]>

checks/pmd-ruleset.xml

@@ -8,23 +8,19 @@
   <exclude-pattern>.*/target/.*</exclude-pattern>
   <exclude-pattern>.*/generated/.*</exclude-pattern>
 
-  <!-- Your rules will come here -->
   <rule ref="rulesets/java/quickstart.xml" />
 
-  <rule ref="category/java/codestyle.xml/TooManyStaticImports" />
-
-  <rule ref="category/java/design.xml">
-    <exclude name="LoosePackageCoupling" />
+  <rule ref="category/java/bestpractices.xml">
+    <exclude name="UseVarargs" />
+    <exclude name="ForLoopVariableCount" />
+    <exclude name="AvoidReassigningLoopVariables" />
+    <exclude name="AccessorClassGeneration" />
   </rule>
 
   <rule ref="category/java/errorprone.xml">
-  </rule>
-
-  <rule ref="category/java/errorprone.xml/EmptyCatchBlock">
-    <properties>
-      <property name="allowCommentedBlocks" value="true" />
-      <property name="allowExceptionNameRegex" value="^(ignored|expected)$" />
-    </properties>
+    <exclude name="AvoidLiteralsInIfCondition" />
+    <exclude name="NullAssignment" />
+    <exclude name="AvoidFieldNameMatchingMethodName" />
   </rule>
 
   <rule ref="category/java/multithreading.xml" />
@@ -33,4 +29,6 @@
     <exclude name="StringInstantiation" />
   </rule>
 
+  <rule ref="category/java/security.xml" />
+
 </ruleset>

scram-client/src/main/java/com/ongres/scram/client/ClientFinalProcessor.java

@@ -74,10 +74,9 @@ private ClientFinalProcessor(ScramMechanism scramMechanism, byte[] clientKey,
         clientFirstMessage, serverFirstMessage);
   }
 
-  private synchronized void generateAndCacheAuthMessage(byte[] cbindData) {
+  private void generateAndCacheAuthMessage(byte[] cbindData) {
     if (null == this.authMessage) {
-      this.authMessage =
-          ScramFunctions.authMessage(clientFirstMessage, serverFirstMessage, cbindData);
+      this.authMessage = ScramFunctions.authMessage(clientFirstMessage, serverFirstMessage, cbindData);
     }
   }
 
@@ -90,9 +89,7 @@ private synchronized void generateAndCacheAuthMessage(byte[] cbindData) {
    */
   @NotNull
   ClientFinalMessage clientFinalMessage(byte @Nullable [] cbindData) {
-    if (null == authMessage) {
-      generateAndCacheAuthMessage(cbindData);
-    }
+    generateAndCacheAuthMessage(cbindData);
 
     return new ClientFinalMessage(
         clientFirstMessage.getGs2Header(),

scram-client/src/main/java/com/ongres/scram/client/ScramClient.java

@@ -378,22 +378,22 @@ FinalBuildStage secureRandomAlgorithmProvider(@NotNull String algorithm,
   static final class Builder
       implements MechanismsBuildStage, UsernameBuildStage, PasswordBuildStage, FinalBuildStage {
 
-    private ScramMechanism selectedScramMechanism;
-    private Collection<String> scramMechanisms;
-    private Gs2CbindFlag channelBinding = Gs2CbindFlag.CLIENT_NOT;
-    private StringPreparation stringPreparation = StringPreparation.SASL_PREPARATION;
-    private int nonceLength = 24;
-    private String nonce;
-    private SecureRandom secureRandom;
-    private String username;
-    private char[] password;
-    private byte[] saltedPassword;
-    private byte[] clientKey;
-    private byte[] serverKey;
-    private String cbindType;
-    private byte[] cbindData;
-    private String authzid;
-    private Supplier<String> nonceSupplier;
+    ScramMechanism selectedScramMechanism;
+    Collection<String> scramMechanisms;
+    Gs2CbindFlag channelBinding = Gs2CbindFlag.CLIENT_NOT;
+    StringPreparation stringPreparation = StringPreparation.SASL_PREPARATION;
+    int nonceLength = 24;
+    String nonce;
+    SecureRandom secureRandom;
+    String username;
+    char[] password;
+    byte[] saltedPassword;
+    byte[] clientKey;
+    byte[] serverKey;
+    String cbindType;
+    byte[] cbindData;
+    String authzid;
+    Supplier<String> nonceSupplier;
 
     private Builder() {
       // called from ScramClient.builder()
@@ -408,7 +408,7 @@ public FinalBuildStage stringPreparation(@NotNull StringPreparation stringPrepar
     @Override
     public FinalBuildStage channelBinding(@Nullable String cbindType, byte @Nullable [] cbindData) {
       this.cbindType = cbindType;
-      this.cbindData = cbindData;
+      this.cbindData = cbindData != null ? cbindData.clone() : null;
       this.channelBinding = cbindType != null && cbindData != null
           && !cbindType.isEmpty() && cbindData.length > 0
               ? Gs2CbindFlag.CLIENT_YES_SERVER_NOT

scram-common/src/main/java/com/ongres/scram/common/ClientFinalMessage.java

@@ -104,11 +104,12 @@ public byte[] getProof() {
   }
 
   private static void checkChannelBinding(Gs2Header gs2Header, byte[] cbindData) {
-    if (gs2Header.getChannelBindingFlag() == Gs2CbindFlag.CHANNEL_BINDING_REQUIRED
+    final Gs2CbindFlag channelBindingFlag = gs2Header.getChannelBindingFlag();
+    if (channelBindingFlag == Gs2CbindFlag.CHANNEL_BINDING_REQUIRED
         && null == cbindData) {
       throw new IllegalArgumentException("Channel binding data is required");
     }
-    if (gs2Header.getChannelBindingFlag() != Gs2CbindFlag.CHANNEL_BINDING_REQUIRED
+    if (channelBindingFlag != Gs2CbindFlag.CHANNEL_BINDING_REQUIRED
         && null != cbindData) {
       throw new IllegalArgumentException("Channel binding data should not be present");
     }

scram-common/src/main/java/com/ongres/scram/common/ScramFunctions.java

@@ -259,7 +259,7 @@ public static String nonce(int nonceSize, SecureRandom random) {
   public static String authMessage(ClientFirstMessage clientFirstMessage,
       ServerFirstMessage serverFirstMessage, byte[] cbindData) {
     StringBuilder sb = clientFirstMessage.clientFirstMessageBare(new StringBuilder(96))
-        .append(",").append(serverFirstMessage).append(",");
+        .append(',').append(serverFirstMessage).append(',');
     ClientFinalMessage.withoutProof(sb, clientFirstMessage.getGs2Header(),
         cbindData, serverFirstMessage.getNonce());
     return sb.toString();

scram-common/src/main/java/com/ongres/scram/common/exception/ServerErrorValue.java

@@ -5,9 +5,8 @@
 
 package com.ongres.scram.common.exception;
 
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 
 /**
  * This attribute specifies an error that occurred during authentication exchange. It is sent by the
@@ -16,14 +15,14 @@
  */
 public final class ServerErrorValue {
 
-  private static final Map<String, String> ERROR_MESSAGE = initServerErrorValue();
+  private static final ConcurrentMap<String, String> ERROR_MESSAGE = initServerErrorValue();
 
   private ServerErrorValue() {
     throw new IllegalStateException();
   }
 
-  private static Map<String, String> initServerErrorValue() {
-    Map<String, String> map = new HashMap<>();
+  private static ConcurrentMap<String, String> initServerErrorValue() {
+    ConcurrentMap<String, String> map = new ConcurrentHashMap<>();
     map.put("invalid-encoding", "The message format or encoding is incorrect");
     map.put("extensions-not-supported", "Requested extensions are not recognized by the server");
     map.put("invalid-proof", "The client-provided proof is invalid");
@@ -39,7 +38,7 @@ private static Map<String, String> initServerErrorValue() {
         "The username encoding is invalid (either invalid UTF-8 or SASLprep failure)");
     map.put("no-resources", "The server lacks resources to process the request");
     map.put("other-error", "A generic error occurred that doesn't fit into other categories");
-    return Collections.unmodifiableMap(map);
+    return map;
   }
 
   /**

scram-common/src/main/java/com/ongres/scram/common/util/TlsServerEndpoint.java

@@ -41,24 +41,24 @@ private TlsServerEndpoint() {
    * @see <a href="https://www.rfc-editor.org/rfc/rfc5929#section-4.1">The tls-server-end-point
    *      Channel Binding Type</a>
    */
-  private static MessageDigest getDigestAlgorithm(String signatureAlgorithm) {
+  private static MessageDigest getDigestAlgorithm(final String signatureAlgorithm) {
     int index = signatureAlgorithm.indexOf("with");
-    signatureAlgorithm = index > 0 ? signatureAlgorithm.substring(0, index) : "SHA-256";
+    String algorithm = index > 0 ? signatureAlgorithm.substring(0, index) : "SHA-256";
     // if the certificate's signatureAlgorithm uses a single hash
     // function and that hash function neither MD5 nor SHA-1, then use
     // the hash function associated with the certificate's signatureAlgorithm.
-    if (!signatureAlgorithm.startsWith("SHA3-")) {
-      signatureAlgorithm = signatureAlgorithm.replace("SHA", "SHA-");
+    if (!algorithm.startsWith("SHA3-")) {
+      algorithm = algorithm.replace("SHA", "SHA-");
     }
     // if the certificate's signatureAlgorithm uses a single hash
     // function, and that hash function is either MD5 [RFC1321] or SHA-1
     // [RFC3174], then use SHA-256 [FIPS-180-3]
-    if ("MD5".equals(signatureAlgorithm) || "SHA-1".equals(signatureAlgorithm)) {
-      signatureAlgorithm = "SHA-256";
+    if ("MD5".equals(algorithm) || "SHA-1".equals(algorithm)) {
+      algorithm = "SHA-256";
     }
 
     try {
-      return MessageDigest.getInstance(signatureAlgorithm);
+      return MessageDigest.getInstance(algorithm);
     } catch (NoSuchAlgorithmException e) {
       return null;
     }

scram-parent/pom.xml

@@ -100,7 +100,7 @@
     <spotbugs.version>4.8.4</spotbugs.version>
     <spotbugs-plugin.version>4.8.4.0</spotbugs-plugin.version>
     <findsecbugs.version>1.11.0</findsecbugs.version>
-    <pmd.version>7.0.0</pmd.version>
+    <pmd.version>7.1.0</pmd.version>
     <pmd-plugin.version>3.22.0</pmd-plugin.version>
     <forbiddenapis.version>3.7</forbiddenapis.version>
     <checks.location>${maven.multiModuleProjectDirectory}/checks</checks.location>
@@ -617,7 +617,7 @@
             <artifactId>maven-pmd-plugin</artifactId>
             <version>${pmd-plugin.version}</version>
             <configuration>
-              <failurePriority>2</failurePriority>
+              <failurePriority>5</failurePriority>
               <failOnViolation>true</failOnViolation>
               <printFailingErrors>true</printFailingErrors>
               <linkXRef>false</linkXRef>