AndrewJSchofield commented on code in PR #23909:
URL: https://github.com/apache/kafka/pull/23909#discussion_r4371969556


##########
clients/src/test/java/org/apache/kafka/common/security/scram/internals/ScramMessagesTest.java:
##########
@@ -166,6 +167,26 @@ public void validServerFirstMessage() throws SaslException 
{
         }
     }
 
+    @Test
+    public void serverFirstMessageUsesAsciiDigitsRegardlessOfDefaultLocale() 
throws SaslException {
+        String clientNonce = formatter.secureRandomString();
+        String serverNonce = formatter.secureRandomString();
+        String nonce = clientNonce + serverNonce;
+        String salt = randomBytesAsString();
+
+        // Under a default locale with native digits (e.g. ar-EG formats %d as 
Arabic-Indic digits) the
+        // iteration count must still be sent as ASCII digits, otherwise 
clients cannot parse the message
+        Locale defaultLocale = Locale.getDefault();
+        Locale.setDefault(Locale.forLanguageTag("ar-EG"));

Review Comment:
   This is a JVM-wide global state which is being changed. Consider 
`@ResourceLock(Resources.LOCALE)` to ensure that concurrent execution of other 
tests will not fall foul of the local change.



##########
clients/src/main/java/org/apache/kafka/common/security/scram/internals/ScramCredentialUtils.java:
##########
@@ -41,7 +42,8 @@ public final class ScramCredentialUtils {
     private ScramCredentialUtils() {}
 
     public static String credentialToString(ScramCredential credential) {

Review Comment:
   This appears to be code that is no longer used, apart from a handful of 
tests.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to