RFR 8042900: Allow com.sun.security.jgss to be in different module than org.ietf.jgss

[Alan Bateman]

On 27/08/2014 08:17, Wang Weijun wrote:
> Hi All
>
> Please review the code change at
>
>     http://cr.openjdk.java.net/~weijun/8042900/webrev.00/
>
> The purpose of this fix is to move com.sun.security.jgss (where the ExtendedGSSXXX classes reside) out of the java.security.jgss module, since it is openjdk-specific and not defined in Java SE. This is listed as the 1st open issue in http://openjdk.java.net/jeps/200.
>
> Because GSSManager::createContext() actually returns an ExtendedGSSContext object now, we will need to add a check there so that when the extended classes are available it still returns ExtendedGSSContext and otherwise a plain GSSContext. An internal OrgIetfJgssExtender class is defined and Extender in com.sun.security.jgss extends it. When GSSManagerImpl is creating a GSSContext (or GSSCredential) object, it detects whether an extension is available and if so returns a "wrapped" object that has extended functions.
>
> Please note that in this fix all extended functions are actually implemented in the basic (say, GSSContextImpl) class, but they are only exported through the extended interface (say, ExtendedGSSContext) to application developers.
>
> The detection of whether an extension is available is now performed by calling "Class.forName("com.sun.security.jgss.Extender")". This will be revisited later if we have other handy methods to detect whether a module is available or be converted to a service loader.
>
> A sub-task (JDK-8056141) will move com.sun.security.jgss and com.sun.security.sasl.gsskerb into a new jdk.security.jgss module.
>
>
I've looked through the changes, I don't claim to know this area very 
well and so I think it needs a Reviewer from the security area to help 
review too.

At a high-level then the approach seems reasonable and I see how this 
will fits with the second part that will facilitate the refactoring of 
com.sun.security.jgss to another module. The Class.forName to tickle the 
Extender to register seems okay.

A minor comment on Extender is that I think I'd prefer the methods to 
named "wrap" rather than "wrapped", also I assume you'll add a copyright 
header to the file before you push.

A minor comment on OrgIetfJgssExtender is that perhaps JgssExtender 
would be a simpler name. Can the setExtender method be protected rather 
than public, I assume it will only ever be called by sub-types. Should 
theOthe be volatile? I can't tell if there are any race conditions here. 
As per Extender I would have a preference for wrap rather than wrapped.

I don't think I understand the update to test S4U2selfGSS.java, is this 
related to this change?

Otherwise I don't see any issues and overall this seems good work.

-Alan.