C0urante commented on code in PR #16522: URL: https://github.com/apache/kafka/pull/16522#discussion_r1704300039
########## clients/src/main/java/org/apache/kafka/common/internals/SecurityManagerCompatibility.java: ########## @@ -0,0 +1,355 @@ +/* + * 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.kafka.common.internals; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.security.PrivilegedAction; +import java.security.PrivilegedActionException; +import java.security.PrivilegedExceptionAction; +import java.util.Objects; +import java.util.concurrent.Callable; +import java.util.concurrent.CompletionException; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; + +import javax.security.auth.Subject; + +/** + * This is a compatibility class to provide dual-support for JREs with and without SecurityManager support. + * <p>Users should call {@link #get()} to retrieve a singleton instance, and call instance methods + * {@link #doPrivileged(PrivilegedAction)}, {@link #current()}, and {@link #callAs(Subject, Callable)}. + * <p>This class's motivation and expected behavior is defined in + * <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-1006%3A+Remove+SecurityManager+Support";>KIP-1006</a> + */ +public interface SecurityManagerCompatibility { + + static SecurityManagerCompatibility get() { + return Composite.INSTANCE; + } + + <T> T doPrivileged(PrivilegedAction<T> action); + + Subject current(); + + <T> T callAs(Subject subject, Callable<T> action) throws CompletionException; + + interface Loader { + Class<?> loadClass(String className) throws ClassNotFoundException; + + static Loader forName() { + return className -> Class.forName(className, true, Loader.class.getClassLoader()); + } + } + + /** + * This class implements reflective access to the deprecated-for-removal methods of AccessController and Subject. + * <p>Instantiating this class may fail if any of the required classes or methods are not found. + * Method invocations for this class may fail with {@link UnsupportedOperationException} if all methods are found, + * but the operation is not permitted to be invoked. + * <p>This class is expected to be instantiable in JRE >=8 until the removal finally takes place. + */ + @SuppressWarnings("unchecked") + class Legacy implements SecurityManagerCompatibility { Review Comment: It'd be nice if the various implementations were all top-level classes in their own files; makes the interface easier to grok the first time through. Maybe all of this logic can be encapsulated in its own Java package with the only public members being the `SecurityManagerCompatibility` interface, its methods, and its `Loader` subinterface? ########## clients/src/main/java/org/apache/kafka/common/internals/SecurityManagerCompatibility.java: ########## @@ -0,0 +1,355 @@ +/* + * 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.kafka.common.internals; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.security.PrivilegedAction; +import java.security.PrivilegedActionException; +import java.security.PrivilegedExceptionAction; +import java.util.Objects; +import java.util.concurrent.Callable; +import java.util.concurrent.CompletionException; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; + +import javax.security.auth.Subject; + +/** + * This is a compatibility class to provide dual-support for JREs with and without SecurityManager support. + * <p>Users should call {@link #get()} to retrieve a singleton instance, and call instance methods + * {@link #doPrivileged(PrivilegedAction)}, {@link #current()}, and {@link #callAs(Subject, Callable)}. + * <p>This class's motivation and expected behavior is defined in + * <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-1006%3A+Remove+SecurityManager+Support";>KIP-1006</a> + */ +public interface SecurityManagerCompatibility { + + static SecurityManagerCompatibility get() { + return Composite.INSTANCE; + } + + <T> T doPrivileged(PrivilegedAction<T> action); + + Subject current(); + + <T> T callAs(Subject subject, Callable<T> action) throws CompletionException; + + interface Loader { + Class<?> loadClass(String className) throws ClassNotFoundException; + + static Loader forName() { + return className -> Class.forName(className, true, Loader.class.getClassLoader()); + } + } + + /** + * This class implements reflective access to the deprecated-for-removal methods of AccessController and Subject. + * <p>Instantiating this class may fail if any of the required classes or methods are not found. + * Method invocations for this class may fail with {@link UnsupportedOperationException} if all methods are found, + * but the operation is not permitted to be invoked. + * <p>This class is expected to be instantiable in JRE >=8 until the removal finally takes place. + */ + @SuppressWarnings("unchecked") + class Legacy implements SecurityManagerCompatibility { + + private final Method doPrivileged; + private final Method getContext; + private final Method getSubject; + private final Method doAs; + + // Visible for testing + Legacy(Loader loader) throws ClassNotFoundException, NoSuchMethodException { + Class<?> accessController = loader.loadClass("java.security.AccessController"); + doPrivileged = accessController.getDeclaredMethod("doPrivileged", PrivilegedAction.class); + getContext = accessController.getDeclaredMethod("getContext"); + Class<?> accessControlContext = loader.loadClass("java.security.AccessControlContext"); + Class<?> subject = loader.loadClass(Subject.class.getName()); + getSubject = subject.getDeclaredMethod("getSubject", accessControlContext); + // Note that we use the real Subject.class rather than the `subject` variable + // This allows for mocking out the method without changing the argument types. + doAs = subject.getDeclaredMethod("doAs", Subject.class, PrivilegedExceptionAction.class); + } + + @Override + public <T> T doPrivileged(PrivilegedAction<T> action) { + try { + return (T) doPrivileged.invoke(null, action); + } catch (IllegalAccessException e) { + throw new UnsupportedOperationException(e); + } catch (InvocationTargetException e) { + Throwable cause = e.getCause(); + if (cause instanceof RuntimeException) { + throw (RuntimeException) cause; + } else { + throw new RuntimeException(cause); + } + } + } + + /** + * @return the result of AccessController.getContext(), of type AccessControlContext + */ + private Object getContext() { + try { + return getContext.invoke(null); + } catch (IllegalAccessException e) { + throw new UnsupportedOperationException(e); + } catch (InvocationTargetException e) { + Throwable cause = e.getCause(); + if (cause instanceof RuntimeException) { + throw (RuntimeException) cause; + } else { + throw new RuntimeException(cause); + } + } + } + + /** + * @param context The current AccessControlContext + * @return The result of Subject.getSubject(AccessControlContext) + */ + private Subject getSubject(Object context) { + try { + return (Subject) getSubject.invoke(null, context); + } catch (IllegalAccessException e) { + throw new UnsupportedOperationException(e); + } catch (InvocationTargetException e) { + Throwable cause = e.getCause(); + if (cause instanceof RuntimeException) { + throw (RuntimeException) cause; + } else { + throw new RuntimeException(cause); + } + } + } + + @Override + public Subject current() { + return getSubject(getContext()); + } + + /** + * @return The result of Subject.doAs(Subject, PrivilegedExceptionAction) + */ + private <T> T doAs(Subject subject, PrivilegedExceptionAction<T> action) throws PrivilegedActionException { + try { + return (T) doAs.invoke(null, subject, action); + } catch (IllegalAccessException e) { + throw new UnsupportedOperationException(e); + } catch (InvocationTargetException e) { + Throwable cause = e.getCause(); + if (cause instanceof PrivilegedActionException) { + throw (PrivilegedActionException) cause; + } else if (cause instanceof RuntimeException) { + throw (RuntimeException) cause; + } else { + throw new RuntimeException(cause); + } + } + } + + @Override + public <T> T callAs(Subject subject, Callable<T> callable) throws CompletionException { + try { + return doAs(subject, callable::call); + } catch (PrivilegedActionException e) { + throw new CompletionException(e.getCause()); + } + } + } + + /** + * This class implements reflective access to the methods of Subject added to replace deprecated methods. + * <p>Instantiating this class may fail if any of the required classes or methods are not found. + * Method invocations for this class may fail with {@link UnsupportedOperationException} if all methods are found, + * but the operation is not permitted to be invoked. + * <p>This class is expected to be instantiable in JRE >= 18. At the time of writing, these methods do not have + * a sunset date, and are expected to be available past the removal of the SecurityManager. + */ Review Comment: Might be nice to add a `@see` tag that points at the new APIs that aren't available in older JDKs, and/or the JEP(s) that deprecate older APIs or introduce the modern ones. ########## clients/src/main/java/org/apache/kafka/common/internals/SecurityManagerCompatibility.java: ########## @@ -0,0 +1,355 @@ +/* + * 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.kafka.common.internals; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.security.PrivilegedAction; +import java.security.PrivilegedActionException; +import java.security.PrivilegedExceptionAction; +import java.util.Objects; +import java.util.concurrent.Callable; +import java.util.concurrent.CompletionException; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; + +import javax.security.auth.Subject; + +/** + * This is a compatibility class to provide dual-support for JREs with and without SecurityManager support. + * <p>Users should call {@link #get()} to retrieve a singleton instance, and call instance methods + * {@link #doPrivileged(PrivilegedAction)}, {@link #current()}, and {@link #callAs(Subject, Callable)}. + * <p>This class's motivation and expected behavior is defined in + * <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-1006%3A+Remove+SecurityManager+Support";>KIP-1006</a> + */ +public interface SecurityManagerCompatibility { + + static SecurityManagerCompatibility get() { + return Composite.INSTANCE; + } + + <T> T doPrivileged(PrivilegedAction<T> action); + + Subject current(); Review Comment: Could we maybe call this `currentSubject`? The name is a little ambiguous and doesn't immediately point readers towards the JDK API that it's a shim for. ########## clients/src/main/java/org/apache/kafka/common/internals/SecurityManagerCompatibility.java: ########## @@ -0,0 +1,355 @@ +/* + * 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.kafka.common.internals; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.security.PrivilegedAction; +import java.security.PrivilegedActionException; +import java.security.PrivilegedExceptionAction; +import java.util.Objects; +import java.util.concurrent.Callable; +import java.util.concurrent.CompletionException; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; + +import javax.security.auth.Subject; + +/** + * This is a compatibility class to provide dual-support for JREs with and without SecurityManager support. + * <p>Users should call {@link #get()} to retrieve a singleton instance, and call instance methods + * {@link #doPrivileged(PrivilegedAction)}, {@link #current()}, and {@link #callAs(Subject, Callable)}. + * <p>This class's motivation and expected behavior is defined in + * <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-1006%3A+Remove+SecurityManager+Support";>KIP-1006</a> + */ +public interface SecurityManagerCompatibility { + + static SecurityManagerCompatibility get() { + return Composite.INSTANCE; + } + + <T> T doPrivileged(PrivilegedAction<T> action); + + Subject current(); + + <T> T callAs(Subject subject, Callable<T> action) throws CompletionException; + + interface Loader { + Class<?> loadClass(String className) throws ClassNotFoundException; + + static Loader forName() { + return className -> Class.forName(className, true, Loader.class.getClassLoader()); + } + } + + /** + * This class implements reflective access to the deprecated-for-removal methods of AccessController and Subject. + * <p>Instantiating this class may fail if any of the required classes or methods are not found. + * Method invocations for this class may fail with {@link UnsupportedOperationException} if all methods are found, + * but the operation is not permitted to be invoked. + * <p>This class is expected to be instantiable in JRE >=8 until the removal finally takes place. + */ + @SuppressWarnings("unchecked") + class Legacy implements SecurityManagerCompatibility { + + private final Method doPrivileged; + private final Method getContext; + private final Method getSubject; + private final Method doAs; + + // Visible for testing + Legacy(Loader loader) throws ClassNotFoundException, NoSuchMethodException { + Class<?> accessController = loader.loadClass("java.security.AccessController"); + doPrivileged = accessController.getDeclaredMethod("doPrivileged", PrivilegedAction.class); + getContext = accessController.getDeclaredMethod("getContext"); + Class<?> accessControlContext = loader.loadClass("java.security.AccessControlContext"); + Class<?> subject = loader.loadClass(Subject.class.getName()); + getSubject = subject.getDeclaredMethod("getSubject", accessControlContext); + // Note that we use the real Subject.class rather than the `subject` variable + // This allows for mocking out the method without changing the argument types. + doAs = subject.getDeclaredMethod("doAs", Subject.class, PrivilegedExceptionAction.class); + } + + @Override + public <T> T doPrivileged(PrivilegedAction<T> action) { + try { + return (T) doPrivileged.invoke(null, action); + } catch (IllegalAccessException e) { + throw new UnsupportedOperationException(e); + } catch (InvocationTargetException e) { + Throwable cause = e.getCause(); + if (cause instanceof RuntimeException) { + throw (RuntimeException) cause; + } else { + throw new RuntimeException(cause); + } + } + } Review Comment: This same stanza is repeated several times, with the only difference being a special clause for `PrivilegedActionException` in `doAs`. Is there a chance we could deduplicate? Edit: It's also used for other reflective operations in other classes, would be nice to see those covered too. Bare minimum we could have something like a `RuntimeException maybeWrap(Throwable cause)` utility method. ########## clients/src/main/java/org/apache/kafka/common/internals/SecurityManagerCompatibility.java: ########## @@ -0,0 +1,355 @@ +/* + * 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.kafka.common.internals; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.security.PrivilegedAction; +import java.security.PrivilegedActionException; +import java.security.PrivilegedExceptionAction; +import java.util.Objects; +import java.util.concurrent.Callable; +import java.util.concurrent.CompletionException; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; + +import javax.security.auth.Subject; + +/** + * This is a compatibility class to provide dual-support for JREs with and without SecurityManager support. + * <p>Users should call {@link #get()} to retrieve a singleton instance, and call instance methods + * {@link #doPrivileged(PrivilegedAction)}, {@link #current()}, and {@link #callAs(Subject, Callable)}. + * <p>This class's motivation and expected behavior is defined in + * <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-1006%3A+Remove+SecurityManager+Support";>KIP-1006</a> + */ +public interface SecurityManagerCompatibility { + + static SecurityManagerCompatibility get() { + return Composite.INSTANCE; + } + + <T> T doPrivileged(PrivilegedAction<T> action); + + Subject current(); + + <T> T callAs(Subject subject, Callable<T> action) throws CompletionException; + + interface Loader { + Class<?> loadClass(String className) throws ClassNotFoundException; + + static Loader forName() { + return className -> Class.forName(className, true, Loader.class.getClassLoader()); + } + } + + /** + * This class implements reflective access to the deprecated-for-removal methods of AccessController and Subject. + * <p>Instantiating this class may fail if any of the required classes or methods are not found. + * Method invocations for this class may fail with {@link UnsupportedOperationException} if all methods are found, + * but the operation is not permitted to be invoked. + * <p>This class is expected to be instantiable in JRE >=8 until the removal finally takes place. + */ + @SuppressWarnings("unchecked") + class Legacy implements SecurityManagerCompatibility { + + private final Method doPrivileged; + private final Method getContext; + private final Method getSubject; + private final Method doAs; + + // Visible for testing + Legacy(Loader loader) throws ClassNotFoundException, NoSuchMethodException { + Class<?> accessController = loader.loadClass("java.security.AccessController"); + doPrivileged = accessController.getDeclaredMethod("doPrivileged", PrivilegedAction.class); + getContext = accessController.getDeclaredMethod("getContext"); + Class<?> accessControlContext = loader.loadClass("java.security.AccessControlContext"); + Class<?> subject = loader.loadClass(Subject.class.getName()); + getSubject = subject.getDeclaredMethod("getSubject", accessControlContext); + // Note that we use the real Subject.class rather than the `subject` variable + // This allows for mocking out the method without changing the argument types. + doAs = subject.getDeclaredMethod("doAs", Subject.class, PrivilegedExceptionAction.class); Review Comment: Check my work: this is fine because, although some of its methods are deprecated, the `Subject` class itself is not and we can continue to has a compile-time dependency on it without worrying about incompatibilities with future JDK versions. And, the reason we load the `Subject` class reflectively above is so that we can mock out the removal of its deprecated method(s). 👍 / 👎 ? ########## clients/src/main/java/org/apache/kafka/common/internals/SecurityManagerCompatibility.java: ########## @@ -0,0 +1,355 @@ +/* + * 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.kafka.common.internals; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.security.PrivilegedAction; +import java.security.PrivilegedActionException; +import java.security.PrivilegedExceptionAction; +import java.util.Objects; +import java.util.concurrent.Callable; +import java.util.concurrent.CompletionException; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; + +import javax.security.auth.Subject; + +/** + * This is a compatibility class to provide dual-support for JREs with and without SecurityManager support. + * <p>Users should call {@link #get()} to retrieve a singleton instance, and call instance methods + * {@link #doPrivileged(PrivilegedAction)}, {@link #current()}, and {@link #callAs(Subject, Callable)}. + * <p>This class's motivation and expected behavior is defined in + * <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-1006%3A+Remove+SecurityManager+Support";>KIP-1006</a> + */ +public interface SecurityManagerCompatibility { + + static SecurityManagerCompatibility get() { + return Composite.INSTANCE; + } + + <T> T doPrivileged(PrivilegedAction<T> action); + + Subject current(); + + <T> T callAs(Subject subject, Callable<T> action) throws CompletionException; Review Comment: Can we add Javadocs on these? They can link to any deprecated and modern APIs that they're meant to be shims for, and call out any differences in semantics between them. For example, we should call out in `callAs` that throwing `CompletionException` differs from the API that it replaces (`Subject::doAs`), which throws `PrivilegedException` instead. ########## clients/src/main/java/org/apache/kafka/common/internals/SecurityManagerCompatibility.java: ########## @@ -0,0 +1,355 @@ +/* + * 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.kafka.common.internals; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.security.PrivilegedAction; +import java.security.PrivilegedActionException; +import java.security.PrivilegedExceptionAction; +import java.util.Objects; +import java.util.concurrent.Callable; +import java.util.concurrent.CompletionException; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; + +import javax.security.auth.Subject; + +/** + * This is a compatibility class to provide dual-support for JREs with and without SecurityManager support. + * <p>Users should call {@link #get()} to retrieve a singleton instance, and call instance methods + * {@link #doPrivileged(PrivilegedAction)}, {@link #current()}, and {@link #callAs(Subject, Callable)}. + * <p>This class's motivation and expected behavior is defined in + * <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-1006%3A+Remove+SecurityManager+Support";>KIP-1006</a> + */ +public interface SecurityManagerCompatibility { + + static SecurityManagerCompatibility get() { + return Composite.INSTANCE; + } + + <T> T doPrivileged(PrivilegedAction<T> action); + + Subject current(); + + <T> T callAs(Subject subject, Callable<T> action) throws CompletionException; + + interface Loader { + Class<?> loadClass(String className) throws ClassNotFoundException; + + static Loader forName() { + return className -> Class.forName(className, true, Loader.class.getClassLoader()); + } + } + + /** + * This class implements reflective access to the deprecated-for-removal methods of AccessController and Subject. + * <p>Instantiating this class may fail if any of the required classes or methods are not found. + * Method invocations for this class may fail with {@link UnsupportedOperationException} if all methods are found, + * but the operation is not permitted to be invoked. + * <p>This class is expected to be instantiable in JRE >=8 until the removal finally takes place. + */ + @SuppressWarnings("unchecked") + class Legacy implements SecurityManagerCompatibility { + + private final Method doPrivileged; + private final Method getContext; + private final Method getSubject; + private final Method doAs; + + // Visible for testing + Legacy(Loader loader) throws ClassNotFoundException, NoSuchMethodException { + Class<?> accessController = loader.loadClass("java.security.AccessController"); + doPrivileged = accessController.getDeclaredMethod("doPrivileged", PrivilegedAction.class); + getContext = accessController.getDeclaredMethod("getContext"); + Class<?> accessControlContext = loader.loadClass("java.security.AccessControlContext"); + Class<?> subject = loader.loadClass(Subject.class.getName()); + getSubject = subject.getDeclaredMethod("getSubject", accessControlContext); + // Note that we use the real Subject.class rather than the `subject` variable + // This allows for mocking out the method without changing the argument types. + doAs = subject.getDeclaredMethod("doAs", Subject.class, PrivilegedExceptionAction.class); + } + + @Override + public <T> T doPrivileged(PrivilegedAction<T> action) { + try { + return (T) doPrivileged.invoke(null, action); + } catch (IllegalAccessException e) { + throw new UnsupportedOperationException(e); + } catch (InvocationTargetException e) { + Throwable cause = e.getCause(); + if (cause instanceof RuntimeException) { + throw (RuntimeException) cause; + } else { + throw new RuntimeException(cause); + } + } + } + + /** + * @return the result of AccessController.getContext(), of type AccessControlContext + */ + private Object getContext() { + try { + return getContext.invoke(null); + } catch (IllegalAccessException e) { + throw new UnsupportedOperationException(e); + } catch (InvocationTargetException e) { + Throwable cause = e.getCause(); + if (cause instanceof RuntimeException) { + throw (RuntimeException) cause; + } else { + throw new RuntimeException(cause); + } + } + } + + /** + * @param context The current AccessControlContext + * @return The result of Subject.getSubject(AccessControlContext) + */ + private Subject getSubject(Object context) { + try { + return (Subject) getSubject.invoke(null, context); + } catch (IllegalAccessException e) { + throw new UnsupportedOperationException(e); + } catch (InvocationTargetException e) { + Throwable cause = e.getCause(); + if (cause instanceof RuntimeException) { + throw (RuntimeException) cause; + } else { + throw new RuntimeException(cause); + } + } + } + + @Override + public Subject current() { + return getSubject(getContext()); + } Review Comment: It'd be a little less verbose if we inlined all of this into a single method; any reason not to? ```suggestion @Override public Subject current() { try { // AccessController.getContext() Object context = getContext.invoke(null); // Subject.getSubject(AccessControlContext) return (Subject) getSubject.invoke(null, context); } catch (IllegalAccessException e) { throw new UnsupportedOperationException(e); } catch (InvocationTargetException e) { Throwable cause = e.getCause(); if (cause instanceof RuntimeException) { throw (RuntimeException) cause; } else { throw new RuntimeException(cause); } } } ``` ########## clients/src/main/java/org/apache/kafka/common/internals/SecurityManagerCompatibility.java: ########## @@ -0,0 +1,355 @@ +/* + * 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.kafka.common.internals; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.security.PrivilegedAction; +import java.security.PrivilegedActionException; +import java.security.PrivilegedExceptionAction; +import java.util.Objects; +import java.util.concurrent.Callable; +import java.util.concurrent.CompletionException; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; + +import javax.security.auth.Subject; + +/** + * This is a compatibility class to provide dual-support for JREs with and without SecurityManager support. + * <p>Users should call {@link #get()} to retrieve a singleton instance, and call instance methods + * {@link #doPrivileged(PrivilegedAction)}, {@link #current()}, and {@link #callAs(Subject, Callable)}. + * <p>This class's motivation and expected behavior is defined in + * <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-1006%3A+Remove+SecurityManager+Support";>KIP-1006</a> + */ +public interface SecurityManagerCompatibility { + + static SecurityManagerCompatibility get() { + return Composite.INSTANCE; + } + + <T> T doPrivileged(PrivilegedAction<T> action); + + Subject current(); + + <T> T callAs(Subject subject, Callable<T> action) throws CompletionException; + + interface Loader { + Class<?> loadClass(String className) throws ClassNotFoundException; + + static Loader forName() { + return className -> Class.forName(className, true, Loader.class.getClassLoader()); + } + } + + /** + * This class implements reflective access to the deprecated-for-removal methods of AccessController and Subject. + * <p>Instantiating this class may fail if any of the required classes or methods are not found. + * Method invocations for this class may fail with {@link UnsupportedOperationException} if all methods are found, + * but the operation is not permitted to be invoked. + * <p>This class is expected to be instantiable in JRE >=8 until the removal finally takes place. + */ + @SuppressWarnings("unchecked") + class Legacy implements SecurityManagerCompatibility { + + private final Method doPrivileged; + private final Method getContext; + private final Method getSubject; + private final Method doAs; + + // Visible for testing + Legacy(Loader loader) throws ClassNotFoundException, NoSuchMethodException { + Class<?> accessController = loader.loadClass("java.security.AccessController"); + doPrivileged = accessController.getDeclaredMethod("doPrivileged", PrivilegedAction.class); + getContext = accessController.getDeclaredMethod("getContext"); + Class<?> accessControlContext = loader.loadClass("java.security.AccessControlContext"); + Class<?> subject = loader.loadClass(Subject.class.getName()); + getSubject = subject.getDeclaredMethod("getSubject", accessControlContext); + // Note that we use the real Subject.class rather than the `subject` variable + // This allows for mocking out the method without changing the argument types. + doAs = subject.getDeclaredMethod("doAs", Subject.class, PrivilegedExceptionAction.class); + } + + @Override + public <T> T doPrivileged(PrivilegedAction<T> action) { + try { + return (T) doPrivileged.invoke(null, action); + } catch (IllegalAccessException e) { + throw new UnsupportedOperationException(e); + } catch (InvocationTargetException e) { + Throwable cause = e.getCause(); + if (cause instanceof RuntimeException) { + throw (RuntimeException) cause; + } else { + throw new RuntimeException(cause); + } + } + } + + /** + * @return the result of AccessController.getContext(), of type AccessControlContext + */ + private Object getContext() { + try { + return getContext.invoke(null); + } catch (IllegalAccessException e) { + throw new UnsupportedOperationException(e); + } catch (InvocationTargetException e) { + Throwable cause = e.getCause(); + if (cause instanceof RuntimeException) { + throw (RuntimeException) cause; + } else { + throw new RuntimeException(cause); + } + } + } + + /** + * @param context The current AccessControlContext + * @return The result of Subject.getSubject(AccessControlContext) + */ + private Subject getSubject(Object context) { + try { + return (Subject) getSubject.invoke(null, context); + } catch (IllegalAccessException e) { + throw new UnsupportedOperationException(e); + } catch (InvocationTargetException e) { + Throwable cause = e.getCause(); + if (cause instanceof RuntimeException) { + throw (RuntimeException) cause; + } else { + throw new RuntimeException(cause); + } + } + } + + @Override + public Subject current() { + return getSubject(getContext()); + } + + /** + * @return The result of Subject.doAs(Subject, PrivilegedExceptionAction) + */ + private <T> T doAs(Subject subject, PrivilegedExceptionAction<T> action) throws PrivilegedActionException { + try { + return (T) doAs.invoke(null, subject, action); + } catch (IllegalAccessException e) { + throw new UnsupportedOperationException(e); + } catch (InvocationTargetException e) { + Throwable cause = e.getCause(); + if (cause instanceof PrivilegedActionException) { + throw (PrivilegedActionException) cause; + } else if (cause instanceof RuntimeException) { + throw (RuntimeException) cause; + } else { + throw new RuntimeException(cause); + } + } + } + + @Override + public <T> T callAs(Subject subject, Callable<T> callable) throws CompletionException { + try { + return doAs(subject, callable::call); + } catch (PrivilegedActionException e) { + throw new CompletionException(e.getCause()); + } + } + } + + /** + * This class implements reflective access to the methods of Subject added to replace deprecated methods. + * <p>Instantiating this class may fail if any of the required classes or methods are not found. + * Method invocations for this class may fail with {@link UnsupportedOperationException} if all methods are found, + * but the operation is not permitted to be invoked. + * <p>This class is expected to be instantiable in JRE >= 18. At the time of writing, these methods do not have + * a sunset date, and are expected to be available past the removal of the SecurityManager. + */ + @SuppressWarnings("unchecked") + class Modern implements SecurityManagerCompatibility { + + private final Method current; + private final Method callAs; + + // Visible for testing + Modern(Loader loader) throws NoSuchMethodException, ClassNotFoundException { + Class<?> subject = loader.loadClass(Subject.class.getName()); + current = subject.getDeclaredMethod("current"); + // Note that we use the real Subject.class rather than the `subject` variable + // This allows for mocking out the method without changing the argument types. + callAs = subject.getDeclaredMethod("callAs", Subject.class, Callable.class); + } + + @Override + public <T> T doPrivileged(PrivilegedAction<T> action) { + // This is intentionally a pass-through + return action.run(); + } + + @Override + public Subject current() { + try { + return (Subject) current.invoke(null); + } catch (IllegalAccessException e) { + throw new UnsupportedOperationException(e); + } catch (InvocationTargetException e) { + Throwable cause = e.getCause(); + if (cause instanceof RuntimeException) { + throw (RuntimeException) cause; + } else { + throw new RuntimeException(cause); + } + } + } + + @Override + public <T> T callAs(Subject subject, Callable<T> action) throws CompletionException { + try { + return (T) callAs.invoke(null, subject, action); + } catch (IllegalAccessException e) { + throw new UnsupportedOperationException(e); + } catch (InvocationTargetException e) { + Throwable cause = e.getCause(); + if (cause instanceof CompletionException) { + throw (CompletionException) cause; + } else if (cause instanceof RuntimeException) { + throw (RuntimeException) cause; + } else { + throw new RuntimeException(cause); + } + } + } + } + + /** + * This is the fallback to use if both {@link Legacy} and {@link Modern} compatibility strategies are unsuitable. + * <p>This is used to improve control flow and provide detailed error messages in unusual situations. + */ + class Unsupported implements SecurityManagerCompatibility { + + private final Throwable e1; + private final Throwable e2; + + private Unsupported(Throwable e1, Throwable e2) { + this.e1 = e1; + this.e2 = e2; + } + + private UnsupportedOperationException createException(String message) { + UnsupportedOperationException e = new UnsupportedOperationException(message); + e.addSuppressed(e1); + e.addSuppressed(e2); + return e; + } + + @Override + public <T> T doPrivileged(PrivilegedAction<T> action) { + throw createException("Unable to find suitable AccessController#doPrivileged implementation"); + } + + @Override + public Subject current() { + throw createException("Unable to find suitable Subject#getCurrent or Subject#current implementation"); + } + + @Override + public <T> T callAs(Subject subject, Callable<T> action) throws CompletionException { + throw createException("Unable to find suitable Subject#doAs or Subject#callAs implementation"); + } + } + + /** + * This strategy combines the functionality of the {@link Legacy}, {@link Modern}, and {@link Unsupported} + * strategies to provide the legacy APIs as long as they are present and not degraded. If the legacy APIs are + * missing or degraded, this falls back to the modern APIs. + */ + class Composite implements SecurityManagerCompatibility { + + private static final Logger log = LoggerFactory.getLogger(Composite.class); + private static final Composite INSTANCE = new Composite(Loader.forName()); + + private final SecurityManagerCompatibility fallbackStrategy; + private final AtomicReference<SecurityManagerCompatibility> activeStrategy; + + // Visible for testing + Composite(Loader loader) { + SecurityManagerCompatibility initial; + SecurityManagerCompatibility fallback = null; + try { + initial = new Legacy(loader); + try { + fallback = new Modern(loader); + // This is expected for JRE 18+ + log.debug("Loaded legacy SecurityManager methods, will fall back to modern methods after UnsupportedOperationException"); + } catch (NoSuchMethodException | ClassNotFoundException ex) { + // This is expected for JRE <= 17 + log.debug("Unable to load modern Subject methods, relying only on legacy methods", ex); + } + } catch (ClassNotFoundException | NoSuchMethodException e) { + try { + initial = new Modern(loader); + // This is expected for JREs after the removal takes place. + log.debug("Unable to load legacy SecurityManager methods, relying only on modern methods", e); + } catch (NoSuchMethodException | ClassNotFoundException ex) { + initial = new Unsupported(e, ex); + // This is not expected in normal use, only in test environments. + log.error("Unable to load legacy SecurityManager methods", e); + log.error("Unable to load modern Subject methods", ex); + } + } + Objects.requireNonNull(initial, "initial strategy must be defined"); + activeStrategy = new AtomicReference<>(initial); + fallbackStrategy = fallback; + } + + private <T> T performAction(Function<SecurityManagerCompatibility, T> action) { + SecurityManagerCompatibility active = activeStrategy.get(); + try { + return action.apply(active); + } catch (UnsupportedOperationException e) { Review Comment: Is it possible that an `UnsupportedOperationException` gets thrown directly by the underlying action, and not because we intercepted an `IllegalAccessException`? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
