Formatter treats index, width and precision > Integer.MAX_VALUE incorrectly

Summary

Formatter should specify a valid range for numeric values given to index, width, and precision arguments in format strings. The existing implementation leaves this undefined and prone to integer parsing issues when the given value is too large or too small. The behavior for invalid numerics in the format string needs to be specified to eliminate this undefined behavior. Additionally for index specifiers, the case for a zero-value is not valid, but behavior for this case leads to unexpected results and should be defined; see JDK-8253393.

Problem

The java.util.Formatter class includes support for numeric arguments to assist with string minimum width, argument indexing for variadic formatter arguments, and the number of digits to print for FP precision.

String.format("%2147483648s", 1) // Acts like no explicit width
String.format("%s %2147483648$s", 1) // Acts like "Relative indexing"
String.format("%.2147483648f", 1.0) // Acts like no explicit precision

The above examples are taken from JDK-8253459. This misbehavior occurs because the numbers for these arguments are assumed to be no larger than Integer.MAX_VALUE (failure to parse by Integer.parseInt). For this issue to be properly resolved, more precision is required on the part of the format string specification.

Background

The existing design of the java.util.Formatter includes highly specific exception types. Most of the exceptions that reside in the java.util package belong to Formatter. There currently are 12 exception classes related to Formatter. Of these, 11 are subclasses to IllegalFormatException to cover every specific error that can occur with a format string. The IllegalFormatException class is not abstract, but it has no public constructor. The intent for this class appears for it to be a "catchall" supertype for a number of number of more specific sub-type exceptions. A number of methods in this library are documented as throwing IllegalFormatException. The IllegalFormatException is a subtype of IllegalArgumentException. Existing code dependent upon this library and exceptions it throws may not catch IllegalArgumentException in these cases, though. Instead usage is steered to catching IllegalArgumentException or one of its subclasses.

Solution

The solution to this problem is to specify the valid ranges for format specifiers. Widths will be valid in the range [1, Integer.MAX_VALUE]. Indexes will be valid in the range [1,Integer.MAX_VALUE]. Precision will be valid in the range [0, Integer.MAX_VALUE].

The goal of this solution is to clarify supported numeric ranges for these specifiers and the behavior that results when a specifier is given outside of this range.

The solution adds an additional package-private subtype of IllegalFormatException (i.e. IllegalFormatArgumentIndexException) to keep with the existing coding convention of an exception for every potential error case. There should be no additional subtypes required as this completes the convention that is already in place. We avoid additional exception types being added to the public API by presenting this error as an IllegalFormatException. Should the need arise, this newly added private type can be made public to alleviate unforeseen compatibility issues.

We additionally apply the use of IllegalFormatArgumentIndexException to the case where the index argument is zero. If presented with such an argument, this exception is thrown. The documentation specifies that all argument index be a positive integer that is 1-indexed. No behavior has been defined for the zeroth case, and currently the undefined behavior results in buggy interplay with ordinary indexing. This will present as IllegalFormatException in the public API.

Alternatives

There are a few alternatives to the aforementioned approach that present a solution that does not add an existing exception type. They come with their own tradeoffs with regards to impacting the design or bending its semantics.

Using IllegalArgumentException

The IllegalArgumentException is the supertype of IllegalFormatException. Semantically, it's also in the right neighborhood of exceptions for our use. An ill-formed format string is technically an illegal argument. However, utilizing this type would cause any code that catches IllegalFormatException to miss an exception intended to be caught in this case. For this reason IllegalArgumentException should be avoided.

Reusing an existing subtype of IllegalFormatException

One possibility is to repurpose an existing exception subclass of IllegalFormatException. This runs counter to the existing design in the most extreme way of possible alternatives. The similar errors that can occur in a format string (IllegalFormatWidthException and IllegalFormatPrecisionException) are the two adjacent exceptions to this exception that correspond to two other specifier errors that can occur in a format string. Neither of them fit the error case at hand very well. Of the remaining subtypes, there is not a good fit for this case without an obtuse reinterpretation the reused class's semantics or just arbitrarily assigning another exception type to this case and describing such in the documentation.

Using IllegalFormatException Directly

One solution could be to throw IllegalFormatException instead of adding the new IllegalFormatArgumentIndexException exception type. This would require retrofitting the existing class with a public constructor as well as some helper methods to support useful error messages. The drawback in this case is that it goes against the original design by making an "abstract" (at least by intent) class concrete. If this route is taken, future work could be to investigate consolidating all of the existing subtypes into the superclass to bring the design into consistency. Of all the given alternatives, this one seems the most viable.

Using IllegalFormatArgumentIndexException Directly

This approach is was the original proposal of this CSR. A compromise solution was developed that uses this exception indirectly by making it package private, but presenting it as IllegalFormatException in the public API. This prevents further exception bloat in this library. If a need is demonstrated for this exception to be made public and presented directly in the public API, the change is as easy as making it public and updating the documentation accordingly.

Specification

diff --git a/src/java.base/share/classes/java/util/Formatter.java b/src/java.base/share/classes/java/util/Formatter.java
index 184bd0bc124..2bfb735aea3 100644
--- a/src/java.base/share/classes/java/util/Formatter.java
+++ b/src/java.base/share/classes/java/util/Formatter.java
@@ -692,12 +692,28 @@ import sun.util.locale.provider.ResourceBundleBasedAdapter;
  * <p> If the format specifier contains a width or precision with an invalid
  * value or which is otherwise unsupported, then a {@link
  * IllegalFormatWidthException} or {@link IllegalFormatPrecisionException}
- * respectively will be thrown.
+ * respectively will be thrown. Similarly, values of zero for an argument
+ * index will result in an {@link IllegalFormatException}.
  *
  * <p> If a format specifier contains a conversion character that is not
  * applicable to the corresponding argument, then an {@link
  * IllegalFormatConversionException} will be thrown.
  *
+ * <p> Values of <i>precision</i> must be in the range zero to
+ * {@link Integer#MAX_VALUE}, inclusive, otherwise
+ * {@link IllegalFormatPrecisionException} is thrown.</p>
+ *
+ * <p> Values of <i>width</i> must be in the range one to
+ * {@link Integer#MAX_VALUE}, inclusive, otherwise
+ * {@link IllegalFormatWidthException} will be thrown
+ * Note that widths can appear to have a negative value, but the negative sign
+ * is a <i>flag</i>. For example in the format string {@code "%-20s"} the
+ * <i>width</i> is <i>20</i> and the <i>flag</i> is "-".</p>
+ *
+ * <p> Values of <i>index</i> must be in the range one to
+ * {@link Integer#MAX_VALUE}, inclusive, otherwise
+ * {@link IllegalFormatException} will be thrown.</p>
+ *
  * <p> All specified exceptions may be thrown by any of the {@code format}
  * methods of {@code Formatter} as well as by any {@code format} convenience
  * methods such as {@link String#format(String,Object...) String.format} and
@@ -2783,8 +2799,11 @@ public final class Formatter implements Closeable, Flushable {
                 try {
                     // skip the trailing '$'
                     index = Integer.parseInt(s, start, end - 1, 10);
+                    if(index <= 0) {
+                       throw new IllegalFormatArgumentIndexException(index);
+                    }
                 } catch (NumberFormatException x) {
-                    assert(false);
+                    throw new IllegalFormatArgumentIndexException(Integer.MIN_VALUE);
                 }
             } else {
                 index = 0;
@@ -2811,7 +2830,7 @@ public final class Formatter implements Closeable, Flushable {
                     if (width < 0)
                         throw new IllegalFormatWidthException(width);
                 } catch (NumberFormatException x) {
-                    assert(false);
+                    throw new IllegalFormatWidthException(Integer.MIN_VALUE);
                 }
             }
             return width;
@@ -2826,7 +2845,7 @@ public final class Formatter implements Closeable, Flushable {
                     if (precision < 0)
                         throw new IllegalFormatPrecisionException(precision);
                 } catch (NumberFormatException x) {
-                    assert(false);
+                    throw new IllegalFormatPrecisionException(Integer.MIN_VALUE);
                 }
             }
             return precision;
diff --git a/src/java.base/share/classes/java/util/IllegalFormatArgumentIndexException.java b/src/java.base/share/classes/java/util/IllegalFormatArgumentIndexException.java
new file mode 100644
index 00000000000..e8c652ae15b
--- /dev/null
+++ b/src/java.base/share/classes/java/util/IllegalFormatArgumentIndexException.java
@@ -0,0 +1,61 @@
+/*
+ * 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.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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 java.util;
+
+/**
+ * Unchecked exception thrown when the argument index is not within the valid
+ * range of supported argument index values. If an index value isn't
+ * representable by an {@code int} type, then the value
+ * {@code Integer.MIN_VALUE} will be used in the exception.
+ *
+ * @since 16
+ */
+class IllegalFormatArgumentIndexException extends IllegalFormatException {
+
+    @java.io.Serial
+    private static final long serialVersionUID = 4191767811181838112L;
+
+    private final int illegalIndex;
+
+    /**
+     * Constructs an instance of this class with the specified argument index
+     * @param index The value of a corresponding illegal argument index.
+     */
+    IllegalFormatArgumentIndexException(int index) {
+        illegalIndex = index;
+    }
+
+    /**
+     * Gets the value of the illegal index.
+     * Returns {@code Integer.MIN_VALUE} if the illegal index is not
+     * representable by an integer type.
+     * @return the illegal index value
+     */
+    int getIndex() {
+        return illegalIndex;
+    }
+
+}
diff --git a/src/java.base/share/classes/java/util/IllegalFormatPrecisionException.java b/src/java.base/share/classes/java/util/IllegalFormatPrecisionException.java
index 1d6ea4ae8d8..e855f22d08e 100644
--- a/src/java.base/share/classes/java/util/IllegalFormatPrecisionException.java
+++ b/src/java.base/share/classes/java/util/IllegalFormatPrecisionException.java
@@ -28,7 +28,9 @@ package java.util;
 /**
  * Unchecked exception thrown when the precision is a negative value other than
  * {@code -1}, the conversion does not support a precision, or the value is
- * otherwise unsupported.
+ * otherwise unsupported. If the precision is not representable by an
+ * {@code int} type, then the value {@code Integer.MIN_VALUE} will be used
+ * in the exception.
  *
  * @since 1.5
  */
@@ -50,7 +52,8 @@ public class IllegalFormatPrecisionException extends IllegalFormatException {
     }

     /**
-     * Returns the precision
+     * Returns the precision. If the precision isn't representable by an
+     * integer type, then will return {@code Integer.MIN_VALUE}.
      *
      * @return  The precision
      */
diff --git a/src/java.base/share/classes/java/util/IllegalFormatWidthException.java b/src/java.base/share/classes/java/util/IllegalFormatWidthException.java
index 7d73ea52c19..8dbb0932924 100644
--- a/src/java.base/share/classes/java/util/IllegalFormatWidthException.java
+++ b/src/java.base/share/classes/java/util/IllegalFormatWidthException.java
@@ -27,7 +27,9 @@ package java.util;

 /**
  * Unchecked exception thrown when the format width is a negative value other
- * than {@code -1} or is otherwise unsupported.
+ * than {@code -1} or is otherwise unsupported. If a given format width is not
+ * representable by an {@code int} type, then the value
+ * {@code Integer.MIN_VALUE} will be used in the exception.
  *
  * @since 1.5
  */
@@ -49,7 +51,8 @@ public class IllegalFormatWidthException extends IllegalFormatException {
     }

     /**
-     * Returns the width
+     * Returns the width. If the width is not representable by an integer type,
+     * then returns {@code Integer.MIN_VALUE}.
      *
      * @return  The width
      */