1.x: Plugin lookup workaround for System.properties access restrictions (#5820)

* 1.x: Plugin lookup workaround for System.properties access restrictions

* System.getProperties() can also throw SecurityException

Author: akarnokd

src/main/java/rx/plugins/RxJavaPlugins.java

@@ -105,7 +105,7 @@ public void reset() {
     public RxJavaErrorHandler getErrorHandler() {
         if (errorHandler.get() == null) {
             // check for an implementation from System.getProperty first
-            Object impl = getPluginImplementationViaProperty(RxJavaErrorHandler.class, System.getProperties());
+            Object impl = getPluginImplementationViaProperty(RxJavaErrorHandler.class, getSystemPropertiesSafe());
             if (impl == null) {
                 // nothing set via properties so initialize with default
                 errorHandler.compareAndSet(null, DEFAULT_ERROR_HANDLER);
@@ -147,7 +147,7 @@ public void registerErrorHandler(RxJavaErrorHandler impl) {
     public RxJavaObservableExecutionHook getObservableExecutionHook() {
         if (observableExecutionHook.get() == null) {
             // check for an implementation from System.getProperty first
-            Object impl = getPluginImplementationViaProperty(RxJavaObservableExecutionHook.class, System.getProperties());
+            Object impl = getPluginImplementationViaProperty(RxJavaObservableExecutionHook.class, getSystemPropertiesSafe());
             if (impl == null) {
                 // nothing set via properties so initialize with default
                 observableExecutionHook.compareAndSet(null, RxJavaObservableExecutionHookDefault.getInstance());
@@ -189,7 +189,7 @@ public void registerObservableExecutionHook(RxJavaObservableExecutionHook impl)
     public RxJavaSingleExecutionHook getSingleExecutionHook() {
         if (singleExecutionHook.get() == null) {
             // check for an implementation from System.getProperty first
-            Object impl = getPluginImplementationViaProperty(RxJavaSingleExecutionHook.class, System.getProperties());
+            Object impl = getPluginImplementationViaProperty(RxJavaSingleExecutionHook.class, getSystemPropertiesSafe());
             if (impl == null) {
                 // nothing set via properties so initialize with default
                 singleExecutionHook.compareAndSet(null, RxJavaSingleExecutionHookDefault.getInstance());
@@ -232,7 +232,7 @@ public void registerSingleExecutionHook(RxJavaSingleExecutionHook impl) {
     public RxJavaCompletableExecutionHook getCompletableExecutionHook() {
         if (completableExecutionHook.get() == null) {
             // check for an implementation from System.getProperty first
-            Object impl = getPluginImplementationViaProperty(RxJavaCompletableExecutionHook.class, System.getProperties());
+            Object impl = getPluginImplementationViaProperty(RxJavaCompletableExecutionHook.class, getSystemPropertiesSafe());
             if (impl == null) {
                 // nothing set via properties so initialize with default
                 completableExecutionHook.compareAndSet(null, new RxJavaCompletableExecutionHook() { });
@@ -262,6 +262,19 @@ public void registerCompletableExecutionHook(RxJavaCompletableExecutionHook impl
         }
     }
 
+    /**
+     * A security manager may prevent accessing the System properties entirely,
+     * therefore, the SecurityException is turned into an empty properties.
+     * @return the Properties to use for looking up settings
+     */
+    /* test */ static Properties getSystemPropertiesSafe() {
+        try {
+            return System.getProperties();
+        } catch (SecurityException ex) {
+            return new Properties();
+        }
+    }
+
     /* test */ static Object getPluginImplementationViaProperty(Class<?> pluginClass, Properties propsIn) {
         // Make a defensive clone because traversal may fail with ConcurrentModificationException
         // if the properties get changed by something outside RxJava.
@@ -284,25 +297,32 @@ public void registerCompletableExecutionHook(RxJavaCompletableExecutionHook impl
             String classSuffix = ".class";
             String implSuffix = ".impl";
 
-            for (Map.Entry<Object, Object> e : props.entrySet()) {
-                String key = e.getKey().toString();
-                if (key.startsWith(pluginPrefix) && key.endsWith(classSuffix)) {
-                    String value = e.getValue().toString();
+            try {
+                for (Map.Entry<Object, Object> e : props.entrySet()) {
+                    String key = e.getKey().toString();
+                    if (key.startsWith(pluginPrefix) && key.endsWith(classSuffix)) {
+                        String value = e.getValue().toString();
+
+                        if (classSimpleName.equals(value)) {
+                            String index = key.substring(0, key.length() - classSuffix.length()).substring(pluginPrefix.length());
 
-                    if (classSimpleName.equals(value)) {
-                        String index = key.substring(0, key.length() - classSuffix.length()).substring(pluginPrefix.length());
+                            String implKey = pluginPrefix + index + implSuffix;
 
-                        String implKey = pluginPrefix + index + implSuffix;
+                            implementingClass = props.getProperty(implKey);
 
-                        implementingClass = props.getProperty(implKey);
+                            if (implementingClass == null) {
+                                throw new IllegalStateException("Implementing class declaration for " + classSimpleName + " missing: " + implKey);
+                            }
 
-                        if (implementingClass == null) {
-                            throw new IllegalStateException("Implementing class declaration for " + classSimpleName + " missing: " + implKey);
+                            break;
                         }
-
-                        break;
                     }
                 }
+            } catch (SecurityException ex) {
+                // https://github.com/ReactiveX/RxJava/issues/5819
+                // We don't seem to have access to all properties.
+                // At least print the exception to the console.
+                ex.printStackTrace();
             }
         }
 
@@ -339,7 +359,7 @@ public void registerCompletableExecutionHook(RxJavaCompletableExecutionHook impl
     public RxJavaSchedulersHook getSchedulersHook() {
         if (schedulersHook.get() == null) {
             // check for an implementation from System.getProperty first
-            Object impl = getPluginImplementationViaProperty(RxJavaSchedulersHook.class, System.getProperties());
+            Object impl = getPluginImplementationViaProperty(RxJavaSchedulersHook.class, getSystemPropertiesSafe());
             if (impl == null) {
                 // nothing set via properties so initialize with default
                 schedulersHook.compareAndSet(null, RxJavaSchedulersHook.getDefaultInstance());

src/test/java/rx/plugins/RxJavaPluginsTest.java

@@ -18,6 +18,7 @@
 import static org.junit.Assert.*;
 import static org.mockito.Mockito.mock;
 
+import java.security.Permission;
 import java.util.*;
 import java.util.concurrent.TimeUnit;
 
@@ -344,4 +345,75 @@ public void onNext(Object o) {
         assertEquals(re, errorHandler.e);
         assertEquals(1, errorHandler.count);
     }
+
+    @Test
+    public void systemPropertiesSecurityException() {
+        assertNull(RxJavaPlugins.getPluginImplementationViaProperty(Object.class, new Properties() {
+
+            private static final long serialVersionUID = -4291534158508255616L;
+
+            @Override
+            public Set<java.util.Map.Entry<Object, Object>> entrySet() {
+                return new HashSet<java.util.Map.Entry<Object, Object>>() {
+
+                    private static final long serialVersionUID = -7714005655772619143L;
+
+                    @Override
+                    public Iterator<java.util.Map.Entry<Object, Object>> iterator() {
+                        return new Iterator<java.util.Map.Entry<Object, Object>>() {
+                            @Override
+                            public boolean hasNext() {
+                                return true;
+                            }
+
+                            @Override
+                            public Map.Entry<Object,Object> next() {
+                                throw new SecurityException();
+                            };
+
+                            @Override
+                            public void remove() {
+                                throw new UnsupportedOperationException();
+                            }
+                        };
+                    }
+                };
+            }
+
+            @Override
+            public synchronized Object clone() {
+                return this;
+            }
+        }));
+    }
+
+    @Test
+    public void securityManagerDenySystemProperties() {
+        SecurityManager old = System.getSecurityManager();
+        try {
+            SecurityManager sm = new SecurityManager() {
+                @Override
+                public void checkPropertiesAccess() {
+                    throw new SecurityException();
+                }
+
+                @Override
+                public void checkPermission(Permission perm) {
+                    // allow restoring the security manager
+                }
+
+                @Override
+                public void checkPermission(Permission perm, Object context) {
+                    // allow restoring the security manager
+                }
+            };
+
+            System.setSecurityManager(sm);
+            assertTrue(RxJavaPlugins.getSystemPropertiesSafe().isEmpty());
+        } finally {
+            System.setSecurityManager(old);
+        }
+
+        assertFalse(RxJavaPlugins.getSystemPropertiesSafe().isEmpty());
+    }
 }