showuon commented on a change in pull request #11430:
URL: https://github.com/apache/kafka/pull/11430#discussion_r736150581



##########
File path: 
clients/src/main/java/org/apache/kafka/common/security/JaasConfig.java
##########
@@ -50,12 +50,24 @@
     private final List<AppConfigurationEntry> configEntries;
 
     public JaasConfig(String loginContextName, String jaasConfigParams) {
+        // All characters except space, comment, quote, equal and semicolon 
are considered to be alphabetic.
+        // That is, numbers or symbols like '@' now can be a part of a word.
+        // All bytes from 0 to ' ' {@code ' '} are considered to be whitespace.
+        // '/' {@code '/'} is a comment character. '//', '/*', '*/' are also 
allowed.
+        // Single quote {@code '\u005C''} and double quote {@code '"'} are 
considered to be quote.
+        // Ends of lines are treated as white space, not as separate tokens.
         StreamTokenizer tokenizer = new StreamTokenizer(new 
StringReader(jaasConfigParams));
+        tokenizer.resetSyntax();
+        tokenizer.wordChars(32, 128); //

Review comment:
       redundant  `//`

##########
File path: 
clients/src/main/java/org/apache/kafka/common/security/JaasConfig.java
##########
@@ -50,12 +50,24 @@
     private final List<AppConfigurationEntry> configEntries;
 
     public JaasConfig(String loginContextName, String jaasConfigParams) {
+        // All characters except space, comment, quote, equal and semicolon 
are considered to be alphabetic.
+        // That is, numbers or symbols like '@' now can be a part of a word.
+        // All bytes from 0 to ' ' {@code ' '} are considered to be whitespace.
+        // '/' {@code '/'} is a comment character. '//', '/*', '*/' are also 
allowed.
+        // Single quote {@code '\u005C''} and double quote {@code '"'} are 
considered to be quote.
+        // Ends of lines are treated as white space, not as separate tokens.
         StreamTokenizer tokenizer = new StreamTokenizer(new 
StringReader(jaasConfigParams));
+        tokenizer.resetSyntax();
+        tokenizer.wordChars(32, 128); //
+        tokenizer.wordChars(128 + 32, 255);
+        tokenizer.ordinaryChar(';');
+        tokenizer.ordinaryChar('=');
+        tokenizer.whitespaceChars(0, ' ');

Review comment:
       So, we make the numbers as "whitespace". Does that mean, the password 
"abc12" will equals to "abc23"?

##########
File path: 
clients/src/main/java/org/apache/kafka/common/security/JaasConfig.java
##########
@@ -50,12 +50,24 @@
     private final List<AppConfigurationEntry> configEntries;
 
     public JaasConfig(String loginContextName, String jaasConfigParams) {
+        // All characters except space, comment, quote, equal and semicolon 
are considered to be alphabetic.
+        // That is, numbers or symbols like '@' now can be a part of a word.

Review comment:
       This line is not required.

##########
File path: 
clients/src/main/java/org/apache/kafka/common/security/JaasConfig.java
##########
@@ -50,12 +50,24 @@
     private final List<AppConfigurationEntry> configEntries;
 
     public JaasConfig(String loginContextName, String jaasConfigParams) {
+        // All characters except space, comment, quote, equal and semicolon 
are considered to be alphabetic.
+        // That is, numbers or symbols like '@' now can be a part of a word.
+        // All bytes from 0 to ' ' {@code ' '} are considered to be whitespace.
+        // '/' {@code '/'} is a comment character. '//', '/*', '*/' are also 
allowed.
+        // Single quote {@code '\u005C''} and double quote {@code '"'} are 
considered to be quote.
+        // Ends of lines are treated as white space, not as separate tokens.

Review comment:
       Could we put the java doc like this:
   ```
   // All characters except space, comment, quote, equal and semicolon are 
considered to be alphabetic.
   // tokenizer rules:
   // 1. All bytes from 0 to ' ' {@code ' '} are considered to be whitespace.
   // 2. '/' {@code '/'} is a comment character. '//', '/*', '*/' are also 
allowed.
   // 3. ....
   ```
   
   What do you think?

##########
File path: 
clients/src/main/java/org/apache/kafka/common/security/JaasConfig.java
##########
@@ -50,12 +50,24 @@
     private final List<AppConfigurationEntry> configEntries;
 
     public JaasConfig(String loginContextName, String jaasConfigParams) {
+        // All characters except space, comment, quote, equal and semicolon 
are considered to be alphabetic.
+        // That is, numbers or symbols like '@' now can be a part of a word.
+        // All bytes from 0 to ' ' {@code ' '} are considered to be whitespace.
+        // '/' {@code '/'} is a comment character. '//', '/*', '*/' are also 
allowed.

Review comment:
       Why do we need 2 identical chars in java doc? 
   ex: 0 to `' ' {@code ' '}` are considered to be whitespace.
   `'/' {@code '/'}` is...




-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to