From 1a60c21395fe7648188d1c91f62ac7baefa12742 Mon Sep 17 00:00:00 2001 From: Henri Yandell Date: Sat, 20 Mar 2010 20:16:57 +0000 Subject: Applying the copy of the HashCodeBuilder code to stop cyclic references over to EqualsBuilder per LANG-606 and Oliver Sauder's patch git-svn-id: https://svn.apache.org/repos/asf/commons/proper/lang/trunk@925671 13f79535-47bb-0310-9956-ffa450edef68 --- .../commons/lang3/builder/EqualsBuilder.java | 168 +++++++++++++++++++-- .../commons/lang3/builder/EqualsBuilderTest.java | 50 ++++++ 2 files changed, 204 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java b/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java index dba3b600d..c725cfffd 100644 --- a/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java @@ -20,8 +20,11 @@ import java.lang.reflect.AccessibleObject; import java.lang.reflect.Field; import java.lang.reflect.Modifier; import java.util.Collection; +import java.util.HashSet; +import java.util.Set; import org.apache.commons.lang3.ArrayUtils; +import org.apache.commons.lang3.Pair; /** *

Assists in implementing {@link Object#equals(Object)} methods.

@@ -79,11 +82,138 @@ import org.apache.commons.lang3.ArrayUtils; * @author Gary Gregory * @author Pete Gieser * @author Arun Mammen Thomas + * @author Oliver Sauder * @since 1.0 * @version $Id$ */ public class EqualsBuilder { + /** + *

+ * A registry of objects used by reflection methods to detect cyclical object references and avoid infinite loops. + *

+ * + * @since 3.0 + */ + private static final ThreadLocal>> REGISTRY = new ThreadLocal>>(); + + /* + * N.B. we cannot store the actual objects in a HashSet, as that would use the very hashCode() + * we are in the process of calculating. + * + * So we generate a one-to-one mapping from the original object to a new object. + * + * Now HashSet uses equals() to determine if two elements with the same hashcode really + * are equal, so we also need to ensure that the replacement objects are only equal + * if the original objects are identical. + * + * The original implementation (2.4 and before) used the System.indentityHashCode() + * method - however this is not guaranteed to generate unique ids (e.g. LANG-459) + * + * We now use the IDKey helper class (adapted from org.apache.axis.utils.IDKey) + * to disambiguate the duplicate ids. + */ + + /** + *

+ * Returns the registry of object pairs being traversed by the reflection + * methods in the current thread. + *

+ * + * @return Set the registry of objects being traversed + * @since 3.0 + */ + static Set> getRegistry() { + return REGISTRY.get(); + } + + /** + *

+ * Converters value pair into a register pair. + *

+ * + * @param lhs this object + * @param rhs the other object + * + * @return + */ + static Pair getRegisterPair(Object lhs, Object rhs) { + IDKey left = new IDKey(lhs); + IDKey right = new IDKey(rhs); + return new Pair(left, right); + } + + /** + *

+ * Returns true if the registry contains the given object pair. + * Used by the reflection methods to avoid infinite loops. + * Objects might be swapped therefore a check is needed if the object pair + * is registered in given or swapped order. + *

+ * + * @param lhs this object to lookup in registry + * @param rhs the other object to lookup on registry + * @return boolean true if the registry contains the given object. + * @since 3.0 + */ + static boolean isRegistered(Object lhs, Object rhs) { + Set> registry = getRegistry(); + Pair pair = getRegisterPair(lhs, rhs); + Pair swappedPair = new Pair(pair.right, + pair.left); + + return registry != null + && (registry.contains(pair) || registry.contains(swappedPair)); + } + + /** + *

+ * Registers the given object pair. + * Used by the reflection methods to avoid infinite loops. + *

+ * + * @param lhs this object to register + * @param rhs the other object to register + */ + static void register(Object lhs, Object rhs) { + synchronized (HashCodeBuilder.class) { + if (getRegistry() == null) { + REGISTRY.set(new HashSet>()); + } + } + + Set> registry = getRegistry(); + Pair pair = getRegisterPair(lhs, rhs); + registry.add(pair); + } + + /** + *

+ * Unregisters the given object pair. + *

+ * + *

+ * Used by the reflection methods to avoid infinite loops. + * + * @param lhs this object to unregister + * @param rhs the other object to unregister + * @since 3.0 + */ + static void unregister(Object lhs, Object rhs) { + Set> registry = getRegistry(); + if (registry != null) { + Pair pair = getRegisterPair(lhs, rhs); + registry.remove(pair); + synchronized (HashCodeBuilder.class) { + //read again + registry = getRegistry(); + if (registry != null && registry.isEmpty()) { + REGISTRY.remove(); + } + } + } + } + /** * If the fields tested are equals. * The default value is true. @@ -316,22 +446,32 @@ public class EqualsBuilder { EqualsBuilder builder, boolean useTransients, String[] excludeFields) { - Field[] fields = clazz.getDeclaredFields(); - AccessibleObject.setAccessible(fields, true); - for (int i = 0; i < fields.length && builder.isEquals; i++) { - Field f = fields[i]; - if (!ArrayUtils.contains(excludeFields, f.getName()) - && (f.getName().indexOf('$') == -1) - && (useTransients || !Modifier.isTransient(f.getModifiers())) - && (!Modifier.isStatic(f.getModifiers()))) { - try { - builder.append(f.get(lhs), f.get(rhs)); - } catch (IllegalAccessException e) { - //this can't happen. Would get a Security exception instead - //throw a runtime exception in case the impossible happens. - throw new InternalError("Unexpected IllegalAccessException"); + + if (isRegistered(lhs, rhs)) { + return; + } + + try { + register(lhs, rhs); + Field[] fields = clazz.getDeclaredFields(); + AccessibleObject.setAccessible(fields, true); + for (int i = 0; i < fields.length && builder.isEquals; i++) { + Field f = fields[i]; + if (!ArrayUtils.contains(excludeFields, f.getName()) + && (f.getName().indexOf('$') == -1) + && (useTransients || !Modifier.isTransient(f.getModifiers())) + && (!Modifier.isStatic(f.getModifiers()))) { + try { + builder.append(f.get(lhs), f.get(rhs)); + } catch (IllegalAccessException e) { + //this can't happen. Would get a Security exception instead + //throw a runtime exception in case the impossible happens. + throw new InternalError("Unexpected IllegalAccessException"); + } } } + } finally { + unregister(lhs, rhs); } } diff --git a/src/test/java/org/apache/commons/lang3/builder/EqualsBuilderTest.java b/src/test/java/org/apache/commons/lang3/builder/EqualsBuilderTest.java index c315bdd46..382997080 100644 --- a/src/test/java/org/apache/commons/lang3/builder/EqualsBuilderTest.java +++ b/src/test/java/org/apache/commons/lang3/builder/EqualsBuilderTest.java @@ -27,6 +27,7 @@ import junit.framework.TestCase; * @author Steve Downey * @author Gary Gregory * @author Maarten Coene + * @author Oliver Sauder * @version $Id$ */ public class EqualsBuilderTest extends TestCase { @@ -991,4 +992,53 @@ public class EqualsBuilderTest extends TestCase { this.three = new TestObject(three); } } + + /** + * Test cyclical object references which cause a StackOverflowException if + * not handled properly. s. LANG-606 + */ + public void testCyclicalObjectReferences() { + TestObjectReference refX1 = new TestObjectReference(1); + TestObjectReference x1 = new TestObjectReference(1); + x1.setObjectReference(refX1); + refX1.setObjectReference(x1); + + TestObjectReference refX2 = new TestObjectReference(1); + TestObjectReference x2 = new TestObjectReference(1); + x2.setObjectReference(refX2); + refX2.setObjectReference(x2); + + TestObjectReference refX3 = new TestObjectReference(2); + TestObjectReference x3 = new TestObjectReference(2); + x3.setObjectReference(refX3); + refX3.setObjectReference(x3); + + assertTrue(x1.equals(x2)); + assertNull(EqualsBuilder.getRegistry()); + assertFalse(x1.equals(x3)); + assertNull(EqualsBuilder.getRegistry()); + assertFalse(x2.equals(x3)); + assertNull(EqualsBuilder.getRegistry()); + } + + static class TestObjectReference { + @SuppressWarnings("unused") + private TestObjectReference reference; + @SuppressWarnings("unused") + private TestObject one; + + public TestObjectReference(int one) { + this.one = new TestObject(one); + } + + public void setObjectReference(TestObjectReference reference) { + this.reference = reference; + } + + @Override + public boolean equals(Object obj) { + return EqualsBuilder.reflectionEquals(this, obj); + } + } } + -- cgit v1.2.3