bvn13 commented on a change in pull request #11430:
URL: https://github.com/apache/kafka/pull/11430#discussion_r736438767
##########
File path:
clients/src/test/java/org/apache/kafka/common/security/JaasContextTest.java
##########
@@ -172,14 +172,42 @@ public void testMissingSemicolon() throws Exception {
@Test
public void testNumericOptionWithoutQuotes() throws Exception {
- checkInvalidConfiguration("test.testNumericOptionWithoutQuotes
required option1=3;");
+ try {
+ Map<String, Object> options = new HashMap<>();
+ options.put("option", "3");
+ checkConfiguration("test.testNumericOptionWithoutQuotes required
option=3;", "test.testNumericOptionWithoutQuotes",
LoginModuleControlFlag.REQUIRED, options);
+ fail("Given Jaas config is parsed properly but
sun.security.provider.ConfigFile$Spi.<init> throws a IOException wrapped with a
SecurityException.");
+ } catch (SecurityException e) {
+ assertEquals(IOException.class, e.getCause().getClass());
+ }
}
@Test
public void testInvalidControlFlag() throws Exception {
checkInvalidConfiguration("test.testInvalidControlFlag { option1=3;");
}
+ @Test
+ public void testNumericWord() throws Exception {
+ Map<String, Object> options = new HashMap<>();
+ options.put("password", "k3fka");
Review comment:
Could you please add another test for checking case when password starts
with digit?
##########
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:
1. I am confused about a character passing into 'hi' param. I mean
you're using ' ' instead of ASCII code. I've checked: it is 32, it is a space.
Could it be replaced with ASCII code instead of character? It is like magic
number :)
2. You are using two settings for space character:
```
tokenizer.wordChars(32, 128);
tokenizer.whitespaceChars(0, ' ');
```
which one has higher priority?
--
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
