From 080c707aabcf1b2ed30009cb408ecf52bd1784fd Mon Sep 17 00:00:00 2001 From: Ian Graves Date: Thu, 19 Nov 2020 20:20:55 +0000 Subject: [PATCH] 8253459: Formatter treats index, width and precision > Integer.MAX_VALUE incorrectly Reviewed-by: rriggs, smarks --- .../share/classes/java/util/Formatter.java | 27 +++++-- .../IllegalFormatArgumentIndexException.java | 72 +++++++++++++++++++ .../util/IllegalFormatPrecisionException.java | 7 +- .../util/IllegalFormatWidthException.java | 7 +- .../TestFormatSpecifierBounds.java | 70 ++++++++++++++++++ 5 files changed, 175 insertions(+), 8 deletions(-) create mode 100644 src/java.base/share/classes/java/util/IllegalFormatArgumentIndexException.java create mode 100644 test/jdk/java/util/IllegalFormatException/TestFormatSpecifierBounds.java diff --git a/src/java.base/share/classes/java/util/Formatter.java b/src/java.base/share/classes/java/util/Formatter.java index 184bd0bc124..e5934b28e78 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 @@ *

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}. * *

If a format specifier contains a conversion character that is not * applicable to the corresponding argument, then an {@link * IllegalFormatConversionException} will be thrown. * + *

Values of precision must be in the range zero to + * {@link Integer#MAX_VALUE}, inclusive, otherwise + * {@link IllegalFormatPrecisionException} is thrown.

+ * + *

Values of width 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 flag. For example in the format string {@code "%-20s"} the + * width is 20 and the flag is "-".

+ * + *

Values of index must be in the range one to + * {@link Integer#MAX_VALUE}, inclusive, otherwise + * {@link IllegalFormatException} will be thrown.

+ * *

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 @@ private int index(String s, int start, int end) { 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 @@ private int width(String s, int start, int end) { if (width < 0) throw new IllegalFormatWidthException(width); } catch (NumberFormatException x) { - assert(false); + throw new IllegalFormatWidthException(Integer.MIN_VALUE); } } return width; @@ -2826,7 +2845,7 @@ private int precision(String s, int start, int end) { 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..02d5449098d --- /dev/null +++ b/src/java.base/share/classes/java/util/IllegalFormatArgumentIndexException.java @@ -0,0 +1,72 @@ +/* + * 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 {@code int}. + * @return the illegal index value + */ + int getIndex() { + return illegalIndex; + } + + @Override + public String getMessage() { + int index = getIndex(); + + if (index == Integer.MIN_VALUE) { + return "Format argument index: (not representable as int)"; + } + + return String.format("Illegal format argument index = %d", getIndex()); + } + +} diff --git a/src/java.base/share/classes/java/util/IllegalFormatPrecisionException.java b/src/java.base/share/classes/java/util/IllegalFormatPrecisionException.java index 1d6ea4ae8d8..bdf3de6f528 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 @@ /** * 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 IllegalFormatPrecisionException(int p) { } /** - * Returns the precision + * Returns the precision. If the precision isn't representable by an + * {@code int}, 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..e00ec01eee0 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 @@ /** * 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 IllegalFormatWidthException(int w) { } /** - * Returns the width + * Returns the width. If the width is not representable by an {@code int}, + * then returns {@code Integer.MIN_VALUE}. * * @return The width */ diff --git a/test/jdk/java/util/IllegalFormatException/TestFormatSpecifierBounds.java b/test/jdk/java/util/IllegalFormatException/TestFormatSpecifierBounds.java new file mode 100644 index 00000000000..e1fdc6dd643 --- /dev/null +++ b/test/jdk/java/util/IllegalFormatException/TestFormatSpecifierBounds.java @@ -0,0 +1,70 @@ +/* + * 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. + */ + +/* + * @test + * @bug 8253459 + * @run testng TestFormatSpecifierBounds + */ + +import java.util.*; +import org.testng.annotations.Test; +import static org.testng.Assert.*; + +@Test +public class TestFormatSpecifierBounds { + + public void testZeroIndex() { + IllegalFormatException e = expectThrows(IllegalFormatException.class, () -> { + String r = String.format("%0$s", "A", "B"); + }); + assertEquals(e.getMessage(), "Illegal format argument index = 0"); + } + + public void testNonRepresentableIntIndex() { + IllegalFormatException e = expectThrows(IllegalFormatException.class, () -> { + String r = String.format("%2147483648$s", "A", "B"); + }); + assertEquals(e.getMessage(), "Format argument index: (not representable as int)"); + } + + public void testZeroWidth() { + assertThrows(IllegalFormatException.class, () -> { + String r = String.format("%0s", "A", "B"); + }); + } + + public void testNonRepresentableWidth() { + IllegalFormatException e = expectThrows(IllegalFormatException.class, () -> { + String r = String.format("%2147483648s", "A", "B"); + }); + assertEquals(e.getMessage(), Integer.toString(Integer.MIN_VALUE)); + } + + public void testNonRepresentablePrecision() { + IllegalFormatException e = expectThrows(IllegalFormatException.class, () -> { + String r = String.format("%.2147483648s", "A", "B"); + }); + assertEquals(e.getMessage(), Integer.toString(Integer.MIN_VALUE)); + } +}