8164366: ZoneOffset.ofHoursMinutesSeconds() does not reject invalid input
authorigerasim
Mon, 22 Aug 2016 22:16:54 +0300
changeset 40527 542d72803808
parent 40519 e17429a7e843
child 40528 c2d4a13ab15a
8164366: ZoneOffset.ofHoursMinutesSeconds() does not reject invalid input Reviewed-by: scolebourne, ntv, coffeys
jdk/src/java.base/share/classes/java/time/ZoneOffset.java
jdk/test/java/time/tck/java/time/TCKZoneOffset.java
--- a/jdk/src/java.base/share/classes/java/time/ZoneOffset.java	Wed Jul 05 22:07:34 2017 +0200
+++ b/jdk/src/java.base/share/classes/java/time/ZoneOffset.java	Mon Aug 22 22:16:54 2016 +0300
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2016, 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
@@ -375,15 +375,15 @@
         } else if ((minutes > 0 && seconds < 0) || (minutes < 0 && seconds > 0)) {
             throw new DateTimeException("Zone offset minutes and seconds must have the same sign");
         }
-        if (Math.abs(minutes) > 59) {
-            throw new DateTimeException("Zone offset minutes not in valid range: abs(value) " +
-                    Math.abs(minutes) + " is not in the range 0 to 59");
+        if (minutes < -59 || minutes > 59) {
+            throw new DateTimeException("Zone offset minutes not in valid range: value " +
+                    minutes + " is not in the range -59 to 59");
         }
-        if (Math.abs(seconds) > 59) {
-            throw new DateTimeException("Zone offset seconds not in valid range: abs(value) " +
-                    Math.abs(seconds) + " is not in the range 0 to 59");
+        if (seconds < -59 || seconds > 59) {
+            throw new DateTimeException("Zone offset seconds not in valid range: value " +
+                    seconds + " is not in the range -59 to 59");
         }
-        if (Math.abs(hours) == 18 && (Math.abs(minutes) > 0 || Math.abs(seconds) > 0)) {
+        if (Math.abs(hours) == 18 && (minutes | seconds) != 0) {
             throw new DateTimeException("Zone offset not in valid range: -18:00 to +18:00");
         }
     }
@@ -411,7 +411,7 @@
      * @throws DateTimeException if the offset is not in the required range
      */
     public static ZoneOffset ofTotalSeconds(int totalSeconds) {
-        if (Math.abs(totalSeconds) > MAX_SECONDS) {
+        if (totalSeconds < -MAX_SECONDS || totalSeconds > MAX_SECONDS) {
             throw new DateTimeException("Zone offset not in valid range: -18:00 to +18:00");
         }
         if (totalSeconds % (15 * SECONDS_PER_MINUTE) == 0) {
@@ -696,11 +696,12 @@
      * The comparison is "consistent with equals", as defined by {@link Comparable}.
      *
      * @param other  the other date to compare to, not null
-     * @return the comparator value, negative if less, postive if greater
+     * @return the comparator value, negative if less, positive if greater
      * @throws NullPointerException if {@code other} is null
      */
     @Override
     public int compareTo(ZoneOffset other) {
+        // abs(totalSeconds) <= MAX_SECONDS, so no overflow can happen here
         return other.totalSeconds - totalSeconds;
     }
 
--- a/jdk/test/java/time/tck/java/time/TCKZoneOffset.java	Wed Jul 05 22:07:34 2017 +0200
+++ b/jdk/test/java/time/tck/java/time/TCKZoneOffset.java	Mon Aug 22 22:16:54 2016 +0300
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2016, 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
@@ -419,6 +419,21 @@
         ZoneOffset.ofHoursMinutesSeconds(-19, 0, 0);
     }
 
+    @Test(expectedExceptions=DateTimeException.class)
+    public void test_factory_int_hours_minutes_seconds_minutesMinValue() {
+        ZoneOffset.ofHoursMinutesSeconds(0, Integer.MIN_VALUE, -1);
+    }
+
+    @Test(expectedExceptions=DateTimeException.class)
+    public void test_factory_int_hours_minutes_seconds_secondsMinValue() {
+        ZoneOffset.ofHoursMinutesSeconds(0, 0, Integer.MIN_VALUE);
+    }
+
+    @Test(expectedExceptions=DateTimeException.class)
+    public void test_factory_int_hours_minutes_seconds_minutesAndSecondsMinValue() {
+        ZoneOffset.ofHoursMinutesSeconds(0, Integer.MIN_VALUE, Integer.MIN_VALUE);
+    }
+
     //-----------------------------------------------------------------------
     @Test
     public void test_factory_ofTotalSeconds() {
@@ -437,6 +452,11 @@
         ZoneOffset.ofTotalSeconds(-18 * 60 * 60 - 1);
     }
 
+    @Test(expectedExceptions=DateTimeException.class)
+    public void test_factory_ofTotalSeconds_minValue() {
+        ZoneOffset.ofTotalSeconds(Integer.MIN_VALUE);
+    }
+
     //-----------------------------------------------------------------------
     // from()
     //-----------------------------------------------------------------------