8190904: Incorrect currency instance returned by java.util.Currency.getInstance()

Reviewed-by: naoto

src/java.base/share/classes/java/util/Currency.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2018, 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
@@ -41,6 +41,7 @@ import java.util.concurrent.ConcurrentMap;
 import java.util.regex.Pattern;
 import java.util.regex.Matcher;
 import java.util.spi.CurrencyNameProvider;
+import java.util.stream.Collectors;
 import sun.util.locale.provider.CalendarDataUtility;
 import sun.util.locale.provider.LocaleServiceProviderPool;
 import sun.util.logging.PlatformLogger;
@@ -77,7 +78,10 @@ import sun.util.logging.PlatformLogger;
  * JP=JPZ,999,0
  * </code>
  * <p>
- * will supersede the currency data for Japan.
+ * will supersede the currency data for Japan. If JPZ is one of the existing
+ * ISO 4217 currency code referred by other countries, the existing
+ * JPZ currency data is updated with the given numeric code and minor
+ * unit value.
  *
  * <p>
  * <code>
@@ -93,6 +97,11 @@ import sun.util.logging.PlatformLogger;
  * country code entries exist, the behavior of the Currency information for that
  * {@code Currency} is undefined and the remainder of entries in file are processed.
  * <p>
+ * If multiple property entries with same currency code but different numeric code
+ * and/or minor unit are encountered, those entries are ignored and the remainder
+ * of entries in file are processed.
+ *
+ * <p>
  * It is recommended to use {@link java.math.BigDecimal} class while dealing
  * with {@code Currency} or monetary values as it provides better handling of floating
  * point numbers and their operations.
@@ -237,19 +246,17 @@ public final class Currency implements Serializable {
                         try (FileReader fr = new FileReader(propFile)) {
                             props.load(fr);
                         }
-                        Set<String> keys = props.stringPropertyNames();
                         Pattern propertiesPattern =
-                            Pattern.compile("([A-Z]{3})\\s*,\\s*(\\d{3})\\s*,\\s*" +
+                                Pattern.compile("([A-Z]{3})\\s*,\\s*(\\d{3})\\s*,\\s*" +
-                                "(\\d+)\\s*,?\\s*(\\d{4}-\\d{2}-\\d{2}T\\d{2}:" +
+                                        "(\\d+)\\s*,?\\s*(\\d{4}-\\d{2}-\\d{2}T\\d{2}:" +
-                                "\\d{2}:\\d{2})?");
+                                        "\\d{2}:\\d{2})?");
-                        for (String key : keys) {
+                        List<CurrencyProperty> currencyEntries
-                           replaceCurrencyData(propertiesPattern,
+                                = getValidCurrencyData(props, propertiesPattern);
-                               key.toUpperCase(Locale.ROOT),
+                        currencyEntries.forEach(Currency::replaceCurrencyData);
-                               props.getProperty(key).toUpperCase(Locale.ROOT));
-                        }
                     }
                 } catch (IOException e) {
-                    info("currency.properties is ignored because of an IOException", e);
+                    CurrencyProperty.info("currency.properties is ignored"
+                            + " because of an IOException", e);
                 }
                 return null;
             }
@@ -769,71 +776,111 @@ public final class Currency implements Serializable {
     }
 
     /**
-     * Replaces currency data found in the currencydata.properties file
+     * Parse currency data found in the properties file (that
+     * java.util.currency.data designates) to a List of CurrencyProperty
+     * instances. Also, remove invalid entries and the multiple currency
+     * code inconsistencies.
      *
-     * @param pattern regex pattern for the properties
+     * @param props properties containing currency data
-     * @param ctry country code
+     * @param pattern regex pattern for the properties entry
-     * @param curdata currency data.  This is a comma separated string that
+     * @return list of parsed property entries
-     *    consists of "three-letter alphabet code", "three-digit numeric code",
-     *    and "one-digit (0-9) default fraction digit".
-     *    For example, "JPZ,392,0".
-     *    An optional UTC date can be appended to the string (comma separated)
-     *    to allow a currency change take effect after date specified.
-     *    For example, "JP=JPZ,999,0,2014-01-01T00:00:00" has no effect unless
-     *    UTC time is past 1st January 2014 00:00:00 GMT.
      */
-    private static void replaceCurrencyData(Pattern pattern, String ctry, String curdata) {
+    private static List<CurrencyProperty> getValidCurrencyData(Properties props,
+            Pattern pattern) {
 
-        if (ctry.length() != 2) {
+        Set<String> keys = props.stringPropertyNames();
-            // ignore invalid country code
+        List<CurrencyProperty> propertyEntries = new ArrayList<>();
-            info("currency.properties entry for " + ctry +
-                    " is ignored because of the invalid country code.", null);
-            return;
-        }
 
-        Matcher m = pattern.matcher(curdata);
+        // remove all invalid entries and parse all valid currency properties
-        if (!m.find() || (m.group(4) == null && countOccurrences(curdata, ',') >= 3)) {
+        // entries to a group of CurrencyProperty, classified by currency code
-            // format is not recognized.  ignore the data
+        Map<String, List<CurrencyProperty>> currencyCodeGroup = keys.stream()
-            // if group(4) date string is null and we've 4 values, bad date value
+                .map(k -> CurrencyProperty
-            info("currency.properties entry for " + ctry +
+                .getValidEntry(k.toUpperCase(Locale.ROOT),
-                    " ignored because the value format is not recognized.", null);
+                        props.getProperty(k).toUpperCase(Locale.ROOT),
-            return;
+                        pattern)).flatMap(o -> o.stream())
-        }
+                .collect(Collectors.groupingBy(entry -> entry.currencyCode));
 
-        try {
+        // check each group for inconsistencies
-            if (m.group(4) != null && !isPastCutoverDate(m.group(4))) {
+        currencyCodeGroup.forEach((curCode, list) -> {
-                info("currency.properties entry for " + ctry +
+            boolean inconsistent = CurrencyProperty
-                        " ignored since cutover date has not passed :" + curdata, null);
+                    .containsInconsistentInstances(list);
-                return;
+            if (inconsistent) {
+                list.forEach(prop -> CurrencyProperty.info("The property"
+                        + " entry for " + prop.country + " is inconsistent."
+                        + " Ignored.", null));
+            } else {
+                propertyEntries.addAll(list);
             }
-        } catch (ParseException ex) {
+        });
-            info("currency.properties entry for " + ctry +
-                        " ignored since exception encountered :" + ex.getMessage(), null);
-            return;
-        }
 
-        String code = m.group(1);
+        return propertyEntries;
-        int numeric = Integer.parseInt(m.group(2));
+    }
+
+    /**
+     * Replaces currency data found in the properties file that
+     * java.util.currency.data designates. This method is invoked for
+     * each valid currency entry.
+     *
+     * @param prop CurrencyProperty instance of the valid property entry
+     */
+    private static void replaceCurrencyData(CurrencyProperty prop) {
+
+
+        String ctry = prop.country;
+        String code = prop.currencyCode;
+        int numeric = prop.numericCode;
+        int fraction = prop.fraction;
         int entry = numeric << NUMERIC_CODE_SHIFT;
-        int fraction = Integer.parseInt(m.group(3));
-        if (fraction > SIMPLE_CASE_COUNTRY_MAX_DEFAULT_DIGITS) {
-            info("currency.properties entry for " + ctry +
-                " ignored since the fraction is more than " +
-                SIMPLE_CASE_COUNTRY_MAX_DEFAULT_DIGITS + ":" + curdata, null);
-            return;
-        }
 
         int index = SpecialCaseEntry.indexOf(code, fraction, numeric);
 
-        /* if a country switches from simple case to special case or
+
+        // If a new entry changes the numeric code/dfd of an existing
+        // currency code, update it in the sc list at the respective
+        // index and also change it in the other currencies list and
+        // main table (if that currency code is also used as a
+        // simple case).
+
+        // If all three components do not match with the new entry,
+        // but the currency code exists in the special case list
+        // update the sc entry with the new entry
+        int scCurrencyCodeIndex = -1;
+        if (index == -1) {
+            scCurrencyCodeIndex = SpecialCaseEntry.currencyCodeIndex(code);
+            if (scCurrencyCodeIndex != -1) {
+                //currency code exists in sc list, then update the old entry
+                specialCasesList.set(scCurrencyCodeIndex,
+                        new SpecialCaseEntry(code, fraction, numeric));
+
+                // also update the entry in other currencies list
+                OtherCurrencyEntry oe = OtherCurrencyEntry.findEntry(code);
+                if (oe != null) {
+                    int oIndex = otherCurrenciesList.indexOf(oe);
+                    otherCurrenciesList.set(oIndex, new OtherCurrencyEntry(
+                            code, fraction, numeric));
+                }
+            }
+        }
+
+        /* If a country switches from simple case to special case or
          * one special case to other special case which is not present
-         * in the sc arrays then insert the new entry in special case arrays
+         * in the sc arrays then insert the new entry in special case arrays.
+         * If an entry with given currency code exists, update with the new
+         * entry.
          */
         if (index == -1 && (ctry.charAt(0) != code.charAt(0)
                 || ctry.charAt(1) != code.charAt(1))) {
 
-            specialCasesList.add(new SpecialCaseEntry(code, fraction, numeric));
+            if(scCurrencyCodeIndex == -1) {
-            index = specialCasesList.size() - 1;
+                specialCasesList.add(new SpecialCaseEntry(code, fraction,
+                        numeric));
+                index = specialCasesList.size() - 1;
+            } else {
+                index = scCurrencyCodeIndex;
+            }
+
+            // update the entry in main table if it exists as a simple case
+            updateMainTableEntry(code, fraction, numeric);
         }
 
         if (index == -1) {
@@ -848,32 +895,29 @@ public final class Currency implements Serializable {
         setMainTableEntry(ctry.charAt(0), ctry.charAt(1), entry);
     }
 
-    private static boolean isPastCutoverDate(String s) throws ParseException {
+    // update the entry in maintable for any simple case found, if a new
-        SimpleDateFormat format = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss", Locale.ROOT);
+    // entry as a special case updates the entry in sc list with
-        format.setTimeZone(TimeZone.getTimeZone("UTC"));
+    // existing currency code
-        format.setLenient(false);
+    private static void updateMainTableEntry(String code, int fraction,
-        long time = format.parse(s.trim()).getTime();
+            int numeric) {
-        return System.currentTimeMillis() > time;
+        // checking the existence of currency code in mainTable
+        int tableEntry = getMainTableEntry(code.charAt(0), code.charAt(1));
+        int entry = numeric << NUMERIC_CODE_SHIFT;
+        if ((tableEntry & COUNTRY_TYPE_MASK) == SIMPLE_CASE_COUNTRY_MASK
+                && tableEntry != INVALID_COUNTRY_ENTRY
+                && code.charAt(2) - 'A' == (tableEntry
+                & SIMPLE_CASE_COUNTRY_FINAL_CHAR_MASK)) {
 
-    }
+            int numericCode = (tableEntry & NUMERIC_CODE_MASK)
-
+                    >> NUMERIC_CODE_SHIFT;
-    private static int countOccurrences(String value, char match) {
+            int defaultFractionDigits = (tableEntry
-        int count = 0;
+                    & SIMPLE_CASE_COUNTRY_DEFAULT_DIGITS_MASK)
-        for (char c : value.toCharArray()) {
+                    >> SIMPLE_CASE_COUNTRY_DEFAULT_DIGITS_SHIFT;
-            if (c == match) {
+            if (numeric != numericCode || fraction != defaultFractionDigits) {
-               ++count;
+                // update the entry in main table
-            }
+                entry |= (fraction << SIMPLE_CASE_COUNTRY_DEFAULT_DIGITS_SHIFT)
-        }
+                        | (code.charAt(2) - 'A');
-        return count;
+                setMainTableEntry(code.charAt(0), code.charAt(1), entry);
-    }
-
-    private static void info(String message, Throwable t) {
-        PlatformLogger logger = PlatformLogger.getLogger("java.util.Currency");
-        if (logger.isLoggable(PlatformLogger.Level.INFO)) {
-            if (t != null) {
-                logger.info(message, t);
-            } else {
-                logger.info(message);
             }
         }
     }
@@ -959,6 +1003,25 @@ public final class Currency implements Serializable {
             return fractionAndNumericCode;
         }
 
+        // get the index based on currency code
+        private static int currencyCodeIndex(String code) {
+            int size = specialCasesList.size();
+            for (int index = 0; index < size; index++) {
+                SpecialCaseEntry scEntry = specialCasesList.get(index);
+                if (scEntry.oldCurrency.equals(code) && (scEntry.cutOverTime == Long.MAX_VALUE
+                        || System.currentTimeMillis() < scEntry.cutOverTime)) {
+                    //consider only when there is no new currency or cutover time is not passed
+                    return index;
+                } else if (scEntry.newCurrency.equals(code)
+                        && System.currentTimeMillis() >= scEntry.cutOverTime) {
+                    //consider only if the cutover time is passed
+                    return index;
+                }
+            }
+            return -1;
+        }
+
+
         // convert the special case entry to sc arrays index
         private static int toIndex(int tableEntry) {
             return (tableEntry & SPECIAL_CASE_COUNTRY_INDEX_MASK) - SPECIAL_CASE_COUNTRY_INDEX_DELTA;
@@ -999,6 +1062,136 @@ public final class Currency implements Serializable {
 
     }
 
+
+    /*
+     * Used to represent an entry of the properties file that
+     * java.util.currency.data designates
+     *
+     * - country: country representing the currency entry
+     * - currencyCode: currency code
+     * - fraction: default fraction digit
+     * - numericCode: numeric code
+     * - date: cutover date
+     */
+    private static class CurrencyProperty {
+        final private String country;
+        final private String currencyCode;
+        final private int fraction;
+        final private int numericCode;
+        final private String date;
+
+        private CurrencyProperty(String country, String currencyCode,
+                int fraction, int numericCode, String date) {
+            this.country = country;
+            this.currencyCode = currencyCode;
+            this.fraction = fraction;
+            this.numericCode = numericCode;
+            this.date = date;
+        }
+
+        /**
+         * Check the valid currency data and create/return an Optional instance
+         * of CurrencyProperty
+         *
+         * @param ctry    country representing the currency data
+         * @param curData currency data of the given {@code ctry}
+         * @param pattern regex pattern for the properties entry
+         * @return Optional containing CurrencyProperty instance, If valid;
+         *         empty otherwise
+         */
+        private static Optional<CurrencyProperty> getValidEntry(String ctry,
+                String curData,
+                Pattern pattern) {
+
+            CurrencyProperty prop = null;
+
+            if (ctry.length() != 2) {
+                // Invalid country code. Ignore the entry.
+            } else {
+
+                prop = parseProperty(ctry, curData, pattern);
+                // if the property entry failed any of the below checked
+                // criteria it is ignored
+                if (prop == null
+                        || (prop.date == null && curData.chars()
+                                .map(c -> c == ',' ? 1 : 0).sum() >= 3)) {
+                    // format is not recognized.  ignore the data if date
+                    // string is null and we've 4 values, bad date value
+                    prop = null;
+                } else if (prop.fraction
+                        > SIMPLE_CASE_COUNTRY_MAX_DEFAULT_DIGITS) {
+                    prop = null;
+                } else {
+                    try {
+                        if (prop.date != null
+                                && !isPastCutoverDate(prop.date)) {
+                            prop = null;
+                        }
+                    } catch (ParseException ex) {
+                        prop = null;
+                    }
+                }
+            }
+
+            if (prop == null) {
+                info("The property entry for " + ctry + " is invalid."
+                        + " Ignored.", null);
+            }
+
+            return Optional.ofNullable(prop);
+        }
+
+        /*
+         * Parse properties entry and return CurrencyProperty instance
+         */
+        private static CurrencyProperty parseProperty(String ctry,
+                String curData, Pattern pattern) {
+            Matcher m = pattern.matcher(curData);
+            if (!m.find()) {
+                return null;
+            } else {
+                return new CurrencyProperty(ctry, m.group(1),
+                        Integer.parseInt(m.group(3)),
+                        Integer.parseInt(m.group(2)), m.group(4));
+            }
+        }
+
+        /**
+         * Checks if the given list contains multiple inconsistent currency instances
+         */
+        private static boolean containsInconsistentInstances(
+                List<CurrencyProperty> list) {
+            int numCode = list.get(0).numericCode;
+            int fractionDigit = list.get(0).fraction;
+            return list.stream().anyMatch(prop -> prop.numericCode != numCode
+                    || prop.fraction != fractionDigit);
+        }
+
+        private static boolean isPastCutoverDate(String s)
+                throws ParseException {
+            SimpleDateFormat format = new SimpleDateFormat(
+                    "yyyy-MM-dd'T'HH:mm:ss", Locale.ROOT);
+            format.setTimeZone(TimeZone.getTimeZone("UTC"));
+            format.setLenient(false);
+            long time = format.parse(s.trim()).getTime();
+            return System.currentTimeMillis() > time;
+
+        }
+
+        private static void info(String message, Throwable t) {
+            PlatformLogger logger = PlatformLogger
+                    .getLogger("java.util.Currency");
+            if (logger.isLoggable(PlatformLogger.Level.INFO)) {
+                if (t != null) {
+                    logger.info(message, t);
+                } else {
+                    logger.info(message);
+                }
+            }
+        }
+
+    }
+
 }
 
 

test/jdk/java/util/Currency/PropertiesTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2007, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2007, 2018, 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
@@ -37,6 +37,8 @@ public class PropertiesTest {
             bug7102969();
         } else if (args.length == 1 && args[0].equals("bug8157138")) {
             bug8157138();
+        } else if (args.length == 1 && args[0].equals("bug8190904")) {
+            bug8190904();
         } else {
             System.err.println("Usage:  java PropertiesTest -d <dumpfile>");
             System.err.println("        java PropertiesTest -c <beforedump> <afterdump> <propsfile>");
@@ -118,8 +120,9 @@ public class PropertiesTest {
         for (String key: keys) {
             String val = p.getProperty(key);
             try {
-                if (countOccurrences(val, ',') == 3 && !isPastCutoverDate(val)) {
+                if (val.chars().map(c -> c == ',' ? 1 : 0).sum() >= 3
-                    System.out.println("Skipping since date is in future");
+                        && !isPastCutoverDate(val)) {
+                    System.out.println("Skipping " + key + " since date is in future");
                     continue; // skip since date in future (no effect)
                 }
             } catch (ParseException pe) {
@@ -130,6 +133,13 @@ public class PropertiesTest {
             System.out.printf("Testing key: %s, val: %s... ", key, val);
             System.out.println("AfterVal is : " + afterVal);
 
+            if (afterVal == null) {
+                System.out.println("Testing key " + key + " is ignored"
+                        + " because of the inconsistent numeric code and/or"
+                        + " dfd for the given currency code: "+val);
+                continue;
+            }
+
             Matcher m = propertiesPattern.matcher(val.toUpperCase(Locale.ROOT));
             if (!m.find()) {
                 // format is not recognized.
@@ -166,22 +176,24 @@ public class PropertiesTest {
             System.out.printf("Success!\n");
         }
         if (!after.isEmpty()) {
-            StringBuilder sb = new StringBuilder()
+
-                .append("Currency data replacement failed.  Unnecessary modification was(were) made for the following currencies:\n");
             keys = after.stringPropertyNames();
             for (String key : keys) {
-                sb.append("    country: ")
+                String modified = after.getProperty(key);
-                .append(key)
+                if(!p.containsValue(modified)) {
-                .append(" currency: ")
+                    throw new RuntimeException("Unnecessary modification was"
-                .append(after.getProperty(key))
+                            + " made to county: "+ key + " with currency value:"
-                .append("\n");
+                                    + " " + modified);
+                } else {
+                    System.out.println(key + " modified by an entry in"
+                            + " currency.properties with currency value "
+                            + modified);
+                }
             }
-            throw new RuntimeException(sb.toString());
         }
     }
 
     private static void bug7102969() {
-
         // check the correct overriding of special case entries
         Currency cur = Currency.getInstance(new Locale("", "JP"));
         if (!cur.getCurrencyCode().equals("ABC")) {
@@ -248,6 +260,41 @@ public class PropertiesTest {
 
     }
 
+    private static void bug8190904() {
+        // should throw IllegalArgumentException as currency code
+        // does not exist as valid ISO 4217 code and failed to load
+        // from currency.properties file because of inconsistent numeric/dfd
+        try {
+            Currency.getInstance("MCC");
+            throw new RuntimeException("[FAILED: Should throw"
+                    + " IllegalArgumentException for invalid currency code]");
+        } catch (IllegalArgumentException ex) {
+            // expected to throw IllegalArgumentException
+        }
+
+        // should keep the XOF instance as XOF,952,0, as the XOF entries in
+        // currency.properties IT=XOF,952,1, XY=XOF,955,0 are ignored because
+        // of inconsistency in numeric code and/or dfd
+        checkCurrencyInstance("XOF", 952, 0);
+        // property entry "AS=USD,841,2" should change all occurences
+        // of USD with USD,841,2
+        checkCurrencyInstance("USD", 841, 2);
+    }
+
+    /**
+     * Test the numeric code and fraction of the Currency instance obtained
+     * by given currencyCode, with the expected numericCode and fraction
+     */
+    private static void checkCurrencyInstance(String currencyCode,
+            int numericCode, int fraction) {
+        Currency cur = Currency.getInstance(currencyCode);
+        if (cur.getNumericCode() != numericCode
+                || cur.getDefaultFractionDigits() != fraction) {
+            throw new RuntimeException("[FAILED: Incorrect numeric code or"
+                    + " dfd for currency code: " + currencyCode + "]");
+        }
+    }
+
     private static boolean isPastCutoverDate(String s)
             throws IndexOutOfBoundsException, NullPointerException, ParseException {
         String dateString = s.substring(s.lastIndexOf(',')+1, s.length()).trim();
@@ -263,13 +310,4 @@ public class PropertiesTest {
         }
     }
 
-    private static int countOccurrences(String value, char match) {
-        int count = 0;
-        for (char c : value.toCharArray()) {
-            if (c == match) {
-               ++count;
-            }
-        }
-        return count;
-    }
 }

test/jdk/java/util/Currency/PropertiesTest.sh

@@ -1,6 +1,6 @@
 #!/bin/sh
 
-# Copyright (c) 2007, 2016, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2007, 2018, 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
@@ -24,7 +24,7 @@
 
 # @test
 # @bug 6332666 6863624 7180362 8003846 8074350 8074351 8130246 8149735 7102969
-#      8157138
+#      8157138 8190904
 # @summary tests the capability of replacing the currency data with user
 #     specified currency properties file
 # @build PropertiesTest
@@ -124,6 +124,11 @@ echo ''
 ${WRITABLEJDK}${FS}bin${FS}java ${TESTVMOPTS} -cp ${TESTCLASSES} PropertiesTest bug8157138
 if [ $? != 0 ]; then failures=`expr $failures + 1`; fi
 
+# run bug8190904 test
+echo ''
+${WRITABLEJDK}${FS}bin${FS}java ${TESTVMOPTS} -cp ${TESTCLASSES} PropertiesTest bug8190904
+if [ $? != 0 ]; then failures=`expr $failures + 1`; fi
+
 # Cleanup
 rm -rf $WRITABLEJDK
 

test/jdk/java/util/Currency/currency.properties

@@ -26,5 +26,9 @@ IE=euR,111,2,#testcomment
 MG=MGG,990,10
 MX=SSS,493,2,2001-01-01-00-00-00
 PE=EUR   ,978  ,2,  20399-01-01T00:00:00
-MG=MGG,990,10
 =euR,111,2, 2099-01-01-00-00-00
+MR=MCC,556,7
+IT=XOF,952,1
+XY=XOF,955,0
+AS=USD,841,2
+CY=CYP,822,2