Hi Max,
Here are my comments:
* KerberosKey
37: Did you mean to use @link instead of @see?
- several methods are now defined to throw IllegalStateExc, but you
(perhaps accidentally) removed the code that does that (ex: getKeyType).
* KerberosTicket
352-3: put braces around the if (destroyed) statement; same comment
applies in rest of code.
* Krb5Context
1446-9: can myName or peerName be null?
* Context
451: I think you should also print the exception
* KerberosCredMessage
52-4: these fields should be private
- is there a reason why you didn't override hashCode/equals?
* KerberosSessionKey
- methods don't need to be final since class is
- some of the methods don't check if it is destroyed (ex: getKeyType)
- I think it would actually be better to name this class EncryptionKey
since that is what it is and that way we would be able to reuse this
type in the future if necessary for other structures or APIs that
reference this type (EncryptionKey)
--Sean
On 07/04/2014 02:12 AM, Wang Weijun wrote:
Hi All
Please review the change at
http://cr.openjdk.java.net/~weijun/8043071/webrev.00/
Two new inquire type KRB5_GET_SESSION_KEY_EX and KRB5_GET_KRB_CRED are added to
get the session key (in a new format) and the KRB_CRED message. Two new classes
are created as the types of their return values.
Spec for InquireType values are moved into the InquireType class itself.
For the existing KerberosKey class, the "A call to any of its other methods after
this will cause an IllegalStateException to be thrown" in the spec of destroy() is
not precise since we know isDestroyed() and toString/hashCode/equals() do not throw it.
The sentence is removed and a @throws clause is added to those methods that do throw the
exception. The new KerberosSessionKey and KerberosCredMessage have the same style.
Since the data class KeyImpl already throws IllegalStateException when it's
destroyed, KerberosKey and KerberosSessionKey do not check again. If you think
this makes the code difficult to read, I can add them back.
Thanks
Max
