dongjoon-hyun commented on code in PR #58041:
URL: https://github.com/apache/spark/pull/58041#discussion_r3798364375
##########
connector/credential-aws/src/test/java/org/apache/spark/security/aws/AwsStsCredentialProviderSuite.java:
##########
@@ -796,88 +737,44 @@ public void testInitWithValidCustomSessionName() {
@Test
public void testInitWithInvalidSessionNameContainingSpace() {
- Map<String, String> conf = new HashMap<>();
- conf.put(AwsStsCredentialProvider.CONF_ROLE_ARN, TEST_ROLE_ARN);
- conf.put(AwsStsCredentialProvider.CONF_SESSION_NAME, "bad session");
-
- AwsStsCredentialProvider p = new AwsStsCredentialProvider();
- IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
- () -> p.init(conf));
-
assertTrue(ex.getMessage().contains(AwsStsCredentialProvider.CONF_SESSION_NAME),
- "Error must name the config key");
+ IllegalArgumentException ex = assertInitThrowsForConfig(
+ AwsStsCredentialProvider.CONF_SESSION_NAME, "bad session");
assertTrue(ex.getMessage().contains("bad session"),
"Error must echo the bad value");
}
@Test
public void testInitWithInvalidSessionNameContainingQuestionMark() {
- Map<String, String> conf = new HashMap<>();
- conf.put(AwsStsCredentialProvider.CONF_ROLE_ARN, TEST_ROLE_ARN);
- conf.put(AwsStsCredentialProvider.CONF_SESSION_NAME, "bad?name");
-
- AwsStsCredentialProvider p = new AwsStsCredentialProvider();
- IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
- () -> p.init(conf));
-
assertTrue(ex.getMessage().contains(AwsStsCredentialProvider.CONF_SESSION_NAME));
+ assertInitThrowsForConfig(AwsStsCredentialProvider.CONF_SESSION_NAME,
"bad?name");
}
@Test
public void testInitWithSessionNameTooShort() {
- Map<String, String> conf = new HashMap<>();
- conf.put(AwsStsCredentialProvider.CONF_ROLE_ARN, TEST_ROLE_ARN);
- conf.put(AwsStsCredentialProvider.CONF_SESSION_NAME, "x");
-
- AwsStsCredentialProvider p = new AwsStsCredentialProvider();
- IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
- () -> p.init(conf));
-
assertTrue(ex.getMessage().contains(AwsStsCredentialProvider.CONF_SESSION_NAME));
+ IllegalArgumentException ex = assertInitThrowsForConfig(
+ AwsStsCredentialProvider.CONF_SESSION_NAME, "x");
assertTrue(ex.getMessage().contains("x"));
}
@Test
public void testInitWithSessionNameTooLong() {
- Map<String, String> conf = new HashMap<>();
- conf.put(AwsStsCredentialProvider.CONF_ROLE_ARN, TEST_ROLE_ARN);
- conf.put(AwsStsCredentialProvider.CONF_SESSION_NAME, "a".repeat(65));
-
- AwsStsCredentialProvider p = new AwsStsCredentialProvider();
- IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
- () -> p.init(conf));
-
assertTrue(ex.getMessage().contains(AwsStsCredentialProvider.CONF_SESSION_NAME));
+ assertInitThrowsForConfig(AwsStsCredentialProvider.CONF_SESSION_NAME,
"a".repeat(65));
}
// ========== Session name: non-ASCII rejection (regression) ==========
@Test
public void testInitRejectsSessionNameWithAccentedChar() {
- // "caf" + (char) 0xE9 produces "cafe" with accented 'e' -- valid under \w
but
- // NOT valid in STS session names. This test would PASS (incorrectly)
under the
- // buggy \w pattern and must FAIL (correctly) under the explicit ASCII
pattern.
- // NOTE: (char) 0xNN construction avoids unicode escapes banned by Spark
- // checkstyle (AvoidEscapedUnicodeCharacters) while keeping source bytes
ASCII.
- Map<String, String> conf = new HashMap<>();
- conf.put(AwsStsCredentialProvider.CONF_ROLE_ARN, TEST_ROLE_ARN);
- conf.put(AwsStsCredentialProvider.CONF_SESSION_NAME, "caf" + (char) 0xE9);
-
- AwsStsCredentialProvider p = new AwsStsCredentialProvider();
- IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
- () -> p.init(conf));
-
assertTrue(ex.getMessage().contains(AwsStsCredentialProvider.CONF_SESSION_NAME));
+ // Accented 'é' is valid under \w but NOT valid in STS session names.
Review Comment:
Please avoid this non-ASCII, @sarutak .
https://github.com/apache/spark/blob/95d93a0116ac5e4a7229a87dfa7b7c85638e3f90/AGENTS.md?plain=1#L23
##########
connector/credential-aws/src/test/java/org/apache/spark/security/aws/AwsStsCredentialProviderSuite.java:
##########
@@ -796,88 +737,44 @@ public void testInitWithValidCustomSessionName() {
@Test
public void testInitWithInvalidSessionNameContainingSpace() {
- Map<String, String> conf = new HashMap<>();
- conf.put(AwsStsCredentialProvider.CONF_ROLE_ARN, TEST_ROLE_ARN);
- conf.put(AwsStsCredentialProvider.CONF_SESSION_NAME, "bad session");
-
- AwsStsCredentialProvider p = new AwsStsCredentialProvider();
- IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
- () -> p.init(conf));
-
assertTrue(ex.getMessage().contains(AwsStsCredentialProvider.CONF_SESSION_NAME),
- "Error must name the config key");
+ IllegalArgumentException ex = assertInitThrowsForConfig(
+ AwsStsCredentialProvider.CONF_SESSION_NAME, "bad session");
assertTrue(ex.getMessage().contains("bad session"),
"Error must echo the bad value");
}
@Test
public void testInitWithInvalidSessionNameContainingQuestionMark() {
- Map<String, String> conf = new HashMap<>();
- conf.put(AwsStsCredentialProvider.CONF_ROLE_ARN, TEST_ROLE_ARN);
- conf.put(AwsStsCredentialProvider.CONF_SESSION_NAME, "bad?name");
-
- AwsStsCredentialProvider p = new AwsStsCredentialProvider();
- IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
- () -> p.init(conf));
-
assertTrue(ex.getMessage().contains(AwsStsCredentialProvider.CONF_SESSION_NAME));
+ assertInitThrowsForConfig(AwsStsCredentialProvider.CONF_SESSION_NAME,
"bad?name");
}
@Test
public void testInitWithSessionNameTooShort() {
- Map<String, String> conf = new HashMap<>();
- conf.put(AwsStsCredentialProvider.CONF_ROLE_ARN, TEST_ROLE_ARN);
- conf.put(AwsStsCredentialProvider.CONF_SESSION_NAME, "x");
-
- AwsStsCredentialProvider p = new AwsStsCredentialProvider();
- IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
- () -> p.init(conf));
-
assertTrue(ex.getMessage().contains(AwsStsCredentialProvider.CONF_SESSION_NAME));
+ IllegalArgumentException ex = assertInitThrowsForConfig(
+ AwsStsCredentialProvider.CONF_SESSION_NAME, "x");
assertTrue(ex.getMessage().contains("x"));
}
@Test
public void testInitWithSessionNameTooLong() {
- Map<String, String> conf = new HashMap<>();
- conf.put(AwsStsCredentialProvider.CONF_ROLE_ARN, TEST_ROLE_ARN);
- conf.put(AwsStsCredentialProvider.CONF_SESSION_NAME, "a".repeat(65));
-
- AwsStsCredentialProvider p = new AwsStsCredentialProvider();
- IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
- () -> p.init(conf));
-
assertTrue(ex.getMessage().contains(AwsStsCredentialProvider.CONF_SESSION_NAME));
+ assertInitThrowsForConfig(AwsStsCredentialProvider.CONF_SESSION_NAME,
"a".repeat(65));
}
// ========== Session name: non-ASCII rejection (regression) ==========
@Test
public void testInitRejectsSessionNameWithAccentedChar() {
- // "caf" + (char) 0xE9 produces "cafe" with accented 'e' -- valid under \w
but
- // NOT valid in STS session names. This test would PASS (incorrectly)
under the
- // buggy \w pattern and must FAIL (correctly) under the explicit ASCII
pattern.
- // NOTE: (char) 0xNN construction avoids unicode escapes banned by Spark
- // checkstyle (AvoidEscapedUnicodeCharacters) while keeping source bytes
ASCII.
- Map<String, String> conf = new HashMap<>();
- conf.put(AwsStsCredentialProvider.CONF_ROLE_ARN, TEST_ROLE_ARN);
- conf.put(AwsStsCredentialProvider.CONF_SESSION_NAME, "caf" + (char) 0xE9);
-
- AwsStsCredentialProvider p = new AwsStsCredentialProvider();
- IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
- () -> p.init(conf));
-
assertTrue(ex.getMessage().contains(AwsStsCredentialProvider.CONF_SESSION_NAME));
+ // Accented 'é' is valid under \w but NOT valid in STS session names.
+ // This test would PASS (incorrectly) under the buggy \w pattern and must
+ // FAIL (correctly) under the explicit ASCII pattern.
+ assertInitThrowsForConfig(AwsStsCredentialProvider.CONF_SESSION_NAME,
"café");
}
@Test
public void testInitRejectsSessionNameWithCjkChar() {
- // CJK ideograph U+4E16 ('world' in Chinese) -- valid under \w but NOT
valid
- // in STS session names. Regression test for the ASCII-only fix.
- // NOTE: (char) 0xNN construction avoids unicode escapes banned by
checkstyle.
- Map<String, String> conf = new HashMap<>();
- conf.put(AwsStsCredentialProvider.CONF_ROLE_ARN, TEST_ROLE_ARN);
- conf.put(AwsStsCredentialProvider.CONF_SESSION_NAME, "session" + (char)
0x4E16);
-
- AwsStsCredentialProvider p = new AwsStsCredentialProvider();
- IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
- () -> p.init(conf));
-
assertTrue(ex.getMessage().contains(AwsStsCredentialProvider.CONF_SESSION_NAME));
+ // CJK ideograph '世' is valid under \w but NOT valid in STS session names.
Review Comment:
Ditto.
https://github.com/apache/spark/blob/95d93a0116ac5e4a7229a87dfa7b7c85638e3f90/AGENTS.md?plain=1#L23
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
