Max,
Yes, I think the current model that you described sounds error prone. I
don't know the history of the current design.
But I do also prefer the described changes that you have.
I'd expect the refactoring would make the code clearer and more robust.
Valerie
On 06/06/12 17:55, Weijun Wang wrote:
Hi Valerie
The krb5 PrincipalName class has a realm field and the class says
If null, means the default realm
Ideally this means if the realm of a name is null then this field can
be null. Otherwise, it must be filled when created.
In fact, inside our codes, the field is often filled (using
setRealm()) after it's created. This leads to several strange coding
styles that make the codes confusing and error-prone.
1. a lot of setRealm() calls that's far from the creation of the
principal name only when the field needs to be used
2. a lot of if (realm == null) checks
3. a lot of "unresolved" names that never has a realm but is
definitely not in the default realm (just because the realm field is
not used inside JDK)
I am planning to fix this to make the PrincipalName immutable and
always with a non-null non-empty realm. I also plan to make Realm
immutable and remove the ServiceName class (it's quite useless).
A brief look into the code and protocol suggests this is quite
feasible. In every krb5 message and serialized data (I mean ccache and
keytab) defined, there is always a realm beside name. This is also
true for most Java methods. And I don't think a name with an
"unresolved" realm should exists at all. If we have to deal with
something like this, I'd rather invent a new class for it.
The code change will be mostly refactoring, removing a lot of realm
arguments/fields and merging it into name. One behavior change is that
there will be no name with "unresolved" realm anymore, but I think
this should never be true in a real production environment. In fact,
the public API KerberosPrincipal has
* @throws IllegalArgumentException if name is improperly
* formatted, if name is null, or if name does not contain
* the realm to use and the default realm is not specified
* in either a Kerberos configuration file or via the
* java.security.krb5.realm system property.
What's your suggestion? I've been haunted by this several times,
mostly because a setRealm() is not called.
Thanks
Max
