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.
