Code review request: 7158329: NPE in sun.security.krb5.Credentials.acquireDefaultCreds()

[Jonathan Gibbons]

Max,

You definitely don't need /othervm, since the test itself is talking 
care to run the subprocess.

Generally, I like to use pb.redirectErrorStream(true) to merge stdout 
and stderr, so that you are guaranteed that neither will fill up the 
buffer and block for want of a reader.

There's lots of ways to write this sort of code; yours is as good as 
any, but when the command line is of fixed length, I like to use 
Arrays.asList() to minimize the boilerplate, and make it as much like a 
command line as possible.

In the subprocess, you are relying on the JVM to set an exit code as a 
result of your throwing the exception. You might want to replace line 78 
with a more explicit call of System.exit(1); or similar.

-- Jon

On 04/05/2012 07:35 PM, Weijun Wang wrote:
> Webrev updated:
>
>   http://cr.openjdk.java.net/~weijun/7158329/webrev.01
>
> No change to src.
>
> *Jon*: Is this the correct style to use ProcessBuilder to launch a 
> test? I guess there is no need to go othervm?
>
> *Valerie*: Can you take a review on this?
>
> Thanks
> Max
>
> On 04/06/2012 02:29 AM, Jonathan Gibbons wrote:
>> Max,
>>
>> If it were me writing the test, I'd avoid using a shell script and would
>> write Java code using ProcessBuilder to set an env-var and then realunch
>> the test via new File(new File(System.getProperty("java.home"), "bin"),
>> "java");
>>
>> This Java code could even be co-located in the EmptyCC.java file.
>>
>> -- Jon
>>
>>
>> On 04/05/2012 02:32 AM, Weijun Wang wrote:
>>> The webrev is at
>>>
>>> http://cr.openjdk.java.net/~weijun/7158329/webrev.00/
>>>
>>> There are two places where the content (getDefaultCreds) of a cache
>>> might be null, one with a specified ccache file name, one default. In
>>> order to check for both, a KRB5CCNAME environment variable is needed.
>>> Therefore the test must be a script calling a Java program.
>>>
>>> *Jon*: I guess this is the only way to feed an environment variable to
>>> a Java test?
>>>
>>> Thanks
>>> Max
>>>
>>> -------- Original Message --------
>>> *Change Request ID*: 7158329
>>> *Synopsis*: NPE in sun.security.krb5.Credentials.acquireDefaultCreds()
>>>
>>> Product: java
>>> Category: java
>>> Subcategory: classes_security
>>> Type: Defect
>>>
>>> === *Description*
>>> ============================================================
>>> FULL PRODUCT VERSION :
>>> java version "1.6.0_26"
>>> Java(TM) SE Runtime Environment (build 1.6.0_26-b03)
>>> Java HotSpot(TM) Client VM (build 20.1-b02, mixed mode, sharing)
>>>
>>> ADDITIONAL OS VERSION INFORMATION :
>>> Microsoft Windows XP [Version 5.1.2600]
>>>
>>> A DESCRIPTION OF THE PROBLEM :
>>> Trying to invoke login with Krb5LoginModule, debug=false,
>>> doNotPrompt=true, useTicketCache=true, storeKey=false.
>>>
>>> When an empty file krb5cc_<username> in <userhome> exists, this throws
>>> a NullPointerException in
>>> sun.security.krb5.Credentials.acquireDefaultCreds().
>>>
>>> The cause is the following code:
>>>
>>> if (cache == null) {
>>> cache = CredentialsCache.getInstance();
>>> }
>>> if (cache != null) {
>>> if (DEBUG) {
>>> System.out.println(">>> KrbCreds found the default ticket " +
>>> "granting ticket in credential cache.");
>>> }
>>> sun.security.krb5.internal.ccache.Credentials temp =
>>> cache.getDefaultCreds();
>>> if (EType.isSupported(temp.getEType())) {
>>> result = temp.setKrbCreds();
>>> } else {
>>> if (DEBUG) {
>>> System.out.println(
>>> ">>> unsupported key type found the default TGT: " +
>>> temp.getEType());
>>> }
>>> }
>>> }
>>>
>>> where cache.getDefaultCreds() can and will return null in case the
>>> ticket cache is empty, so the EType.isSupported(...) fails.
>>>
>>>
>>> REPRODUCIBILITY :
>>> This bug can be reproduced always.
>>>
>>