This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push: new 5ce4262fed Deprecate SecurityManager references in ContextBind 5ce4262fed is described below commit 5ce4262fededa6effcd5bdaebd0b8a4911bcda5c Author: Mark Thomas <ma...@apache.org> AuthorDate: Thu Jan 19 08:38:24 2023 +0000 Deprecate SecurityManager references in ContextBind --- .../apache/catalina/connector/CoyoteAdapter.java | 16 +++--- .../catalina/core/ApplicationDispatcher.java | 4 +- .../org/apache/catalina/core/AsyncContextImpl.java | 12 ++--- java/org/apache/catalina/core/ContainerBase.java | 4 +- java/org/apache/catalina/core/StandardContext.java | 12 +++++ .../apache/catalina/core/StandardHostValve.java | 4 +- .../apache/catalina/session/DataSourceStore.java | 8 +-- java/org/apache/catalina/session/FileStore.java | 4 +- .../apache/catalina/session/StandardSession.java | 8 +-- .../org/apache/catalina/startup/FailedContext.java | 10 ++++ .../apache/catalina/valves/PersistentValve.java | 4 +- java/org/apache/coyote/AbstractProtocol.java | 8 +-- .../http11/upgrade/UpgradeServletInputStream.java | 8 +-- .../http11/upgrade/UpgradeServletOutputStream.java | 8 +-- java/org/apache/tomcat/ContextBind.java | 58 +++++++++++++++++----- test/org/apache/tomcat/unittest/TesterContext.java | 10 ++++ 16 files changed, 121 insertions(+), 57 deletions(-) diff --git a/java/org/apache/catalina/connector/CoyoteAdapter.java b/java/org/apache/catalina/connector/CoyoteAdapter.java index 0fd2a482ff..9efcce07e6 100644 --- a/java/org/apache/catalina/connector/CoyoteAdapter.java +++ b/java/org/apache/catalina/connector/CoyoteAdapter.java @@ -153,7 +153,7 @@ public class CoyoteAdapter implements Adapter { Context context = request.getContext(); ClassLoader oldCL = null; try { - oldCL = context.bind(false, null); + oldCL = context.bind(null); if (req.getReadListener() != null) { req.getReadListener().onError(t); } @@ -163,7 +163,7 @@ public class CoyoteAdapter implements Adapter { res.action(ActionCode.CLOSE_NOW, t); asyncConImpl.setErrorState(t, true); } finally { - context.unbind(false, oldCL); + context.unbind(oldCL); } } @@ -175,7 +175,7 @@ public class CoyoteAdapter implements Adapter { Context context = request.getContext(); ClassLoader oldCL = null; try { - oldCL = context.bind(false, null); + oldCL = context.bind(null); res.onWritePossible(); if (request.isFinished() && req.sendAllDataReadEvent() && readListener != null) { @@ -196,13 +196,13 @@ public class CoyoteAdapter implements Adapter { res.action(ActionCode.CLOSE_NOW, t); asyncConImpl.setErrorState(t, true); } finally { - context.unbind(false, oldCL); + context.unbind(oldCL); } } else if (readListener != null && status == SocketEvent.OPEN_READ) { Context context = request.getContext(); ClassLoader oldCL = null; try { - oldCL = context.bind(false, null); + oldCL = context.bind(null); // If data is being read on a non-container thread a // dispatch with status OPEN_READ will be used to get // execution back on a container thread for the @@ -229,7 +229,7 @@ public class CoyoteAdapter implements Adapter { res.action(ActionCode.CLOSE_NOW, t); asyncConImpl.setErrorState(t, true); } finally { - context.unbind(false, oldCL); + context.unbind(oldCL); } } } @@ -365,12 +365,12 @@ public class CoyoteAdapter implements Adapter { // method so this needs to be checked here ClassLoader oldCL = null; try { - oldCL = request.getContext().bind(false, null); + oldCL = request.getContext().bind(null); if (req.sendAllDataReadEvent()) { req.getReadListener().onAllDataRead(); } } finally { - request.getContext().unbind(false, oldCL); + request.getContext().unbind(oldCL); } } diff --git a/java/org/apache/catalina/core/ApplicationDispatcher.java b/java/org/apache/catalina/core/ApplicationDispatcher.java index 03568af66d..2e0d80ac32 100644 --- a/java/org/apache/catalina/core/ApplicationDispatcher.java +++ b/java/org/apache/catalina/core/ApplicationDispatcher.java @@ -517,7 +517,7 @@ final class ApplicationDispatcher implements AsyncDispatcher, RequestDispatcher // Checking to see if the context classloader is the current context // classloader. If it's not, we're saving it, and setting the context // classloader to the Context classloader - ClassLoader oldCCL = context.bind(false, null); + ClassLoader oldCCL = context.bind(null); // Initialize local variables we may need HttpServletResponse hresponse = state.hresponse; @@ -619,7 +619,7 @@ final class ApplicationDispatcher implements AsyncDispatcher, RequestDispatcher } // Reset the old context class loader - context.unbind(false, oldCCL); + context.unbind(oldCCL); // Unwrap request/response if needed // See Bugzilla 30949 diff --git a/java/org/apache/catalina/core/AsyncContextImpl.java b/java/org/apache/catalina/core/AsyncContextImpl.java index 776aa8acf9..3b0fbd0b57 100644 --- a/java/org/apache/catalina/core/AsyncContextImpl.java +++ b/java/org/apache/catalina/core/AsyncContextImpl.java @@ -98,7 +98,7 @@ public class AsyncContextImpl implements AsyncContext, AsyncContextCallback { } List<AsyncListenerWrapper> listenersCopy = new ArrayList<>(listeners); - ClassLoader oldCL = context.bind(false, null); + ClassLoader oldCL = context.bind(null); try { for (AsyncListenerWrapper listener : listenersCopy) { try { @@ -112,7 +112,7 @@ public class AsyncContextImpl implements AsyncContext, AsyncContextCallback { } finally { context.fireRequestDestroyEvent(request.getRequest()); clearServletRequestResponse(); - context.unbind(false, oldCL); + context.unbind(oldCL); } } @@ -127,7 +127,7 @@ public class AsyncContextImpl implements AsyncContext, AsyncContextCallback { if (log.isDebugEnabled()) { log.debug(sm.getString("asyncContextImpl.fireOnTimeout")); } - ClassLoader oldCL = context.bind(false, null); + ClassLoader oldCL = context.bind(null); try { List<AsyncListenerWrapper> listenersCopy = new ArrayList<>(listeners); for (AsyncListenerWrapper listener : listenersCopy) { @@ -142,7 +142,7 @@ public class AsyncContextImpl implements AsyncContext, AsyncContextCallback { request.getCoyoteRequest().action( ActionCode.ASYNC_IS_TIMINGOUT, result); } finally { - context.unbind(false, oldCL); + context.unbind(oldCL); } } return !result.get(); @@ -543,7 +543,7 @@ public class AsyncContextImpl implements AsyncContext, AsyncContextCallback { @Override public void run() { - ClassLoader oldCL = context.bind(false, null); + ClassLoader oldCL = context.bind(null); try { wrapped.run(); } catch (Throwable t) { @@ -554,7 +554,7 @@ public class AsyncContextImpl implements AsyncContext, AsyncContextCallback { coyoteResponse.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); coyoteResponse.setError(); } finally { - context.unbind(false, oldCL); + context.unbind(oldCL); } // Since this runnable is not executing as a result of a socket diff --git a/java/org/apache/catalina/core/ContainerBase.java b/java/org/apache/catalina/core/ContainerBase.java index c3167f9055..67b4354bc3 100644 --- a/java/org/apache/catalina/core/ContainerBase.java +++ b/java/org/apache/catalina/core/ContainerBase.java @@ -1320,7 +1320,7 @@ public abstract class ContainerBase extends LifecycleMBeanBase implements Contai // Ensure background processing for Contexts and Wrappers // is performed under the web app's class loader - originalClassLoader = ((Context) container).bind(false, null); + originalClassLoader = ((Context) container).bind(null); } container.backgroundProcess(); Container[] children = container.findChildren(); @@ -1334,7 +1334,7 @@ public abstract class ContainerBase extends LifecycleMBeanBase implements Contai log.error(sm.getString("containerBase.backgroundProcess.error"), t); } finally { if (container instanceof Context) { - ((Context) container).unbind(false, originalClassLoader); + ((Context) container).unbind(originalClassLoader); } } } diff --git a/java/org/apache/catalina/core/StandardContext.java b/java/org/apache/catalina/core/StandardContext.java index eb51979e16..1a3c33bdb5 100644 --- a/java/org/apache/catalina/core/StandardContext.java +++ b/java/org/apache/catalina/core/StandardContext.java @@ -5761,6 +5761,12 @@ public class StandardContext extends ContainerBase @Override public ClassLoader bind(boolean usePrivilegedAction, ClassLoader originalClassLoader) { + return bind(originalClassLoader); + } + + + @Override + public ClassLoader bind(ClassLoader originalClassLoader) { Loader loader = getLoader(); ClassLoader webApplicationClassLoader = null; if (loader != null) { @@ -5797,6 +5803,12 @@ public class StandardContext extends ContainerBase @Override public void unbind(boolean usePrivilegedAction, ClassLoader originalClassLoader) { + unbind(originalClassLoader); + } + + + @Override + public void unbind(ClassLoader originalClassLoader) { if (originalClassLoader == null) { return; } diff --git a/java/org/apache/catalina/core/StandardHostValve.java b/java/org/apache/catalina/core/StandardHostValve.java index 159eb26296..76250cd173 100644 --- a/java/org/apache/catalina/core/StandardHostValve.java +++ b/java/org/apache/catalina/core/StandardHostValve.java @@ -100,7 +100,7 @@ final class StandardHostValve extends ValveBase { boolean asyncAtStart = request.isAsync(); try { - context.bind(false, MY_CLASSLOADER); + context.bind(MY_CLASSLOADER); if (!asyncAtStart && !context.fireRequestInitEvent(request.getRequest())) { // Don't fire listeners during async processing (the listener @@ -167,7 +167,7 @@ final class StandardHostValve extends ValveBase { request.getSession(false); } - context.unbind(false, MY_CLASSLOADER); + context.unbind(MY_CLASSLOADER); } } diff --git a/java/org/apache/catalina/session/DataSourceStore.java b/java/org/apache/catalina/session/DataSourceStore.java index 7bd8957a1e..c46aa2b17c 100644 --- a/java/org/apache/catalina/session/DataSourceStore.java +++ b/java/org/apache/catalina/session/DataSourceStore.java @@ -465,7 +465,7 @@ public class DataSourceStore extends StoreBase { return null; } - ClassLoader oldThreadContextCL = context.bind(false, null); + ClassLoader oldThreadContextCL = context.bind(null); try (PreparedStatement preparedLoadSql = _conn.prepareStatement(loadSql)){ preparedLoadSql.setString(1, id); @@ -492,7 +492,7 @@ public class DataSourceStore extends StoreBase { } catch (SQLException e) { contextLog.error(sm.getString(getStoreName() + ".SQLException", e)); } finally { - context.unbind(false, oldThreadContextCL); + context.unbind(oldThreadContextCL); release(_conn); } numberOfTries--; @@ -697,7 +697,7 @@ public class DataSourceStore extends StoreBase { org.apache.catalina.Context context = getManager().getContext(); ClassLoader oldThreadContextCL = null; if (localDataSource) { - oldThreadContextCL = context.bind(false, null); + oldThreadContextCL = context.bind(null); } Context initCtx; @@ -711,7 +711,7 @@ public class DataSourceStore extends StoreBase { this.dataSourceName), e); } finally { if (localDataSource) { - context.unbind(false, oldThreadContextCL); + context.unbind(oldThreadContextCL); } } } diff --git a/java/org/apache/catalina/session/FileStore.java b/java/org/apache/catalina/session/FileStore.java index 22107f3141..c4002048bf 100644 --- a/java/org/apache/catalina/session/FileStore.java +++ b/java/org/apache/catalina/session/FileStore.java @@ -225,7 +225,7 @@ public final class FileStore extends StoreBase { contextLog.debug(sm.getString(getStoreName()+".loading", id, file.getAbsolutePath())); } - ClassLoader oldThreadContextCL = context.bind(false, null); + ClassLoader oldThreadContextCL = context.bind(null); try (FileInputStream fis = new FileInputStream(file.getAbsolutePath()); ObjectInputStream ois = getObjectInputStream(fis)) { @@ -240,7 +240,7 @@ public final class FileStore extends StoreBase { } return null; } finally { - context.unbind(false, oldThreadContextCL); + context.unbind(oldThreadContextCL); } } diff --git a/java/org/apache/catalina/session/StandardSession.java b/java/org/apache/catalina/session/StandardSession.java index 13b7d2af7d..54b5927e7d 100644 --- a/java/org/apache/catalina/session/StandardSession.java +++ b/java/org/apache/catalina/session/StandardSession.java @@ -763,7 +763,7 @@ public class StandardSession implements HttpSession, Session, Serializable { if (notify) { ClassLoader oldContextClassLoader = null; try { - oldContextClassLoader = context.bind(false, null); + oldContextClassLoader = context.bind(null); Object listeners[] = context.getApplicationLifecycleListeners(); if (listeners != null && listeners.length > 0) { HttpSessionEvent event = @@ -795,7 +795,7 @@ public class StandardSession implements HttpSession, Session, Serializable { } } } finally { - context.unbind(false, oldContextClassLoader); + context.unbind(oldContextClassLoader); } } @@ -831,12 +831,12 @@ public class StandardSession implements HttpSession, Session, Serializable { String keys[] = keys(); ClassLoader oldContextClassLoader = null; try { - oldContextClassLoader = context.bind(false, null); + oldContextClassLoader = context.bind(null); for (String key : keys) { removeAttributeInternal(key, notify); } } finally { - context.unbind(false, oldContextClassLoader); + context.unbind(oldContextClassLoader); } } diff --git a/java/org/apache/catalina/startup/FailedContext.java b/java/org/apache/catalina/startup/FailedContext.java index d24dfd135b..f892b61298 100644 --- a/java/org/apache/catalina/startup/FailedContext.java +++ b/java/org/apache/catalina/startup/FailedContext.java @@ -745,11 +745,21 @@ public class FailedContext extends LifecycleMBeanBase implements Context { return null; } + @Override + public ClassLoader bind(ClassLoader originalClassLoader) { + return null; + } + @Override public void unbind(boolean usePrivilegedAction, ClassLoader originalClassLoader) { // NO-OP } + @Override + public void unbind(ClassLoader originalClassLoader) { + // NO-OP + } + @Override public Object getNamingToken() { return null; } diff --git a/java/org/apache/catalina/valves/PersistentValve.java b/java/org/apache/catalina/valves/PersistentValve.java index cd36b1386d..bdf11da313 100644 --- a/java/org/apache/catalina/valves/PersistentValve.java +++ b/java/org/apache/catalina/valves/PersistentValve.java @@ -230,14 +230,14 @@ public class PersistentValve extends ValveBase { private void bind(Context context) { if (clBindRequired) { - context.bind(false, MY_CLASSLOADER); + context.bind(MY_CLASSLOADER); } } private void unbind(Context context) { if (clBindRequired) { - context.unbind(false, MY_CLASSLOADER); + context.unbind(MY_CLASSLOADER); } } diff --git a/java/org/apache/coyote/AbstractProtocol.java b/java/org/apache/coyote/AbstractProtocol.java index 55eb2a7b3c..29881aacd3 100644 --- a/java/org/apache/coyote/AbstractProtocol.java +++ b/java/org/apache/coyote/AbstractProtocol.java @@ -901,11 +901,11 @@ public abstract class AbstractProtocol<S> implements ProtocolHandler, if (upgradeToken.getInstanceManager() == null) { httpUpgradeHandler.init((WebConnection) processor); } else { - ClassLoader oldCL = upgradeToken.getContextBind().bind(false, null); + ClassLoader oldCL = upgradeToken.getContextBind().bind(null); try { httpUpgradeHandler.init((WebConnection) processor); } finally { - upgradeToken.getContextBind().unbind(false, oldCL); + upgradeToken.getContextBind().unbind(oldCL); } } if (httpUpgradeHandler instanceof InternalHttpUpgradeHandler) { @@ -965,7 +965,7 @@ public abstract class AbstractProtocol<S> implements ProtocolHandler, if (instanceManager == null) { httpUpgradeHandler.destroy(); } else { - ClassLoader oldCL = upgradeToken.getContextBind().bind(false, null); + ClassLoader oldCL = upgradeToken.getContextBind().bind(null); try { httpUpgradeHandler.destroy(); } finally { @@ -975,7 +975,7 @@ public abstract class AbstractProtocol<S> implements ProtocolHandler, ExceptionUtils.handleThrowable(e); getLog().error(sm.getString("abstractConnectionHandler.error"), e); } - upgradeToken.getContextBind().unbind(false, oldCL); + upgradeToken.getContextBind().unbind(oldCL); } } } diff --git a/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java b/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java index 63ad3f473d..562ec5e3f4 100644 --- a/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java +++ b/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java @@ -223,7 +223,7 @@ public class UpgradeServletInputStream extends ServletInputStream { onError(e); } ready = Boolean.TRUE; - ClassLoader oldCL = processor.getUpgradeToken().getContextBind().bind(false, null); + ClassLoader oldCL = processor.getUpgradeToken().getContextBind().bind(null); try { if (!eof) { listener.onDataAvailable(); @@ -235,7 +235,7 @@ public class UpgradeServletInputStream extends ServletInputStream { ExceptionUtils.handleThrowable(t); onError(t); } finally { - processor.getUpgradeToken().getContextBind().unbind(false, oldCL); + processor.getUpgradeToken().getContextBind().unbind(oldCL); } } @@ -244,14 +244,14 @@ public class UpgradeServletInputStream extends ServletInputStream { if (listener == null) { return; } - ClassLoader oldCL = processor.getUpgradeToken().getContextBind().bind(false, null); + ClassLoader oldCL = processor.getUpgradeToken().getContextBind().bind(null); try { listener.onError(t); } catch (Throwable t2) { ExceptionUtils.handleThrowable(t2); log.warn(sm.getString("upgrade.sis.onErrorFail"), t2); } finally { - processor.getUpgradeToken().getContextBind().unbind(false, oldCL); + processor.getUpgradeToken().getContextBind().unbind(oldCL); } try { close(); diff --git a/java/org/apache/coyote/http11/upgrade/UpgradeServletOutputStream.java b/java/org/apache/coyote/http11/upgrade/UpgradeServletOutputStream.java index 62d44eb05f..74ade3025d 100644 --- a/java/org/apache/coyote/http11/upgrade/UpgradeServletOutputStream.java +++ b/java/org/apache/coyote/http11/upgrade/UpgradeServletOutputStream.java @@ -249,14 +249,14 @@ public class UpgradeServletOutputStream extends ServletOutputStream { } if (fire) { - ClassLoader oldCL = processor.getUpgradeToken().getContextBind().bind(false, null); + ClassLoader oldCL = processor.getUpgradeToken().getContextBind().bind(null); try { listener.onWritePossible(); } catch (Throwable t) { ExceptionUtils.handleThrowable(t); onError(t); } finally { - processor.getUpgradeToken().getContextBind().unbind(false, oldCL); + processor.getUpgradeToken().getContextBind().unbind(oldCL); } } } @@ -266,14 +266,14 @@ public class UpgradeServletOutputStream extends ServletOutputStream { if (listener == null) { return; } - ClassLoader oldCL = processor.getUpgradeToken().getContextBind().bind(false, null); + ClassLoader oldCL = processor.getUpgradeToken().getContextBind().bind(null); try { listener.onError(t); } catch (Throwable t2) { ExceptionUtils.handleThrowable(t2); log.warn(sm.getString("upgrade.sos.onErrorFail"), t2); } finally { - processor.getUpgradeToken().getContextBind().unbind(false, oldCL); + processor.getUpgradeToken().getContextBind().unbind(oldCL); } try { close(); diff --git a/java/org/apache/tomcat/ContextBind.java b/java/org/apache/tomcat/ContextBind.java index ab9a1d7db3..2fb868a6f7 100644 --- a/java/org/apache/tomcat/ContextBind.java +++ b/java/org/apache/tomcat/ContextBind.java @@ -27,19 +27,14 @@ public interface ContextBind { * {@link org.apache.catalina.ThreadBindingListener#bind()} will be called * after the change has been made. * - * @param usePrivilegedAction - * Should a {@link java.security.PrivilegedAction} be used when - * obtaining the current thread context class loader and setting - * the new one? - * @param originalClassLoader - * The current class loader if known to save this method having to - * look it up + * @param originalClassLoader The current class loader if known to save this + * method having to look it up * * @return If the class loader has been changed by the method it will return - * the thread context class loader in use when the method was - * called. If no change was made then this method returns null. + * the thread context class loader in use when the method was called. If + * no change was made then this method returns null. */ - ClassLoader bind(boolean usePrivilegedAction, ClassLoader originalClassLoader); + ClassLoader bind(ClassLoader originalClassLoader); /** * Restore the current thread context class loader to the original class @@ -50,11 +45,48 @@ public interface ContextBind { * {@link org.apache.catalina.ThreadBindingListener#unbind()} will be called * before the change is made. * - * @param usePrivilegedAction - * Should a {@link java.security.PrivilegedAction} be used when - * setting the current thread context class loader? * @param originalClassLoader * The class loader to restore as the thread context class loader */ + void unbind(ClassLoader originalClassLoader); + + /** + * Change the current thread context class loader to the web application + * class loader. If no web application class loader is defined, or if the + * current thread is already using the web application class loader then no + * change will be made. If the class loader is changed and a + * {@link org.apache.catalina.ThreadBindingListener} is configured then + * {@link org.apache.catalina.ThreadBindingListener#bind()} will be called + * after the change has been made. + * + * @param usePrivilegedAction Unused + * @param originalClassLoader The current class loader if known to save this + * method having to look it up + * + * @return If the class loader has been changed by the method it will return + * the thread context class loader in use when the method was called. If + * no change was made then this method returns null. + * + * @deprecated Unused. Will be removed in Tomcat 12 onwards. + */ + @Deprecated + ClassLoader bind(boolean usePrivilegedAction, ClassLoader originalClassLoader); + + /** + * Restore the current thread context class loader to the original class + * loader in used before {@link #bind(boolean, ClassLoader)} was called. If + * no original class loader is passed to this method then no change will be + * made. If the class loader is changed and a + * {@link org.apache.catalina.ThreadBindingListener} is configured then + * {@link org.apache.catalina.ThreadBindingListener#unbind()} will be called + * before the change is made. + * + * @param usePrivilegedAction Unused + * @param originalClassLoader The class loader to restore as the thread + * context class loader + * + * @deprecated Unused. Will be removed in Tomcat 12 onwards. + */ + @Deprecated void unbind(boolean usePrivilegedAction, ClassLoader originalClassLoader); } diff --git a/test/org/apache/tomcat/unittest/TesterContext.java b/test/org/apache/tomcat/unittest/TesterContext.java index 16c22425fc..9c2e10754c 100644 --- a/test/org/apache/tomcat/unittest/TesterContext.java +++ b/test/org/apache/tomcat/unittest/TesterContext.java @@ -1210,11 +1210,21 @@ public class TesterContext implements Context { return null; } + @Override + public ClassLoader bind(ClassLoader originalClassLoader) { + return null; + } + @Override public void unbind(boolean usePrivilegedAction, ClassLoader originalClassLoader) { // NO-OP } + @Override + public void unbind(ClassLoader originalClassLoader) { + // NO-OP + } + @Override public Object getNamingToken() { return null; } --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org