Handle plus/minus of months correctly

Errors close to Integer.MIN_VALUE/MAX_VALUE Fixes JodaOrg#418
kluever · Mar 22, 2017 · 67e9e5c · 67e9e5c
1 parent ff0473d
commit 67e9e5c
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 2 deletions.
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
@@ -12,6 +12,10 @@
       <action dev="jodastephen" type="update">
         DateTimeZone data updated to version 2017b.
       </action>
+      <action dev="jodastephen" type="fix">
+        Handle plus/minus of Integer.MIN_VALUE/MAX_VALUE months correctly.
+        Fixes #418.
+      </action>
       <action dev="sebkur" type="fix">
         Clarify time fields are based on the local time-line.
         Fixes #415.

diff --git a/src/main/java/org/joda/time/chrono/BasicMonthOfYearDateTimeField.java b/src/main/java/org/joda/time/chrono/BasicMonthOfYearDateTimeField.java
@@ -112,8 +112,13 @@ public long add(long instant, int months) {
         // Initially, monthToUse is zero-based
         int monthToUse = thisMonth - 1 + months;
         if (thisMonth > 0 && monthToUse < 0) {
-            yearToUse++;
-            months -= iMax;
+            if (Math.signum(months + iMax) == Math.signum(months)) {
+                yearToUse--;
+                months += iMax;
+            } else {
+                yearToUse++;
+                months -= iMax;
+            }
             monthToUse = thisMonth - 1 + months;
         }
         if (monthToUse >= 0) {

diff --git a/src/test/java/org/joda/time/TestDateTime_Basics.java b/src/test/java/org/joda/time/TestDateTime_Basics.java
@@ -1087,6 +1087,21 @@ public void testPlusMonths_intMax() {
         assertEquals(new DateTime(178958986, 7, 20, 1, 2, 3, 4, ISO_UTC), test.plusMonths(Integer.MAX_VALUE - 2));
         assertEquals(new DateTime(178958986, 8, 20, 1, 2, 3, 4, ISO_UTC), test.plusMonths(Integer.MAX_VALUE - 1));
         assertEquals(new DateTime(178958986, 9, 20, 1, 2, 3, 4, ISO_UTC), test.plusMonths(Integer.MAX_VALUE));
+
+        assertEquals(new DateTime(178958986, 7, 20, 1, 2, 3, 4, ISO_UTC), test.monthOfYear().addToCopy(Integer.MAX_VALUE - 2));
+        assertEquals(new DateTime(178958986, 8, 20, 1, 2, 3, 4, ISO_UTC), test.monthOfYear().addToCopy(Integer.MAX_VALUE - 1));
+        assertEquals(new DateTime(178958986, 9, 20, 1, 2, 3, 4, ISO_UTC), test.monthOfYear().addToCopy(Integer.MAX_VALUE));
+    }
+
+    public void testPlusMonths_intMin() {
+        DateTime test = new DateTime(2016, 2, 20, 1, 2, 3, 4, ISO_UTC);
+        assertEquals(new DateTime(-178954955, 8, 20, 1, 2, 3, 4, ISO_UTC), test.plusMonths(Integer.MIN_VALUE + 2));
+        assertEquals(new DateTime(-178954955, 7, 20, 1, 2, 3, 4, ISO_UTC), test.plusMonths(Integer.MIN_VALUE + 1));
+        assertEquals(new DateTime(-178954955, 6, 20, 1, 2, 3, 4, ISO_UTC), test.plusMonths(Integer.MIN_VALUE));
+
+        assertEquals(new DateTime(-178954955, 8, 20, 1, 2, 3, 4, ISO_UTC), test.monthOfYear().addToCopy(Integer.MIN_VALUE + 2));
+        assertEquals(new DateTime(-178954955, 7, 20, 1, 2, 3, 4, ISO_UTC), test.monthOfYear().addToCopy(Integer.MIN_VALUE + 1));
+        assertEquals(new DateTime(-178954955, 6, 20, 1, 2, 3, 4, ISO_UTC), test.monthOfYear().addToCopy(Integer.MIN_VALUE));
     }
 
     public void testPlusWeeks_int() {
@@ -1197,6 +1212,20 @@ public void testMinusMonths_int() {
         assertSame(test, result);
     }
 
+    public void testMinusMonths_intMax() {
+        DateTime test = new DateTime(2016, 2, 20, 1, 2, 3, 4, ISO_UTC);
+        assertEquals(new DateTime(-178954955, 9, 20, 1, 2, 3, 4, ISO_UTC), test.minusMonths(Integer.MAX_VALUE - 2));
+        assertEquals(new DateTime(-178954955, 8, 20, 1, 2, 3, 4, ISO_UTC), test.minusMonths(Integer.MAX_VALUE - 1));
+        assertEquals(new DateTime(-178954955, 7, 20, 1, 2, 3, 4, ISO_UTC), test.minusMonths(Integer.MAX_VALUE));
+    }
+
+    public void testMinusMonths_intMin() {
+        DateTime test = new DateTime(2016, 2, 20, 1, 2, 3, 4, ISO_UTC);
+        assertEquals(new DateTime(178958986, 8, 20, 1, 2, 3, 4, ISO_UTC), test.minusMonths(Integer.MIN_VALUE + 2));
+        assertEquals(new DateTime(178958986, 9, 20, 1, 2, 3, 4, ISO_UTC), test.minusMonths(Integer.MIN_VALUE + 1));
+        assertEquals(new DateTime(178958986, 10, 20, 1, 2, 3, 4, ISO_UTC), test.minusMonths(Integer.MIN_VALUE));
+    }
+
     public void testMinusWeeks_int() {
         DateTime test = new DateTime(2002, 5, 3, 1, 2, 3, 4, BUDDHIST_DEFAULT);
         DateTime result = test.minusWeeks(1);