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

[Weijun Wang]

On 04/06/2012 11:31 AM, Jonathan Gibbons wrote:
> 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.

I have been relying on it all the time, say, in a test script. If this 
is not always true, does that mean I should wrap the whole subprocess in 
a try block and call System.exit(1) whenever an exception is thrown?

Thanks
Max

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