8212749: DecimalFormat.setGroupingSize(int) allows setting negative grouping size

8231984: Clarify semantics of DecimalFormat.getGroupingSize(0)

Reviewed-by: rriggs

src/java.base/share/classes/java/text/DecimalFormat.java

@@ -2756,7 +2756,10 @@ public class DecimalFormat extends NumberFormat {
     /**
      * Return the grouping size. Grouping size is the number of digits between
      * grouping separators in the integer portion of a number.  For example,
-     * in the number "123,456.78", the grouping size is 3.
+     * in the number "123,456.78", the grouping size is 3. Grouping size of
+     * zero designates that grouping is not used, which provides the same
+     * formatting as if calling {@link #setGroupingUsed(boolean)
+     * setGroupingUsed(false)}.
      *
      * @return the grouping size
      * @see #setGroupingSize
@@ -2770,16 +2773,28 @@ public class DecimalFormat extends NumberFormat {
     /**
      * Set the grouping size. Grouping size is the number of digits between
      * grouping separators in the integer portion of a number.  For example,
-     * in the number "123,456.78", the grouping size is 3.
+     * in the number "123,456.78", the grouping size is 3. Grouping size of
-     * <br>
+     * zero designates that grouping is not used, which provides the same
+     * formatting as if calling {@link #setGroupingUsed(boolean)
+     * setGroupingUsed(false)}.
+     * <p>
      * The value passed in is converted to a byte, which may lose information.
+     * Values that are negative or greater than
+     * {@link java.lang.Byte#MAX_VALUE Byte.MAX_VALUE}, will throw an
+     * {@code IllegalArgumentException}.
      *
      * @param newValue the new grouping size
      * @see #getGroupingSize
      * @see java.text.NumberFormat#setGroupingUsed
      * @see java.text.DecimalFormatSymbols#setGroupingSeparator
+     * @throws IllegalArgumentException if {@code newValue} is negative or
+     *          greater than {@link java.lang.Byte#MAX_VALUE Byte.MAX_VALUE}
      */
     public void setGroupingSize (int newValue) {
+        if (newValue < 0 || newValue > Byte.MAX_VALUE) {
+            throw new IllegalArgumentException(
+                "newValue is out of valid range. value: " + newValue);
+        }
         groupingSize = (byte)newValue;
         fastPathCheckNeeded = true;
     }
@@ -3906,6 +3921,12 @@ public class DecimalFormat extends NumberFormat {
             // Didn't have exponential fields
             useExponentialNotation = false;
         }
+
+        // Restore the invariant value if groupingSize is invalid.
+        if (groupingSize < 0) {
+            groupingSize = 3;
+        }
+
         serialVersionOnStream = currentSerialVersion;
     }
 
@@ -4009,14 +4030,15 @@ public class DecimalFormat extends NumberFormat {
 
     /**
      * The number of digits between grouping separators in the integer
-     * portion of a number.  Must be greater than 0 if
+     * portion of a number.  Must be non-negative and less than or equal to
+     * {@link java.lang.Byte#MAX_VALUE Byte.MAX_VALUE} if
      * {@code NumberFormat.groupingUsed} is true.
      *
      * @serial
      * @see #getGroupingSize
      * @see java.text.NumberFormat#isGroupingUsed
      */
-    private byte    groupingSize = 3;  // invariant, > 0 if useThousands
+    private byte    groupingSize = 3;  // invariant, 0 - 127, if groupingUsed
 
     /**
      * If true, forces the decimal separator to always appear in a formatted

test/jdk/java/text/Format/DecimalFormat/SetGroupingSizeTest.java

@@ -0,0 +1,74 @@
+/*
+ * Copyright (c) 2019, 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.
+ */
+
+/*
+ * @test
+ * @bug 8212749
+ * @summary test whether input value check for
+ *          DecimalFormat.setGroupingSize(int) works correctly.
+ * @run testng/othervm SetGroupingSizeTest
+ */
+
+import java.text.DecimalFormat;
+
+import static org.testng.Assert.assertEquals;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+@Test
+public class SetGroupingSizeTest {
+
+    @DataProvider
+    public static Object[][] validGroupingSizes() {
+        return new Object[][] {
+            { 0 },
+            { Byte.MAX_VALUE },
+        };
+    }
+
+    @DataProvider
+    public static Object[][] invalidGroupingSizes() {
+        return new Object[][] {
+            { Byte.MIN_VALUE - 1 },
+            { Byte.MIN_VALUE },
+            { -1 },
+            { Byte.MAX_VALUE + 1 },
+            { Integer.MIN_VALUE },
+            { Integer.MAX_VALUE },
+        };
+    }
+
+    @Test(dataProvider = "validGroupingSizes")
+    public void test_validGroupingSize(int newVal) {
+        DecimalFormat df = new DecimalFormat();
+        df.setGroupingSize(newVal);
+        assertEquals(df.getGroupingSize(), newVal);
+    }
+
+    @Test(dataProvider = "invalidGroupingSizes",
+        expectedExceptions = IllegalArgumentException.class)
+    public void test_invalidGroupingSize(int newVal) {
+        DecimalFormat df = new DecimalFormat();
+        df.setGroupingSize(newVal);
+    }
+}