Keep HashCodeBuilder.append(Object) from poisoning reflection hashCode

src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java

@@ -169,6 +169,13 @@ public Builder setMultiplierOddNumber(final int multiplierOddNumber) {
      */
     private static final ThreadLocal<Set<IDKey>> REGISTRY = ThreadLocal.withInitial(HashSet::new);
 
+    /**
+     * A registry of objects being appended by {@link #append(Object)}, kept separate from {@link #REGISTRY} so that
+     * guarding {@code append} against its own re-entrant cycles does not trip the reflection cycle guard checked by
+     * {@link #reflectionAppend(Object, Class, HashCodeBuilder, boolean, String[], boolean)}.
+     */
+    private static final ThreadLocal<Set<IDKey>> APPEND_REGISTRY = ThreadLocal.withInitial(HashSet::new);
+
     /**
      * Constructs a new Builder.
      *
@@ -541,6 +548,38 @@ private static void unregister(final Object value) {
         }
     }
 
+    /**
+     * Tests whether the append registry contains the given object. Used by {@link #append(Object)} to break its own re-entrant cycles.
+     *
+     * @param value The object to look up in the append registry.
+     * @return {@code true} if the append registry contains the given object.
+     */
+    private static boolean isAppendRegistered(final Object value) {
+        return APPEND_REGISTRY.get().contains(new IDKey(value));
+    }
+
+    /**
+     * Registers the given object in the append registry.
+     *
+     * @param value The object to register.
+     */
+    private static void appendRegister(final Object value) {
+        APPEND_REGISTRY.get().add(new IDKey(value));
+    }
+
+    /**
+     * Unregisters the given object from the append registry.
+     *
+     * @param value The object to unregister.
+     */
+    private static void appendUnregister(final Object value) {
+        final Set<IDKey> registry = APPEND_REGISTRY.get();
+        registry.remove(new IDKey(value));
+        if (registry.isEmpty()) {
+            APPEND_REGISTRY.remove();
+        }
+    }
+
     /**
      * Constant to use in building the hashCode.
      */
@@ -820,22 +859,22 @@ public HashCodeBuilder append(final long[] array) {
     public HashCodeBuilder append(final Object object) {
         if (object == null) {
             total = total * constant;
-        } else if (isRegistered(object)) {
+        } else if (isRegistered(object) || isAppendRegistered(object)) {
             // Cycle detected: skip to avoid infinite recursion (mirrors reflectionAppend).
             total = total * constant;
         } else if (ObjectUtils.isArray(object)) {
             try {
-                register(object);
+                appendRegister(object);
                 appendArray(object);
             } finally {
-                unregister(object);
+                appendUnregister(object);
             }
         } else {
             try {
-                register(object);
+                appendRegister(object);
                 total = total * constant + object.hashCode();
             } finally {
-                unregister(object);
+                appendUnregister(object);
             }
         }
         return this;

src/test/java/org/apache/commons/lang3/builder/HashCodeBuilderTest.java

@@ -66,6 +66,48 @@ public int hashCode() {
         }
     }
 
+    /**
+     * A reflection test fixture whose {@code hashCode()} is reflection based.
+     */
+    static final class ReflectionHashCodeMember {
+        final int value;
+
+        ReflectionHashCodeMember(final int value) {
+            this.value = value;
+        }
+
+        @Override
+        public boolean equals(final Object o) {
+            return EqualsBuilder.reflectionEquals(this, o);
+        }
+
+        @Override
+        public int hashCode() {
+            return HashCodeBuilder.reflectionHashCode(this);
+        }
+    }
+
+    /**
+     * Holds a {@link ReflectionHashCodeMember} and is itself reflection hashed.
+     */
+    static final class ReflectionHashCodeHolder {
+        final ReflectionHashCodeMember member;
+
+        ReflectionHashCodeHolder(final ReflectionHashCodeMember member) {
+            this.member = member;
+        }
+
+        @Override
+        public boolean equals(final Object o) {
+            return EqualsBuilder.reflectionEquals(this, o);
+        }
+
+        @Override
+        public int hashCode() {
+            return HashCodeBuilder.reflectionHashCode(this);
+        }
+    }
+
     static class TestObject {
         private int a;
 
@@ -636,4 +678,23 @@ void testToHashCodeExclude() {
         assertEquals(INITIAL, HashCodeBuilder.reflectionHashCode(two));
     }
 
+    /**
+     * {@code append(Object)} must not collapse an object whose own {@code hashCode()} is reflection based to the bare
+     * initial value. Regression for the cycle guard leaking into the reflection registry.
+     */
+    @Test
+    void testAppendObjectReflectionHashCodeNotPoisoned() {
+        final ReflectionHashCodeMember member = new ReflectionHashCodeMember(42);
+        assertEquals(INITIAL * CONSTANT + member.hashCode(),
+            new HashCodeBuilder(INITIAL, CONSTANT).append((Object) member).toHashCode());
+
+        // The same defect reached through a normal, acyclic graph: a nested object's content must still influence
+        // the enclosing reflection hash.
+        final int h1 = new ReflectionHashCodeHolder(new ReflectionHashCodeMember(1)).hashCode();
+        final int h2 = new ReflectionHashCodeHolder(new ReflectionHashCodeMember(2)).hashCode();
+        assertNotEquals(h1, h2);
+
+        assertTrue(HashCodeBuilder.getRegistry().isEmpty());
+    }
+
 }