8189264: (sl) ServiceLoader does not wrap Errors thrown by provider classes when running with a security manager
Reviewed-by: mchung
--- a/src/java.base/share/classes/java/util/ServiceLoader.java Fri Oct 13 18:34:37 2017 -0700
+++ b/src/java.base/share/classes/java/util/ServiceLoader.java Sat Oct 14 09:51:25 2017 +0100
@@ -747,8 +747,10 @@
// invoke factory method with permissions restricted by acc
try {
result = AccessController.doPrivileged(pa, acc);
- } catch (PrivilegedActionException pae) {
- exc = pae.getCause();
+ } catch (Throwable x) {
+ if (x instanceof PrivilegedActionException)
+ x = x.getCause();
+ exc = x;
}
}
if (exc != null) {
@@ -788,8 +790,10 @@
// invoke constructor with permissions restricted by acc
try {
p = AccessController.doPrivileged(pa, acc);
- } catch (PrivilegedActionException pae) {
- exc = pae.getCause();
+ } catch (Throwable x) {
+ if (x instanceof PrivilegedActionException)
+ x = x.getCause();
+ exc = x;
}
}
if (exc != null) {
@@ -852,8 +856,9 @@
PrivilegedExceptionAction<Class<?>> pa = () -> Class.forName(module, cn);
try {
clazz = AccessController.doPrivileged(pa);
- } catch (PrivilegedActionException pae) {
- Throwable x = pae.getCause();
+ } catch (Throwable x) {
+ if (x instanceof PrivilegedActionException)
+ x = x.getCause();
fail(service, "Unable to load " + cn, x);
return null;
}
--- a/src/java.base/share/classes/sun/util/cldr/CLDRLocaleProviderAdapter.java Fri Oct 13 18:34:37 2017 -0700
+++ b/src/java.base/share/classes/sun/util/cldr/CLDRLocaleProviderAdapter.java Sat Oct 14 09:51:25 2017 +0100
@@ -26,6 +26,7 @@
package sun.util.cldr;
import java.security.AccessController;
+import java.security.AccessControlException;
import java.security.PrivilegedExceptionAction;
import java.text.spi.BreakIteratorProvider;
import java.text.spi.CollatorProvider;
@@ -37,6 +38,7 @@
import java.util.Map;
import java.util.Objects;
import java.util.ServiceLoader;
+import java.util.ServiceConfigurationError;
import java.util.Set;
import java.util.StringTokenizer;
import java.util.concurrent.ConcurrentHashMap;
@@ -81,8 +83,11 @@
return null;
}
});
- } catch (Exception e) {
+ } catch (Exception e) {
// Catch any exception, and continue as if only CLDR's base locales exist.
+ } catch (ServiceConfigurationError sce) {
+ Throwable cause = sce.getCause();
+ if (!(cause instanceof AccessControlException)) throw sce;
}
nonBaseMetaInfo = nbmi;
--- a/src/java.base/share/classes/sun/util/locale/provider/JRELocaleProviderAdapter.java Fri Oct 13 18:34:37 2017 -0700
+++ b/src/java.base/share/classes/sun/util/locale/provider/JRELocaleProviderAdapter.java Sat Oct 14 09:51:25 2017 +0100
@@ -26,6 +26,7 @@
package sun.util.locale.provider;
import java.security.AccessController;
+import java.security.AccessControlException;
import java.security.PrivilegedAction;
import java.security.PrivilegedExceptionAction;
import java.text.spi.BreakIteratorProvider;
@@ -40,6 +41,7 @@
import java.util.Locale;
import java.util.ResourceBundle;
import java.util.ServiceLoader;
+import java.util.ServiceConfigurationError;
import java.util.Set;
import java.util.StringTokenizer;
import java.util.concurrent.ConcurrentHashMap;
@@ -476,8 +478,11 @@
if (nonBaseTags != null) {
supportedLocaleString += " " + nonBaseTags;
}
- } catch (Exception e) {
+ } catch (Exception e) {
// catch any exception, and ignore them as if non-EN locales do not exist.
+ } catch (ServiceConfigurationError sce) {
+ Throwable cause = sce.getCause();
+ if (!(cause instanceof AccessControlException)) throw sce;
}
return supportedLocaleString;
--- a/test/jdk/java/util/ServiceLoader/security/test/module-info.java Fri Oct 13 18:34:37 2017 -0700
+++ b/test/jdk/java/util/ServiceLoader/security/test/module-info.java Sat Oct 14 09:51:25 2017 +0100
@@ -26,8 +26,16 @@
module test {
uses S1;
uses S2;
+ uses S3;
+ uses S4;
+ uses S5;
+ uses S6;
provides S1 with P1;
provides S2 with P2;
+ provides S3 with P3;
+ provides S4 with P4;
+ provides S5 with P5;
+ provides S6 with P6;
requires testng;
exports p to testng;
}
--- a/test/jdk/java/util/ServiceLoader/security/test/p/Tests.java Fri Oct 13 18:34:37 2017 -0700
+++ b/test/jdk/java/util/ServiceLoader/security/test/p/Tests.java Sat Oct 14 09:51:25 2017 +0100
@@ -38,14 +38,16 @@
import java.util.ServiceLoader.Provider;
import static java.security.AccessController.doPrivileged;
+import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
import org.testng.annotations.BeforeTest;
import static org.testng.Assert.*;
/**
- * Basic tests with a security manager to ensure that the provider code
- * is run with permissions restricted by whatever created the ServiceLoader
- * object.
+ * Tests ServiceLoader when running with a security manager, specifically
+ * tests to ensure that provider code is run with permissions restricted by
+ * the creater of ServiceLoader, and also testing of exceptions thrown
+ * when loading or initializing a provider.
*/
public class Tests {
@@ -163,11 +165,35 @@
}
}
+ @DataProvider(name = "failingServices")
+ public Object[][] failingServices() {
+ return new Object[][] {
+ { S3.class, P3.Error3.class },
+ { S4.class, P4.Error4.class },
+ { S5.class, P5.Error5.class },
+ { S6.class, P6.Error6.class },
+ };
+ }
+
+ @Test(dataProvider = "failingServices")
+ public void testFailingService(Class<?> service, Class<? extends Error> errorClass) {
+ ServiceLoader<?> sl = ServiceLoader.load(service);
+ try {
+ sl.iterator().next();
+ assertTrue(false);
+ } catch (ServiceConfigurationError e) {
+ assertTrue(e.getCause().getClass() == errorClass);
+ }
+ }
// service types and implementations
public static interface S1 { }
public static interface S2 { }
+ public static interface S3 { }
+ public static interface S4 { }
+ public static interface S5 { }
+ public static interface S6 { }
public static class P1 implements S1 {
public P1() {
@@ -182,4 +208,36 @@
return new P2();
}
}
+
+ public static class P3 implements S3 {
+ static class Error3 extends Error { }
+ static {
+ if (1==1) throw new Error3(); // fail
+ }
+ public P3() { }
+ }
+
+ public static class P4 implements S4 {
+ static class Error4 extends Error { }
+ static {
+ if (1==1) throw new Error4(); // fail
+ }
+ public static S4 provider() {
+ return new P4();
+ }
+ }
+
+ public static class P5 implements S5 {
+ static class Error5 extends Error { }
+ public P5() {
+ throw new Error5(); // fail
+ }
+ }
+
+ public static class P6 implements S6 {
+ static class Error6 extends Error { }
+ public static S6 provider() {
+ throw new Error6(); // fail
+ }
+ }
}