diff --git a/src/java.base/share/classes/java/text/DecimalFormatSymbols.java b/src/java.base/share/classes/java/text/DecimalFormatSymbols.java index 1a968170a30..54e045e0a85 100644 --- a/src/java.base/share/classes/java/text/DecimalFormatSymbols.java +++ b/src/java.base/share/classes/java/text/DecimalFormatSymbols.java @@ -349,10 +349,11 @@ public class DecimalFormatSymbols implements Cloneable, Serializable { * unchanged. * * @param infinity the string representing infinity + * @throws NullPointerException if {@code infinity} is {@code null} */ public void setInfinity(String infinity) { + this.infinity = Objects.requireNonNull(infinity); hashCode = 0; - this.infinity = infinity; } /** @@ -370,10 +371,11 @@ public class DecimalFormatSymbols implements Cloneable, Serializable { * unchanged. * * @param NaN the string representing "not a number" + * @throws NullPointerException if {@code NaN} is {@code null} */ public void setNaN(String NaN) { + this.NaN = Objects.requireNonNull(NaN); hashCode = 0; - this.NaN = NaN; } /** @@ -414,14 +416,18 @@ public class DecimalFormatSymbols implements Cloneable, Serializable { } /** - * Sets the currency symbol for the currency of these - * DecimalFormatSymbols in their locale. + * Sets the currency symbol for the currency of this + * {@code DecimalFormatSymbols} in their locale. Unlike {@link + * #setInternationalCurrencySymbol(String)}, this method does not update + * the currency attribute nor the international currency symbol attribute. * * @param currency the currency symbol + * @throws NullPointerException if {@code currency} is {@code null} * @since 1.2 */ public void setCurrencySymbol(String currency) { + Objects.requireNonNull(currency); initializeCurrency(locale); hashCode = 0; currencySymbol = currency; @@ -448,35 +454,29 @@ public class DecimalFormatSymbols implements Cloneable, Serializable { * this also sets the currency attribute to the corresponding Currency * instance and the currency symbol attribute to the currency's symbol * in the DecimalFormatSymbols' locale. If the currency code is not valid, - * then the currency attribute is set to null and the currency symbol - * attribute is not modified. + * then the currency attribute and the currency symbol attribute are not modified. * * @param currencyCode the currency code + * @throws NullPointerException if {@code currencyCode} is {@code null} * @see #setCurrency * @see #setCurrencySymbol * @since 1.2 */ - public void setInternationalCurrencySymbol(String currencyCode) - { + public void setInternationalCurrencySymbol(String currencyCode) { + Objects.requireNonNull(currencyCode); + // init over setting currencyInit flag as true so that currency has + // fallback if code is not valid initializeCurrency(locale); hashCode = 0; intlCurrencySymbol = currencyCode; - currency = null; - if (currencyCode != null) { - try { - currency = Currency.getInstance(currencyCode); - currencySymbol = currency.getSymbol(); - } catch (IllegalArgumentException e) { - } - } + try { + currency = Currency.getInstance(currencyCode); + currencySymbol = currency.getSymbol(locale); + } catch (IllegalArgumentException _) {} // Simply ignore if not valid } /** - * Gets the currency of these DecimalFormatSymbols. May be null if the - * currency symbol attribute was previously set to a value that's not - * a valid ISO 4217 currency code. - * - * @return the currency used, or null + * {@return the {@code Currency} of this {@code DecimalFormatSymbols}} * @since 1.4 */ public Currency getCurrency() { @@ -485,7 +485,7 @@ public class DecimalFormatSymbols implements Cloneable, Serializable { } /** - * Sets the currency of these DecimalFormatSymbols. + * Sets the currency of this {@code DecimalFormatSymbols}. * This also sets the currency symbol attribute to the currency's symbol * in the DecimalFormatSymbols' locale, and the international currency * symbol attribute to the currency's ISO 4217 currency code. @@ -497,9 +497,7 @@ public class DecimalFormatSymbols implements Cloneable, Serializable { * @see #setInternationalCurrencySymbol */ public void setCurrency(Currency currency) { - if (currency == null) { - throw new NullPointerException(); - } + Objects.requireNonNull(currency); initializeCurrency(locale); hashCode = 0; this.currency = currency; @@ -555,9 +553,7 @@ public class DecimalFormatSymbols implements Cloneable, Serializable { */ public void setExponentSeparator(String exp) { - if (exp == null) { - throw new NullPointerException(); - } + Objects.requireNonNull(exp); hashCode = 0; exponentialSeparator = exp; } @@ -767,7 +763,8 @@ public class DecimalFormatSymbols implements Cloneable, Serializable { patternSeparator == other.patternSeparator && infinity.equals(other.infinity) && NaN.equals(other.NaN) && - getCurrencySymbol().equals(other.getCurrencySymbol()) && // possible currency init occurs here + // Currency fields are lazy. Init via get call to ensure non-null + getCurrencySymbol().equals(other.getCurrencySymbol()) && intlCurrencySymbol.equals(other.intlCurrencySymbol) && currency == other.currency && monetarySeparator == other.monetarySeparator && @@ -800,7 +797,8 @@ public class DecimalFormatSymbols implements Cloneable, Serializable { patternSeparator, infinity, NaN, - getCurrencySymbol(), // possible currency init occurs here + // Currency fields are lazy. Init via get call to ensure non-null + getCurrencySymbol(), intlCurrencySymbol, currency, monetarySeparator, diff --git a/test/jdk/java/text/Format/DecimalFormat/SettersShouldThrowNPETest.java b/test/jdk/java/text/Format/DecimalFormat/SettersShouldThrowNPETest.java new file mode 100644 index 00000000000..b09c9102833 --- /dev/null +++ b/test/jdk/java/text/Format/DecimalFormat/SettersShouldThrowNPETest.java @@ -0,0 +1,109 @@ +/* + * Copyright (c) 2024, 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 8341445 + * @summary DFS setters should throw NPE. This ensures that NPE is not thrown + * by equals(). + * @run junit SettersShouldThrowNPETest + */ + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.text.DecimalFormatSymbols; +import java.util.Arrays; +import java.util.Currency; +import java.util.List; +import java.util.Locale; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +public class SettersShouldThrowNPETest { + + // The public setter methods that should throw NPE + private static final List NPE_SETTERS = + Arrays.stream(DecimalFormatSymbols.class.getDeclaredMethods()) + .filter(m -> Modifier.isPublic(m.getModifiers()) + && m.getName().startsWith("set") + && Stream.of(m.getParameterTypes()).noneMatch(Class::isPrimitive)) + .toList(); + + // Non-primitive setters should throw NPE + @ParameterizedTest + @MethodSource("setters") + public void settersThrowNPETest(Method m) { + var dfs = new DecimalFormatSymbols(); + InvocationTargetException e = + assertThrows(InvocationTargetException.class, () -> m.invoke(dfs, (Object) null)); + if (!(e.getCause() instanceof NullPointerException)) { + throw new RuntimeException(e.getCause() + " was thrown instead of NPE by : " + m); + } + } + + // Currency fields are lazy and can be null + // Ensure when exposed to users, they are never null + @ParameterizedTest + @MethodSource("locales") + public void lazyCurrencyFieldsTest(Locale locale) { + var dfs = new DecimalFormatSymbols(locale); + assertDoesNotThrow(() -> dfs.equals(new DecimalFormatSymbols())); + assertNotNull(dfs.getCurrency()); + assertNotNull(dfs.getInternationalCurrencySymbol()); + assertNotNull(dfs.getCurrencySymbol()); + } + + // Prior to 8341445, if the international currency symbol was invalid, + // the currency attribute was set to null. However, we should not have null + // currency fields post initializeCurrency() call. Ensure invalid code + // does not update the other fields. + @Test + public void setInternationalCurrencySymbolFallbackTest() { + var code = "fooBarBazQux"; + // initialize() should provide null for all currency related fields + var dfs = new DecimalFormatSymbols(Locale.ROOT); + // Load the fallbacks via initCurrency() since the loc is Locale.ROOT + dfs.setInternationalCurrencySymbol(code); // set invalid code + // Ensure our values are the expected fallbacks, minus the updated intl code + assertEquals(Currency.getInstance("XXX"), dfs.getCurrency()); + assertEquals("\u00A4", dfs.getCurrencySymbol()); + assertEquals("fooBarBazQux", dfs.getInternationalCurrencySymbol()); + } + + private static List setters() { + return NPE_SETTERS; + } + + private static List locales() { + return List.of(Locale.ROOT, Locale.US, Locale.forLanguageTag("XXX")); + } +}