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