8179222: SimpleConsoleLogger should protect against MissingResourceException
authordfuchs
Tue, 25 Apr 2017 11:54:34 +0100
changeset 44911 7cfd28612947
parent 44910 238b6aff1b9c
child 44912 d852532f406b
child 46859 36fd4dcf9df0
8179222: SimpleConsoleLogger should protect against MissingResourceException Summary: SimpleConsoleLogger now emulates the behaviour of java.util.logging.Formatter, trapping MissingResourceException and using the key as the message if the ResourceBundle has no match for that key. Reviewed-by: naoto
jdk/src/java.base/share/classes/jdk/internal/logger/SimpleConsoleLogger.java
jdk/test/java/lang/System/LoggerFinder/LoggerFinderAPI/LoggerFinderAPI.java
--- a/jdk/src/java.base/share/classes/jdk/internal/logger/SimpleConsoleLogger.java	Tue Apr 25 08:19:35 2017 +0000
+++ b/jdk/src/java.base/share/classes/jdk/internal/logger/SimpleConsoleLogger.java	Tue Apr 25 11:54:34 2017 +0100
@@ -33,6 +33,7 @@
 import java.security.PrivilegedAction;
 import java.time.ZonedDateTime;
 import java.util.Optional;
+import java.util.MissingResourceException;
 import java.util.ResourceBundle;
 import java.util.function.Function;
 import java.lang.System.Logger;
@@ -106,7 +107,7 @@
     public final void log(Level level, ResourceBundle bundle, String key, Throwable thrown) {
         if (isLoggable(level)) {
             if (bundle != null) {
-                key = bundle.getString(key);
+                key = getString(bundle, key);
             }
             publish(getCallerInfo(), logLevel(level), key, thrown);
         }
@@ -116,7 +117,7 @@
     public final void log(Level level, ResourceBundle bundle, String format, Object... params) {
         if (isLoggable(level)) {
             if (bundle != null) {
-                format = bundle.getString(format);
+                format = getString(bundle, format);
             }
             publish(getCallerInfo(), logLevel(level), format, params);
         }
@@ -364,7 +365,7 @@
     public final void logrb(PlatformLogger.Level level, String sourceClass,
             String sourceMethod, ResourceBundle bundle, String key, Object... params) {
         if (isLoggable(level)) {
-            String msg = bundle == null ? key : bundle.getString(key);
+            String msg = bundle == null ? key : getString(bundle, key);
             publish(getCallerInfo(sourceClass, sourceMethod), logLevel(level), msg, params);
         }
     }
@@ -373,7 +374,7 @@
     public final void logrb(PlatformLogger.Level level, String sourceClass,
             String sourceMethod, ResourceBundle bundle, String key, Throwable thrown) {
         if (isLoggable(level)) {
-            String msg = bundle == null ? key : bundle.getString(key);
+            String msg = bundle == null ? key : getString(bundle, key);
             publish(getCallerInfo(sourceClass, sourceMethod), logLevel(level), msg, thrown);
         }
     }
@@ -382,7 +383,7 @@
     public final void logrb(PlatformLogger.Level level, ResourceBundle bundle,
             String key, Object... params) {
         if (isLoggable(level)) {
-            String msg = bundle == null ? key : bundle.getString(key);
+            String msg = bundle == null ? key : getString(bundle,key);
             publish(getCallerInfo(), logLevel(level), msg, params);
         }
     }
@@ -391,11 +392,23 @@
     public final void logrb(PlatformLogger.Level level, ResourceBundle bundle,
             String key, Throwable thrown) {
         if (isLoggable(level)) {
-            String msg = bundle == null ? key : bundle.getString(key);
+            String msg = bundle == null ? key : getString(bundle,key);
             publish(getCallerInfo(), logLevel(level), msg, thrown);
         }
     }
 
+    static String getString(ResourceBundle bundle, String key) {
+        if (bundle == null || key == null) return key;
+        try {
+            return bundle.getString(key);
+        } catch (MissingResourceException x) {
+            // Emulate what java.util.logging Formatters do
+            // We don't want unchecked exception to propagate up to
+            // the caller's code.
+            return key;
+        }
+    }
+
     static final class Formatting {
         // The default simple log format string.
         // Used both by SimpleConsoleLogger when java.logging is not present,
--- a/jdk/test/java/lang/System/LoggerFinder/LoggerFinderAPI/LoggerFinderAPI.java	Tue Apr 25 08:19:35 2017 +0000
+++ b/jdk/test/java/lang/System/LoggerFinder/LoggerFinderAPI/LoggerFinderAPI.java	Tue Apr 25 11:54:34 2017 +0100
@@ -28,17 +28,18 @@
 import java.lang.System.Logger;
 import java.lang.System.Logger.Level;
 import java.lang.System.LoggerFinder;
+import java.util.Collections;
 import java.util.Enumeration;
 import java.util.Locale;
+import java.util.Map;
 import java.util.Objects;
 import java.util.ResourceBundle;
-import java.util.concurrent.ConcurrentHashMap;
 // Can't use testng because testng requires java.logging
 //import org.testng.annotations.Test;
 
 /**
  * @test
- * @bug 8177835
+ * @bug 8177835 8179222
  * @summary Checks that the DefaultLoggerFinder and LoggingProviderImpl
  *          implementations of the System.LoggerFinder conform to the
  *          LoggerFinder specification, in particular with respect to
@@ -60,6 +61,8 @@
     static final String JDK_FORMAT_PROP_KEY = "jdk.system.logger.format";
     static final String JUL_FORMAT_PROP_KEY =
         "java.util.logging.SimpleFormatter.format";
+    static final String MESSAGE = "{0} with {1}: PASSED";
+    static final String LOCALIZED = "[localized] ";
 
     static class RecordStream extends OutputStream {
         static final Object LOCK = new Object[0];
@@ -106,15 +109,28 @@
     }
 
     public static class MyResourceBundle extends ResourceBundle {
-        final ConcurrentHashMap<String, String> map = new ConcurrentHashMap<>();
+        final Map<String, String> map = Map.of(MESSAGE, LOCALIZED + MESSAGE);
         @Override
         protected Object handleGetObject(String string) {
-            return map.computeIfAbsent(string, s -> "[localized] " + s);
+            return map.get(string);
         }
 
         @Override
         public Enumeration<String> getKeys() {
-            return map.keys();
+            return Collections.enumeration(map.keySet());
+        }
+
+    }
+
+    public static class EmptyResourceBundle extends ResourceBundle {
+        @Override
+        protected Object handleGetObject(String string) {
+            return null;
+        }
+
+        @Override
+        public Enumeration<String> getKeys() {
+            return Collections.emptyEnumeration();
         }
 
     }
@@ -129,17 +145,23 @@
 
         LoggerFinderAPI apiTest = new LoggerFinderAPI();
         for (Object[] params : getLoggerDataProvider()) {
+            @SuppressWarnings("unchecked")
+            Class<? extends Throwable> throwableClass  =
+                    Throwable.class.getClass().cast(params[3]);
             apiTest.testGetLogger((String)params[0],
                                   (String)params[1],
                                   (Module)params[2],
-                                  Throwable.class.getClass().cast(params[3]));
+                                  throwableClass);
         }
         for (Object[] params : getLocalizedLoggerDataProvider()) {
+            @SuppressWarnings("unchecked")
+            Class<? extends Throwable> throwableClass =
+                    Throwable.class.getClass().cast(params[4]);
             apiTest.testGetLocalizedLogger((String)params[0],
                                   (String)params[1],
                                   (ResourceBundle)params[2],
                                   (Module)params[3],
-                                  Throwable.class.getClass().cast(params[4]));
+                                  throwableClass);
         }
     }
 
@@ -158,15 +180,16 @@
             // Make sure we don't fail if tests are run in parallel
             synchronized(RecordStream.LOCK) {
                 LOG_STREAM.startRecording();
+                byte[] logged = null;
                 try {
                     logger.log(Level.INFO, "{0} with {1}: PASSED",
                                "LoggerFinder.getLogger",
                                desc);
                 } finally {
-                    byte[] logged = LOG_STREAM.stopRecording();
-                    check(logged, "testGetLogger", desc, null,
-                          "LoggerFinder.getLogger");
+                    logged = LOG_STREAM.stopRecording();
                 }
+                check(logged, "testGetLogger", desc, null,
+                      "LoggerFinder.getLogger");
             }
         } catch (Throwable x) {
             if (thrown != null && thrown.isInstance(x)) {
@@ -192,15 +215,16 @@
             // Make sure we don't fail if tests are run in parallel
             synchronized(RecordStream.LOCK) {
                 LOG_STREAM.startRecording();
+                byte[] logged = null;
                 try {
-                    logger.log(Level.INFO, "{0} with {1}: PASSED",
+                    logger.log(Level.INFO, MESSAGE,
                               "LoggerFinder.getLocalizedLogger",
                               desc);
                 } finally {
-                    byte[] logged = LOG_STREAM.stopRecording();
-                    check(logged, "testGetLocalizedLogger", desc, bundle,
-                          "LoggerFinder.getLocalizedLogger");
+                   logged = LOG_STREAM.stopRecording();
                 }
+                check(logged, "testGetLocalizedLogger", desc, bundle,
+                      "LoggerFinder.getLocalizedLogger");
             }
         } catch (Throwable x) {
             if (thrown != null && thrown.isInstance(x)) {
@@ -213,9 +237,11 @@
     private void check(byte[] logged, String test, String desc,
                        ResourceBundle bundle, String meth) {
         String msg = new String(logged);
+        String localizedPrefix =
+                ((bundle==null || bundle==EMPTY_BUNDLE)?"":LOCALIZED);
         String expected = String.format(TEST_FORMAT, null,
                 "LoggerFinderAPI " + test, null, Level.INFO.name(),
-                (bundle==null?"":"[localized] ") + meth + " with " + desc + ": PASSED",
+                localizedPrefix + meth + " with " + desc + ": PASSED",
                 "");
         if (!Objects.equals(msg, expected)) {
             throw new AssertionError("Expected log message not found: "
@@ -227,6 +253,7 @@
 
     static final Module MODULE = LoggerFinderAPI.class.getModule();
     static final ResourceBundle BUNDLE = new MyResourceBundle();
+    static final ResourceBundle EMPTY_BUNDLE = new EmptyResourceBundle();
     static final Object[][] GET_LOGGER = {
         {"null name", null, MODULE , NullPointerException.class},
         {"null module", "foo", null, NullPointerException.class},
@@ -236,12 +263,15 @@
     static final Object[][] GET_LOCALIZED_LOGGER = {
         {"null name", null, BUNDLE, MODULE , NullPointerException.class},
         {"null module", "foo", BUNDLE, null, NullPointerException.class},
-        {"null name and module", null, BUNDLE, null, NullPointerException.class},
-        {"non null name and module", "foo", BUNDLE, MODULE, null},
+        {"null name and module, non null bundle", null, BUNDLE, null, NullPointerException.class},
+        {"non null name, module, and bundle", "foo", BUNDLE, MODULE, null},
         {"null name and bundle", null, null, MODULE , NullPointerException.class},
         {"null module and bundle", "foo", null, null, NullPointerException.class},
         {"null name and module and bundle", null, null, null, NullPointerException.class},
         {"non null name and module, null bundle", "foo", null, MODULE, null},
+        // tests that MissingResourceBundle is not propagated to the caller of
+        // logger.log() if the key is not found in the resource bundle
+        {"non null name, module, and empty bundle", "foo", EMPTY_BUNDLE, MODULE, null},
     };
     public static Object[][] getLoggerDataProvider() {
         return GET_LOGGER;