Avoid PrincipalName with realm == null

[Valerie (Yu-Ching) Peng]

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