SimplePrincipal - equals() Implementation

[dlofthouse]

I am interested in some thoughts on how valid our implementation of equals is on our SimplePrincipal.

The javadoc for Principal described the requirements of the equals method as: -

Compares this principal to the specified object. Returns true if the object passed in matches the principal represented by the implementation of this interface.

To me I think our equals method goes too far. I should be able to implement my own Principal which contains additional information that I want to be included in an equality check but if the equals method of SimplePrincipal is called it is going to claim they are equal even though they are not.

So we could have a situation where SimplePrincipal.equals(OtherPrincipal) returns true when OtherPrincipal.equals(SimplePrinicpal) returns false.

 public boolean equals(Object another)
 {
 if (!(another instanceof Principal))
 return false;
 String anotherName = ((Principal) another).getName();
 boolean equals = false;
 if (name == null)
 equals = anotherName == null;
 else
 equals = name.equals(anotherName);
 return equals;
 }

Looking at other Principal implementation Sun seem to have the same interpretation as me as well, here is their description of their equals method for their KerberosPrincipal implementation: -

Compares the specified Object with this Principal for equality. Returns true if the given object is also a KerberosPrincipal and the two KerberosPrincipal instances are equivalent. More formally two KerberosPrincipal instances are equal if the values returned by getName() are equal and the values returned by getNameType() are equal.

[starksm64]

The problem seems to be the OtherPrincipal implementation as all SimplePrincipal is doing is checking for a Principal with equivalent names. This is less restrictive than the KerberosPrincipal implementation. You can't be less restrictive than a Principal with equal names.

[dlofthouse]

But our SimplePrincipal is going to claim it is the same as the KerberosPrincipal if they both happen to have the same name - this is opposite to what the equals method of the KerberosPrincipal would return if called with the SimplePrincipal.

[starksm64]

Which is expected. The KerberosPrincipal is more restrictive and should not view the jboss implementation as equivalent. I don't see what your asking for as you started by saying we go too far. I'm saying we go the minimal distance for a meaningful Principal equals implementation. The only other Principal implementations for which mutual equality exists are those for which the name is sufficient.

[dlofthouse]

But it fails the following two requirements for an equals implementation: -

# It is symmetric: for any non-null reference values x and y, x.equals(y) should return true if and only if y.equals(x) returns true.
# It is transitive: for any non-null reference values x, y, and z, if x.equals(y) returns true and y.equals(z) returns true, then x.equals(z) should return true.

The following requirement from hashCode will also not be met: -

If two objects are equal according to the equals(Object) method, then calling the hashCode method on each of the two objects must produce the same integer result.

Basically you can not put different principal implementation in a HashMap and expect consistent behaviour.

[starksm64]

But it fails the following two requirements for an equals implementation: -

# It is symmetric: for any non-null reference values x and y, x.equals(y) should return true if and only if y.equals(x) returns true.
# It is transitive: for any non-null reference values x, y, and z, if x.equals(y) returns true and y.equals(z) returns true, then x.equals(z) should return true.

The following requirement from hashCode will also not be met: -

If two objects are equal according to the equals(Object) method, then calling the hashCode method on each of the two objects must produce the same integer result.

Basically you can not put different principal implementation in a HashMap and expect consistent behaviour.

Its not the SimplePrincipal.equals/hashCode that is breaking these constraints. Symmetry and transitivity exist for any Principal implementation relying on the Principal.getName() alone. Any other implementation introducing additional information, such as the KerberosPrincipal, is separating itself into a new category. There is nothing our implementation can do to change this.

You would have to create a Map with an ordered collection like TreeMap(Comparator<? super Principal> c) with a Comparator implementation that uses the least common denominator Principal.getName.

What is the context in which this is a problem?

[anil.saldhana]

From a pure Java perspective, Darran you are correct that we need to determine equality on the same type of objects.

But as Scott said, the name of a Principal is what differentiates from another.

I want to propose that we incorporate check on the type of principal being compared in SimplePrincipal (as Darran said). This is important.

[starksm64]

Ok, in talking to Anil the issue is what Darran wants a more restrictive implementation of SimplePrincipal.equals to only consider equality with other SimplePrincipals. That is valid if you are going to mix Principal implementations, but this should not be happening for a given security domain. My question is why is it? Is it because a single cache is being used by multiple security domains?

[mmoyses]

I created a JIRA[1] to modify the equals method.
We now have a customer having issues with the authentication cache and custom Principal because of this.

[1] https://jira.jboss.org/jira/browse/JBAS-6706