8246816: XMLGregorianCalendar.hashCode() produces far too many identical hashes

Reviewed-by: naoto, rriggs

Author: JoeWang-Java

src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/XMLGregorianCalendarImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2004, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2004, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -190,7 +190,7 @@
  * @author Sunitha Reddy
  * @see javax.xml.datatype.Duration
  * @since 1.5
- * @LastModified: June 2018
+ * @LastModified: Aug 2020
  */
 
 public class XMLGregorianCalendarImpl
@@ -1682,49 +1682,6 @@ private static int compareField(BigDecimal Pfield, BigDecimal Qfield) {
         return Pfield.compareTo(Qfield);
     }
 
-    /**
-     * <p>Indicates whether parameter <code>obj</code> is "equal to" this one.</p>
-     *
-     * @param obj to compare.
-     *
-     * @return <code>true</code> when <code>compare(this,(XMLGregorianCalendar)obj) == EQUAL.</code>.
-     */
-    public boolean equals(Object obj) {
-
-        if (obj == null || !(obj instanceof XMLGregorianCalendar)) {
-            return false;
-        }
-        if (obj == this) {
-            return true;
-        }
-        return compare((XMLGregorianCalendar) obj) == DatatypeConstants.EQUAL;
-    }
-
-    /**
-     * <p>Returns a hash code consistent with the definition of the equals method.</p>
-     *
-     * @return hash code of this object.
-     */
-    public int hashCode() {
-
-        // Following two dates compare to EQUALS since in different timezones.
-        // 2000-01-15T12:00:00-05:00 == 2000-01-15T13:00:00-04:00
-        //
-        // Must ensure both instances generate same hashcode by normalizing
-        // this to UTC timezone.
-        int timezone = getTimezone();
-        if (timezone == DatatypeConstants.FIELD_UNDEFINED) {
-            timezone = 0;
-        }
-        XMLGregorianCalendar gc = this;
-        if (timezone != 0) {
-            gc = this.normalizeToTimezone(getTimezone());
-        }
-        return gc.getYear() + gc.getMonth() + gc.getDay() +
-                gc.getHour() + gc.getMinute() + gc.getSecond();
-    }
-
-
     /**
      * <p>Constructs a new XMLGregorianCalendar object by
      * parsing its lexical string representation as defined in

src/java.xml/share/classes/javax/xml/datatype/XMLGregorianCalendar.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -28,6 +28,7 @@
 import javax.xml.namespace.QName;
 import java.math.BigDecimal;
 import java.math.BigInteger;
+import java.util.Arrays;
 import java.util.TimeZone;
 import java.util.GregorianCalendar;
 
@@ -688,10 +689,12 @@ public int getMillisecond() {
      */
     @Override
     public boolean equals(Object obj) {
-
         if (obj == null || !(obj instanceof XMLGregorianCalendar)) {
             return false;
         }
+        if (obj == this) {
+            return true;
+        }
         return compare((XMLGregorianCalendar) obj) == DatatypeConstants.EQUAL;
     }
 
@@ -716,12 +719,10 @@ public int hashCode() {
         if (timezone != 0) {
             gc = this.normalize();
         }
-        return gc.getYear()
-                + gc.getMonth()
-                + gc.getDay()
-                + gc.getHour()
-                + gc.getMinute()
-                + gc.getSecond();
+
+        int[] elements = {gc.getYear(), gc.getMonth(), gc.getDay(), gc.getHour(),
+                          gc.getMinute(), gc.getSecond(), gc.getMillisecond()};
+        return Arrays.hashCode(elements);
     }
 
     /**

test/jaxp/javax/xml/jaxp/unittest/datatype/HashCodeTest.java

@@ -0,0 +1,80 @@
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package datatype;
+
+import javax.xml.datatype.DatatypeFactory;
+import javax.xml.datatype.XMLGregorianCalendar;
+import org.testng.annotations.Test;
+import org.testng.Assert;
+import org.testng.annotations.DataProvider;
+
+/*
+ * @test
+ * @bug 8246816
+ * @run testng datatype.HashCodeTest
+ * @summary Test hashCode generation.
+ */
+public class HashCodeTest {
+    /*
+       DataProvider: for testHashCode
+       Data: datetime1, datetime2, flag indicating if their hashCodes are equal
+     */
+    @DataProvider(name = "testHashCode")
+    public Object[][] getData() {
+
+        return new Object[][]{
+            // the reported case: identical hash codes before the patch
+            {"2020-04-24T12:53:00+02:00", "2020-06-04T06:58:17.727Z", false},
+            // a case mentioned in the dev note of hashCode() implementation
+            {"2000-01-15T12:00:00-05:00", "2000-01-15T13:00:00-04:00", true},
+            /**
+             * Comparing with a datetime that needs to be normalized.
+             * Before the patch, XMLGregorianCalendarImpl called the normalizeToTimezone
+             * method that will set UNDEFINED fractional second to zero.
+             */
+            {"2000-01-01T03:19:04Z", "1999-12-31T23:49:04-03:30", true},
+            // another case mentioned in the javadoc of XMLGregorianCalendar::normalize()
+            {"2000-03-04T23:00:00+03:00", "2000-03-04T20:00:00Z", true},
+        };
+    }
+
+    @Test(dataProvider = "testHashCode")
+    public final void testHashCode(String dt1, String dt2, boolean equal) throws Exception {
+        DatatypeFactory dataTypeFactory = DatatypeFactory.newInstance();
+        XMLGregorianCalendar cal1 = dataTypeFactory.newXMLGregorianCalendar(dt1);
+        XMLGregorianCalendar cal2 = dataTypeFactory.newXMLGregorianCalendar(dt2);
+
+        // identical hash codes before the patch
+        int hashCode1 = cal1.hashCode();
+        int hashCode2 = cal2.hashCode();
+
+        if (equal) {
+            Assert.assertTrue(cal1.equals(cal2));
+            Assert.assertEquals(hashCode1, hashCode2);
+        } else {
+            Assert.assertFalse(cal1.equals(cal2));
+            Assert.assertNotEquals(hashCode1, hashCode2);
+        }
+    }
+}