Add test and use older API

Turns out Android doesn't support Java 8 all that much.

Author: jvz

log4j-api-test/src/test/java/org/apache/logging/log4j/util/LoaderUtilSecurityManagerTest.java

@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.logging.log4j.util;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+
+import java.security.Permission;
+import org.apache.logging.log4j.test.junit.SecurityManagerTestRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.jupiter.api.parallel.ResourceLock;
+
+@ResourceLock("java.lang.SecurityManager")
+public class LoaderUtilSecurityManagerTest {
+    @Rule
+    public final SecurityManagerTestRule rule = new SecurityManagerTestRule(new TestSecurityManager());
+
+    private static class TestSecurityManager extends SecurityManager {
+        @Override
+        public void checkPermission(final Permission perm) {
+            if (perm.equals(LoaderUtil.GET_CLASS_LOADER)) {
+                throw new SecurityException("disabled");
+            }
+        }
+    }
+
+    @Test
+    public void canGetClassLoaderThroughPrivileges() {
+        assertFalse(LoaderUtil.GET_CLASS_LOADER_DISABLED);
+        assertDoesNotThrow(() -> LoaderUtil.getClassLoader(LoaderUtilSecurityManagerTest.class, String.class));
+    }
+}

log4j-api/src/main/java/org/apache/logging/log4j/util/LoaderUtil.java

@@ -50,36 +50,31 @@ public final class LoaderUtil {
     // wants to use PropertiesUtil, but then PropertiesUtil wants to use LoaderUtil.
     private static Boolean ignoreTCCL;
 
-    private static final RuntimePermission GET_CLASS_LOADER = new RuntimePermission("getClassLoader");
-    private static final boolean GET_CLASS_LOADER_DISABLED;
-
-    private static final PrivilegedAction<ClassLoader> TCCL_GETTER = new ThreadContextClassLoaderGetter();
-
-    static {
-        if (System.getSecurityManager() != null) {
-            boolean getClassLoaderDisabled;
+    static final RuntimePermission GET_CLASS_LOADER = new RuntimePermission("getClassLoader");
+    static final LazyBoolean GET_CLASS_LOADER_DISABLED = new LazyBoolean(() -> {
+        if (System.getSecurityManager() == null) {
+            return false;
+        }
+        try {
+            AccessController.checkPermission(GET_CLASS_LOADER);
+            // seems like we'll be ok
+            return false;
+        } catch (final SecurityException ignored) {
             try {
-                AccessController.checkPermission(GET_CLASS_LOADER);
-                // seems like we'll be ok
-                getClassLoaderDisabled = false;
-            } catch (final SecurityException ignored) {
-                try {
-                    // let's see if we can obtain that permission
-                    runPrivilegedActionWithGetClassLoaderPermission((PrivilegedAction<Void>) () -> {
-                        AccessController.checkPermission(GET_CLASS_LOADER);
-                        return null;
-                    });
-                    getClassLoaderDisabled = false;
-                } catch (final SecurityException ignore) {
-                    // no chance
-                    getClassLoaderDisabled = true;
-                }
+                // let's see if we can obtain that permission
+                AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
+                    AccessController.checkPermission(GET_CLASS_LOADER);
+                    return null;
+                });
+                return false;
+            } catch (final SecurityException ignore) {
+                // no chance
+                return true;
             }
-            GET_CLASS_LOADER_DISABLED = getClassLoaderDisabled;
-        } else {
-            GET_CLASS_LOADER_DISABLED = false;
         }
-    }
+    });
+
+    private static final PrivilegedAction<ClassLoader> TCCL_GETTER = new ThreadContextClassLoaderGetter();
 
     private LoaderUtil() {}
 
@@ -97,15 +92,15 @@ public static ClassLoader getClassLoader(final Class<?> class1, final Class<?> c
         PrivilegedAction<ClassLoader> action = () -> {
             final ClassLoader loader1 = class1 == null ? null : class1.getClassLoader();
             final ClassLoader loader2 = class2 == null ? null : class2.getClassLoader();
-            final ClassLoader referenceLoader = GET_CLASS_LOADER_DISABLED
+            final ClassLoader referenceLoader = GET_CLASS_LOADER_DISABLED.getAsBoolean()
                     ? getThisClassLoader()
                     : Thread.currentThread().getContextClassLoader();
             if (isChild(referenceLoader, loader1)) {
                 return isChild(referenceLoader, loader2) ? referenceLoader : loader2;
             }
             return isChild(loader1, loader2) ? loader1 : loader2;
         };
-        return runActionInvolvingGetClassLoaderPermission(action);
+        return runPrivileged(action);
     }
 
     /**
@@ -141,9 +136,7 @@ private static boolean isChild(final ClassLoader loader1, final ClassLoader load
      */
     public static ClassLoader getThreadContextClassLoader() {
         try {
-            return GET_CLASS_LOADER_DISABLED
-                    ? getThisClassLoader()
-                    : runActionInvolvingGetClassLoaderPermission(TCCL_GETTER);
+            return GET_CLASS_LOADER_DISABLED.getAsBoolean() ? getThisClassLoader() : runPrivileged(TCCL_GETTER);
         } catch (final SecurityException ignored) {
             return null;
         }
@@ -153,14 +146,8 @@ private static ClassLoader getThisClassLoader() {
         return LoaderUtil.class.getClassLoader();
     }
 
-    private static <T> T runActionInvolvingGetClassLoaderPermission(final PrivilegedAction<T> action) {
-        return System.getSecurityManager() != null
-                ? runPrivilegedActionWithGetClassLoaderPermission(action)
-                : action.run();
-    }
-
-    private static <T> T runPrivilegedActionWithGetClassLoaderPermission(final PrivilegedAction<T> action) {
-        return AccessController.doPrivileged(action, null, GET_CLASS_LOADER);
+    private static <T> T runPrivileged(final PrivilegedAction<T> action) {
+        return System.getSecurityManager() != null ? AccessController.doPrivileged(action) : action.run();
     }
 
     private static class ThreadContextClassLoaderGetter implements PrivilegedAction<ClassLoader> {
@@ -171,7 +158,7 @@ public ClassLoader run() {
                 return contextClassLoader;
             }
             final ClassLoader thisClassLoader = getThisClassLoader();
-            if (thisClassLoader != null || GET_CLASS_LOADER_DISABLED) {
+            if (thisClassLoader != null || GET_CLASS_LOADER_DISABLED.getAsBoolean()) {
                 return thisClassLoader;
             }
             return ClassLoader.getSystemClassLoader();
@@ -476,7 +463,7 @@ static Collection<UrlResource> findUrlResources(final String resource, final boo
         final ClassLoader[] candidates = {
             useTccl ? getThreadContextClassLoader() : null,
             LoaderUtil.class.getClassLoader(),
-            GET_CLASS_LOADER_DISABLED ? null : ClassLoader.getSystemClassLoader()
+            GET_CLASS_LOADER_DISABLED.getAsBoolean() ? null : ClassLoader.getSystemClassLoader()
         };
         // @formatter:on
         final Collection<UrlResource> resources = new LinkedHashSet<>();

src/site/_release-notes/_2.x.x.adoc

@@ -38,6 +38,11 @@ This releases contains ...
 
 * Deprecated the `RingBufferLogEventHandler` class for removal from the public API in 3.x.
 
+[#release-notes-2-x-x-fixed]
+=== Fixed
+
+* Fixed use of `SecurityManager` in `LoaderUtil` where `AccessController::doPrivileged` should only be invoked when a `SecurityManager` is installed. Some runtimes do not seem to have this method available. (https://github.com/apache/logging-log4j2/issues/2129[2129])
+
 [#release-notes-2-x-x-updated]
 === Updated