dongjinleekr commented on a change in pull request #11430: URL: https://github.com/apache/kafka/pull/11430#discussion_r741090237
########## 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 reviewed the implementation of `sun.security.provider.ConfigFile` [here](https://github.com/openjdk/jdk/blob/jdk-9%2B181/jdk/src/java.base/share/classes/sun/security/provider/ConfigFile.java#L413) and found the following: - `ConfigFile` treats only '\*', '\_', '-', '$' as word characters; that is, the numbers or other symbols like '%', '^' are not allowed by default - if the user needs to use these symbols, they should use quotes. So, **the problem described in the Jira issue is actually not a problem, but rather a documentation issue.** Instead, we need to add '*' as a word character. (Currently, it is not.) Obviously, it is a bug. - It seems like we don't need to define the ordinary characters (like ';', '='), spaces, comment characters, and quote characters explicitly, like `ConfigFile` does. - It would be better to improve `JaasContextTest` by not only checking it works like the documentation, but also checking it works like the original, `ConfigFile` implementation. I greatly appreciate you for guiding the right direction. I am now updating the PR now. Thanks again and stay tuned! :smiley: -- 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