From b5749b4f54b30c0c2050e456c12cfcf516434f13 Mon Sep 17 00:00:00 2001 From: Benedikt Ritter Date: Fri, 1 May 2015 22:21:11 +0200 Subject: [PATCH] LANG-456: HashCodeBuilder throws StackOverflowError in bidirectional navigable association. Thanks to Bob Fields who provides the inital patch. Thanks to Woosan Ko who wrote the final patch. Thanks to Bruno P. Kinoshita who updated the final patch so that it was easy to apply. --- src/changes/changes.xml | 1 + .../lang3/builder/HashCodeBuilder.java | 30 +++++++++- .../lang3/builder/HashCodeBuilderTest.java | 59 ++++++++++++++++++- 3 files changed, 88 insertions(+), 2 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index c53f3f6de..1ecfd8fc0 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -22,6 +22,7 @@ + HashCodeBuilder throws StackOverflowError in bidirectional navigable association DateFormatUtilsTest.testSMTP depends on the default Locale Unit test FastDatePrinterTimeZonesTest needs a timezone set CLONE - DateFormatUtils.format does not correctly change Calendar TimeZone in certain situations diff --git a/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java b/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java index c2d865a55..537b9f579 100644 --- a/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java @@ -856,7 +856,35 @@ public HashCodeBuilder append(final Object object) { append((Object[]) object); } } else { - iTotal = iTotal * iConstant + object.hashCode(); + if (object instanceof Long) { + append(((Long) object).longValue()); + } else if (object instanceof Integer) { + append(((Integer) object).intValue()); + } else if (object instanceof Short) { + append(((Short) object).shortValue()); + } else if (object instanceof Character) { + append(((Character) object).charValue()); + } else if (object instanceof Byte) { + append(((Byte) object).byteValue()); + } else if (object instanceof Double) { + append(((Double) object).doubleValue()); + } else if (object instanceof Float) { + append(((Float) object).floatValue()); + } else if (object instanceof Boolean) { + append(((Boolean) object).booleanValue()); + } else if (object instanceof String) { + iTotal = iTotal * iConstant + object.hashCode(); + } else { + if (isRegistered(object)) { + return this; + } + try { + register(object); + iTotal = iTotal * iConstant + object.hashCode(); + } finally { + unregister(object); + } + } } } return this; diff --git a/src/test/java/org/apache/commons/lang3/builder/HashCodeBuilderTest.java b/src/test/java/org/apache/commons/lang3/builder/HashCodeBuilderTest.java index 0b51cd5d0..12aba9ee3 100644 --- a/src/test/java/org/apache/commons/lang3/builder/HashCodeBuilderTest.java +++ b/src/test/java/org/apache/commons/lang3/builder/HashCodeBuilderTest.java @@ -19,6 +19,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; + import org.junit.Test; /** @@ -32,6 +33,8 @@ public class HashCodeBuilderTest { * A reflection test fixture. */ static class ReflectionTestCycleA { + int index = 10; + String name = "ReflectionTestCycleA"; ReflectionTestCycleB b; @Override @@ -44,6 +47,8 @@ public int hashCode() { * A reflection test fixture. */ static class ReflectionTestCycleB { + int index = 11; + String name = "ReflectionTestCycleB"; ReflectionTestCycleA a; @Override @@ -52,6 +57,42 @@ public int hashCode() { } } + /** + * A nonreflection test fixture. + */ + static class NonreflectionTestCycleA { + int index = 20; + String name = "NonreflectionTestCycleA"; + NonreflectionTestCycleB b; + + @Override + public int hashCode() { + HashCodeBuilder builder = new HashCodeBuilder(); + builder.append(index); + builder.append(name); + builder.append(b); + return builder.toHashCode(); + } + } + + /** + * A nonreflection test fixture. + */ + static class NonreflectionTestCycleB { + int index = 21; + String name = "NonreflectionTestCycleB"; + NonreflectionTestCycleA a; + + @Override + public int hashCode() { + HashCodeBuilder builder = new HashCodeBuilder(); + builder.append(index); + builder.append(name); + builder.append(a); + return builder.toHashCode(); + } + } + // ----------------------------------------------------------------------- @Test(expected=IllegalArgumentException.class) @@ -523,7 +564,7 @@ public TestObjectWithMultipleFields(final int one, final int two, final int thre } /** - * Test Objects pointing to each other. + * Test Objects pointing to each other when {@link HashCodeBuilder#reflectionHashCode(Object, String...)} used. */ @Test public void testReflectionObjectCycle() { @@ -555,6 +596,22 @@ public void testReflectionObjectCycle() { assertNull(HashCodeBuilder.getRegistry()); } + /** + * Test Objects pointing to each other when append() methods are used on HashCodeBuilder instance. + */ + @Test + public void testNonreflectionObjectCycle() { + final NonreflectionTestCycleA a = new NonreflectionTestCycleA(); + final NonreflectionTestCycleB b = new NonreflectionTestCycleB(); + a.b = b; + b.a = a; + + a.hashCode(); + assertNull(HashCodeBuilder.getRegistry()); + b.hashCode(); + assertNull(HashCodeBuilder.getRegistry()); + } + /** * Ensures LANG-520 remains true */