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

Jayashree Huttanagoudar duke at openjdk.org
Mon Aug 22 08:36:47 UTC 2022


On Fri, 19 Aug 2022 21:35:17 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Jayashree Huttanagoudar has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Address review 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.

ok

> test/jdk/javax/security/auth/login/LoginContext/OptionalJaas/UnixNTPlatform.java line 27:
> 
>> 25:  * @test
>> 26:  * @bug 8215916
>> 27:  * @summary This Sample application attempts to authenticate a user
> 
> Update the summary.

Ok

> 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`. The config file name can be hardcoded.

ok

> 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`.

I will try

> 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?

I am not sure about that. Let me give a try by removing this and see what happens.

> 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..

Ok. But at present I am not getting why we should do this :) 
I mean, is  it going to improvise something or helpful ?

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

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



More information about the security-dev mailing list