On Sep 9, 2014, at 6:25, Valerie Peng <valerie.p...@oracle.com> wrote:
> Max, > > Mostly look fine. Just some comments, questions (see below): > > <src/java.security.jgss/share/classes/com/sun/security/jgss/ExtendedGSSContext.java> > 1) line 71 - 76 was done in Krb5Context.java. Is it really necessary to move > it here? I don't see a reason to. You are right. I thought KerberosCredMessage is a com.sun.security.jgss class but actually it's in javax.security.auth.kerberos. > > <src/java.security.jgss/share/classes/sun/security/jgss/krb5/Krb5Context.java> > 1) line 1435, the cloning is removed? I didn't see a corresponding clone > being done in the caller, e.g. ExtendedGSSContext. Will add it back. > > <src/java.security.jgss/share/classes/sun/security/jgss/JgssExtender.java> > 1) the class description mentioned the registration process is hardcoded in > GSSManagerImpl. But it looks to me that it's actually done by the > com.sun.security.jgss.Extender class? It is done inside Extender. I meant the registration is triggered in GSSManagerImpl using Class.forName(). Will change the words. > 2) do you think we should require permissions for calls to set/getExtender()? The method is defined in a sun.security.jgss class so I guess we don't need a permission. > > <test/sun/security/krb5/auto/Context.java> > 1) line 364: move it inside the if-block? Seems no value calling xstatus() > when x is null. Correct. > 2) line 401: move it inside the if-block? Since if x is null, then there is > no output following this println() call. No block needed if I move line 364 into if-block. Thanks Max > > Thanks, > Valerie > > On 8/30/2014 6:59 PM, Weijun Wang wrote: >> Webrev updated at >> >> http://cr.openjdk.java.net/~weijun/8042900/webrev.01/ >> >> as per Alan's suggestions. >> >> Can anyone in the security team take a look? >> >> Thanks >> Max >>
