RFR: 8215916: The failure reason of an optional JAAS LoginModule is not logged [v16]

Weijun Wang weijun at openjdk.org
Fri Aug 19 21:46:00 UTC 2022


On Fri, 19 Aug 2022 20:33:23 GMT, Jayashree Huttanagoudar <duke at openjdk.org> wrote:

>> Could you please review the changes?
>> This patch is to address : https://bugs.openjdk.org/browse/JDK-8215916?jql=labels%20%3D%20starter-bug
>
> Jayashree Huttanagoudar has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Address review comments

Almost done. Some comments.

test/jdk/javax/security/auth/login/LoginContext/OptionalJaas/UnixNTPlatform.java line 2:

> 1: /*
> 2:  * Copyright (c) 2014, 2022, Oracle and/or its affiliates. All rights reserved.

Only "Copyright (c) 2022" is needed. Actually, you don't need to include "Oracle" here. See https://github.com/openjdk/jdk/blob/3e60e828148a0490a4422d0724d15f3eccec17f0/test/jdk/sun/security/pkcs11/Cipher/TestPaddingOOB.java for an example.

test/jdk/javax/security/auth/login/LoginContext/OptionalJaas/UnixNTPlatform.java line 50:

> 48:     }
> 49: 
> 50:     static void login(String test, String... conf) throws Exception {

All arguments are useless now. You can simply remove this `login` method and merge the content into `main`.

test/jdk/javax/security/auth/login/LoginContext/OptionalJaas/UnixNTPlatform.java line 61:

> 59: 
> 60:         System.out.println("config is : \n"+config);
> 61:         Files.write(java.nio.file.Path.of(test), config.toString().getBytes());

Use `Files.writeString`.

test/jdk/javax/security/auth/login/LoginContext/OptionalJaas/UnixNTPlatform.java line 64:

> 62: 
> 63:         // Must be called. Configuration has an internal static field.
> 64:         Configuration.setConfiguration(null);

Is this necessary since you run this test in othervm mode?

test/jdk/javax/security/auth/login/LoginContext/OptionalJaas/UnixNTPlatform.java line 69:

> 67:         ByteArrayOutputStream stream = new ByteArrayOutputStream();
> 68:         PrintStream ps = new PrintStream(stream);
> 69:         System.setErr(ps);

Store `System.err` in a local variable so you can call `System.setErr(oldSystemErr)` in a `finally` clause of the `try` block at line 71 below..

-------------

PR: https://git.openjdk.org/jdk/pull/9159



More information about the security-dev mailing list