gharris1727 commented on code in PR #16522: URL: https://github.com/apache/kafka/pull/16522#discussion_r1704430053
########## 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: I'll deduplicate the try-catch in these new classes, for some reason I thought that it would be more clear if it was verbose. I like the deduplicated form much better. I won't implement a `maybeWrap(Throwable)` as it has a much larger scope and the benefit is much more unclear to me. ########## 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: I originally left out all links to the underlying methods because I thought they would prevent compilation. TIL that javadocs in non-public-API classes can be "broken" with bad links because they're never processed by :javadoc. -- 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]
