You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by br...@apache.org on 2015/05/01 22:27:21 UTC

[lang] 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

Repository: commons-lang
Updated Branches:
  refs/heads/master 4796a0a79 -> b5749b4f5


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.


Project: http://git-wip-us.apache.org/repos/asf/commons-lang/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-lang/commit/b5749b4f
Tree: http://git-wip-us.apache.org/repos/asf/commons-lang/tree/b5749b4f
Diff: http://git-wip-us.apache.org/repos/asf/commons-lang/diff/b5749b4f

Branch: refs/heads/master
Commit: b5749b4f54b30c0c2050e456c12cfcf516434f13
Parents: 4796a0a
Author: Benedikt Ritter <br...@apache.org>
Authored: Fri May 1 22:21:11 2015 +0200
Committer: Benedikt Ritter <br...@apache.org>
Committed: Fri May 1 22:26:42 2015 +0200

----------------------------------------------------------------------
 src/changes/changes.xml                         |  1 +
 .../commons/lang3/builder/HashCodeBuilder.java  | 30 +++++++++-
 .../lang3/builder/HashCodeBuilderTest.java      | 59 +++++++++++++++++++-
 3 files changed, 88 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/commons-lang/blob/b5749b4f/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index c53f3f6..1ecfd8f 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -22,6 +22,7 @@
   <body>
 
   <release version="3.5" date="tba" description="tba">
+    <action issue="LANG-456" type="fix" dev="britter" due-to="Bob Fields, Woosan Ko, Bruno P. Kinoshita">HashCodeBuilder throws StackOverflowError in bidirectional navigable association</action>
     <action issue="LANG-1126" type="fix" dev="britter">DateFormatUtilsTest.testSMTP depends on the default Locale</action>
     <action issue="LANG-1123" type="fix" dev="chas" due-to="Christian P. Momon">Unit test FastDatePrinterTimeZonesTest needs a timezone set</action>
     <action issue="LANG-916" type="fix" dev="chas" due-to="Christian P. Momon">CLONE - DateFormatUtils.format does not correctly change Calendar TimeZone in certain situations</action>

http://git-wip-us.apache.org/repos/asf/commons-lang/blob/b5749b4f/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java
----------------------------------------------------------------------
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 c2d865a..537b9f5 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 class HashCodeBuilder implements Builder<Integer> {
                     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;

http://git-wip-us.apache.org/repos/asf/commons-lang/blob/b5749b4f/src/test/java/org/apache/commons/lang3/builder/HashCodeBuilderTest.java
----------------------------------------------------------------------
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 0b51cd5..12aba9e 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 @@ package org.apache.commons.lang3.builder;
 
 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 class HashCodeBuilderTest {
      * A reflection test fixture.
      */
     static class ReflectionTestCycleB {
+        int index = 11;
+        String name = "ReflectionTestCycleB";
         ReflectionTestCycleA a;
 
         @Override
@@ -52,6 +57,42 @@ public class HashCodeBuilderTest {
         }
     }
 
+    /**
+     * 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 class HashCodeBuilderTest {
     }
 
     /**
-     * Test Objects pointing to each other.
+     * Test Objects pointing to each other when {@link HashCodeBuilder#reflectionHashCode(Object, String...)} used.
      */
     @Test
     public void testReflectionObjectCycle() {
@@ -556,6 +597,22 @@ public class HashCodeBuilderTest {
     }
 
     /**
+     * Test Objects pointing to each other when <code>append()</code> methods are used on <code>HashCodeBuilder</code> 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
      */
     @Test