Re: RFR 8035986: KerberosKey algorithm names are not specified

2014-04-08 Thread Xuelei Fan
Looks fine to me.

I was wondering, whether it is a little bit more instinctive to return a
string with the type number for unknown and private algorithm in
KerberosKey.getAlgorithm().  For example:

unknown - kid-2014
private - kid-(2014)

Thanks,
Xuelei

On 4/8/2014 10:37 AM, Weijun Wang wrote:
 Hi All
 
 Please review the code changes at
 
http://cr.openjdk.java.net/~weijun/8035986/webrev.00/
 
 It's about using IANA names in KerberosKey instead of old non-standard
 names.
 
 Thanks
 Max



Re: RFR 8035986: KerberosKey algorithm names are not specified

2014-04-08 Thread Weijun Wang
There is already getKeyType() and toString(). Also I don't think 
kid-2014 is useful. If people really want to inspect the result, I 
expect they would fall into the default or else block anyway.


--Max

On 4/9/2014 7:57, Xuelei Fan wrote:

Looks fine to me.

I was wondering, whether it is a little bit more instinctive to return a
string with the type number for unknown and private algorithm in
KerberosKey.getAlgorithm().  For example:

 unknown - kid-2014
 private - kid-(2014)

Thanks,
Xuelei

On 4/8/2014 10:37 AM, Weijun Wang wrote:

Hi All

Please review the code changes at

http://cr.openjdk.java.net/~weijun/8035986/webrev.00/

It's about using IANA names in KerberosKey instead of old non-standard
names.

Thanks
Max




Re: RFR 8035986: KerberosKey algorithm names are not specified

2014-04-08 Thread Xuelei Fan
On 4/9/2014 8:53 AM, Weijun Wang wrote:
 There is already getKeyType() and toString().
;-) They should not lower the standards to design another good method.

 Also I don't think
 kid-2014 is useful. If people really want to inspect the result, I
 expect they would fall into the default or else block anyway.
 
There is a constructor to put unknown or private key type:
KerberosKey(KerberosPrincipal principal,
byte[] keyBytes,
int keyType, int versionNum)

Which will accept any kind of integer key type.

I think it might be help to get the algorithm in string even if key type
is not supported (getKeyType() is not as convenient as getAlgorithm() to
get the string algorithm, toString() covers too much information if one
only needs to know the algorithm).

   KerberosKey kk = new KerberosKey(..., 123, 0);
   String alg = kk.getAlgorithm();   // unknown returns

   KerberosKey kk = new KerberosKey(..., 124, 0);
   String alg = kk.getAlgorithm();   // unknown returns

   KerberosKey kk = new KerberosKey(..., -123, 0);
   String alg = kk.getAlgorithm();   // private returns

   KerberosKey kk = new KerberosKey(..., -124, 0);
   String alg = kk.getAlgorithm();   // private returns

At least for meaningful debug log or exception message, unknown and
private is not as instinctive as xxx-123 and xxx-124.

Anyway, not a big concern of mine.  Please go ahead if you prefer
unknown and private.

Xuelei

 --Max
 
 On 4/9/2014 7:57, Xuelei Fan wrote:
 Looks fine to me.

 I was wondering, whether it is a little bit more instinctive to return a
 string with the type number for unknown and private algorithm in
 KerberosKey.getAlgorithm().  For example:

  unknown - kid-2014
  private - kid-(2014)

 Thanks,
 Xuelei

 On 4/8/2014 10:37 AM, Weijun Wang wrote:
 Hi All

 Please review the code changes at

 http://cr.openjdk.java.net/~weijun/8035986/webrev.00/

 It's about using IANA names in KerberosKey instead of old non-standard
 names.

 Thanks
 Max




Re: RFR 8035986: KerberosKey algorithm names are not specified

2014-04-08 Thread Weijun Wang



On 4/9/2014 9:15, Xuelei Fan wrote:

On 4/9/2014 8:53 AM, Weijun Wang wrote:

There is already getKeyType() and toString().

;-) They should not lower the standards to design another good method.


I just meant different methods serve for different purposes.




Also I don't think
kid-2014 is useful. If people really want to inspect the result, I
expect they would fall into the default or else block anyway.


There is a constructor to put unknown or private key type:
 KerberosKey(KerberosPrincipal principal,
 byte[] keyBytes,
 int keyType, int versionNum)

Which will accept any kind of integer key type.


Yes, this method does not need to understand the keyType to generate a 
key. However, the one we are talking about now must understand the 
algorithm name and call its string2key() method to generate the key from 
a passphrase. So even if you provide kid-2014, it still has to throw 
an IllegalArgumentException.




I think it might be help to get the algorithm in string even if key type
is not supported (getKeyType() is not as convenient as getAlgorithm() to
get the string algorithm, toString() covers too much information if one
only needs to know the algorithm).

KerberosKey kk = new KerberosKey(..., 123, 0);
String alg = kk.getAlgorithm();   // unknown returns

KerberosKey kk = new KerberosKey(..., 124, 0);
String alg = kk.getAlgorithm();   // unknown returns

KerberosKey kk = new KerberosKey(..., -123, 0);
String alg = kk.getAlgorithm();   // private returns

KerberosKey kk = new KerberosKey(..., -124, 0);
String alg = kk.getAlgorithm();   // private returns


I would expect actual developers calling getKeyType() more often because 
it's easy to deal with in Kerberos. In this sense, getAlgorithm() only 
exists to override the method in Key.




At least for meaningful debug log or exception message, unknown and
private is not as instinctive as xxx-123 and xxx-124.

Anyway, not a big concern of mine.  Please go ahead if you prefer
unknown and private.


Yes, that is still my preference.

Thanks
Max



Xuelei


--Max

On 4/9/2014 7:57, Xuelei Fan wrote:

Looks fine to me.

I was wondering, whether it is a little bit more instinctive to return a
string with the type number for unknown and private algorithm in
KerberosKey.getAlgorithm().  For example:

  unknown - kid-2014
  private - kid-(2014)

Thanks,
Xuelei

On 4/8/2014 10:37 AM, Weijun Wang wrote:

Hi All

Please review the code changes at

 http://cr.openjdk.java.net/~weijun/8035986/webrev.00/

It's about using IANA names in KerberosKey instead of old non-standard
names.

Thanks
Max






Re: RFR 8035986: KerberosKey algorithm names are not specified

2014-04-08 Thread Xuelei Fan
 Yes, that is still my preference.
OK.

Thanks,
Xuelei

On 4/9/2014 9:50 AM, Weijun Wang wrote:
 
 
 On 4/9/2014 9:15, Xuelei Fan wrote:
 On 4/9/2014 8:53 AM, Weijun Wang wrote:
 There is already getKeyType() and toString().
 ;-) They should not lower the standards to design another good method.
 
 I just meant different methods serve for different purposes.
 

 Also I don't think
 kid-2014 is useful. If people really want to inspect the result, I
 expect they would fall into the default or else block anyway.

 There is a constructor to put unknown or private key type:
  KerberosKey(KerberosPrincipal principal,
  byte[] keyBytes,
  int keyType, int versionNum)

 Which will accept any kind of integer key type.
 
 Yes, this method does not need to understand the keyType to generate a
 key. However, the one we are talking about now must understand the
 algorithm name and call its string2key() method to generate the key from
 a passphrase. So even if you provide kid-2014, it still has to throw
 an IllegalArgumentException.
 

 I think it might be help to get the algorithm in string even if key type
 is not supported (getKeyType() is not as convenient as getAlgorithm() to
 get the string algorithm, toString() covers too much information if one
 only needs to know the algorithm).

 KerberosKey kk = new KerberosKey(..., 123, 0);
 String alg = kk.getAlgorithm();   // unknown returns

 KerberosKey kk = new KerberosKey(..., 124, 0);
 String alg = kk.getAlgorithm();   // unknown returns

 KerberosKey kk = new KerberosKey(..., -123, 0);
 String alg = kk.getAlgorithm();   // private returns

 KerberosKey kk = new KerberosKey(..., -124, 0);
 String alg = kk.getAlgorithm();   // private returns
 
 I would expect actual developers calling getKeyType() more often because
 it's easy to deal with in Kerberos. In this sense, getAlgorithm() only
 exists to override the method in Key.
 

 At least for meaningful debug log or exception message, unknown and
 private is not as instinctive as xxx-123 and xxx-124.

 Anyway, not a big concern of mine.  Please go ahead if you prefer
 unknown and private.
 
 Yes, that is still my preference.
 
 Thanks
 Max
 

 Xuelei

 --Max

 On 4/9/2014 7:57, Xuelei Fan wrote:
 Looks fine to me.

 I was wondering, whether it is a little bit more instinctive to
 return a
 string with the type number for unknown and private algorithm in
 KerberosKey.getAlgorithm().  For example:

   unknown - kid-2014
   private - kid-(2014)

 Thanks,
 Xuelei

 On 4/8/2014 10:37 AM, Weijun Wang wrote:
 Hi All

 Please review the code changes at

  http://cr.openjdk.java.net/~weijun/8035986/webrev.00/

 It's about using IANA names in KerberosKey instead of old non-standard
 names.

 Thanks
 Max





RFR 8035986: KerberosKey algorithm names are not specified

2014-04-07 Thread Weijun Wang

Hi All

Please review the code changes at

   http://cr.openjdk.java.net/~weijun/8035986/webrev.00/

It's about using IANA names in KerberosKey instead of old non-standard 
names.


Thanks
Max