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



##########
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.
+        // Tokenizer rules:
+        // 1. All bytes from 0 to 32 ({@code ' '}) are considered to be 
whitespace.
+        // 2. {@code '/'} (47) is a comment character. '//', '/*', '*/' are 
also allowed.
+        // 3. Single quote ({@code '\u005C''}, 39) and double quote ({@code 
'"'}, 34) are considered to be quote.
+        // 4. Ends of lines are treated as white space, not as separate tokens.
         StreamTokenizer tokenizer = new StreamTokenizer(new 
StringReader(jaasConfigParams));
-        tokenizer.slashSlashComments(true);
-        tokenizer.slashStarComments(true);
-        tokenizer.wordChars('-', '-');
-        tokenizer.wordChars('_', '_');
-        tokenizer.wordChars('$', '$');
+        tokenizer.resetSyntax();            // Reset the default configuration.
+        tokenizer.wordChars(32, 128);       // All characters in [32, 128] are 
allowed.
+        tokenizer.wordChars(128 + 32, 255); // All characters in [160, 255] 
are allowed.
+        tokenizer.ordinaryChar(';');        // ';' is treated as a reserved 
word.
+        tokenizer.ordinaryChar('=');        // '=' is treated as a reserved 
word.
+        tokenizer.whitespaceChars(0, ' ');  // All characters in [0, 32] 
(including ' ') are treated as space character.
+        tokenizer.commentChar('/');         // '/' is treated as a comment 
character.
+        tokenizer.quoteChar('"');           // '"' is treated as a quote.
+        tokenizer.quoteChar('\'');          // ''' is treated as a quote.
+        tokenizer.slashSlashComments(true); // Allow '//' comments.
+        tokenizer.slashStarComments(true);  // Allow '/*', '*/' comments.

Review comment:
       Hi @rajinisivaram,
   
   I searched the definition of Java's standard file-based JAAS `Configuration` 
but could not find one. But, after reviewing [the related `LoginModule` 
implementations](https://docs.oracle.com/javase/10/security/appendix-b-jaas-login-configuration-file.htm)
 
([#1](https://github.com/openjdk/jdk/blob/jdk-9%2B181/jdk/src/jdk.security.auth/share/classes/com/sun/security/auth/module/KeyStoreLoginModule.java),
 
[#2](https://github.com/openjdk/jdk/blob/jdk-9%2B181/jdk/src/jdk.security.auth/share/classes/com/sun/security/auth/module/Krb5LoginModule.java))
 and 
[example](https://docs.cloudera.com/HDPDocuments/HDP3/HDP-3.1.4/security-reference/content/kerberos_nonambari_create_jaas_configuration_files.html),
 what we need to test seems almost clear:
   
   - It should be able to parse a string (including non-reserved symbols), 
number, boolean, asterisk, canonical java class name, and URL; regardless it is 
quoted or not.
   - **We should remove the extended ASCII codes (i.e., [128, 256)) from the 
PR.** That is, we should allow 7-bit alphabets and symbols only.
   
   Is this okay? Then, I will proceed to update the tests accordingly.




-- 
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