Make ContextHandler catch and ignore exceptions thrown by Thread.setContextClassLoader() (#13677)

Ignore SecurityException thrown by setContextClassLoader() so JDK25's InnocuousThread can call into a context's request and response.

Signed-off-by: Ludovic Orban <lorban@bitronix.be>

Author: lorban

jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java

@@ -77,7 +77,7 @@
 public class ContextHandler extends Handler.Wrapper implements Attributes, AliasCheck, Deployable
 {
     private static final Logger LOG = LoggerFactory.getLogger(ContextHandler.class);
-    private static final ThreadLocal<Context> __context = new ThreadLocal<>();
+    private static final ThreadLocal<Context> CURRENT_CONTEXT = new ThreadLocal<>();
 
     public static final String MANAGED_ATTRIBUTES = "org.eclipse.jetty.server.context.ManagedAttributes";
 
@@ -95,7 +95,7 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Alias
      */
     public static Context getCurrentContext()
     {
-        return __context.get();
+        return CURRENT_CONTEXT.get();
     }
 
     /**
@@ -106,7 +106,7 @@ public static Context getCurrentContext()
      */
     public static Context getCurrentContext(Server server)
     {
-        Context context = __context.get();
+        Context context = CURRENT_CONTEXT.get();
         return context == null ? (server == null ? null : server.getContext()) : context;
     }
 
@@ -513,6 +513,11 @@ public ClassLoader getClassLoader()
         return _classLoader;
     }
 
+    /**
+     * The {@link ClassLoader} to set as the {@link Thread#setContextClassLoader(ClassLoader) thread's context classloader}
+     * when the thread executing this handler enters the scope of this context.
+     * @param contextLoader the {@link ClassLoader} or {@code null} to avoid changing the thread's context classloader.
+     */
     public void setClassLoader(ClassLoader contextLoader)
     {
         if (isStarted())
@@ -655,8 +660,8 @@ public boolean addEventListener(EventListener listener)
             if (listener instanceof ContextScopeListener contextScopeListener)
             {
                 _contextListeners.add(contextScopeListener);
-                if (__context.get() != null)
-                    contextScopeListener.enterScope(__context.get(), null);
+                if (CURRENT_CONTEXT.get() != null)
+                    contextScopeListener.enterScope(CURRENT_CONTEXT.get(), null);
             }
             return true;
         }
@@ -671,8 +676,8 @@ public boolean removeEventListener(EventListener listener)
             if (listener instanceof ContextScopeListener contextScopeListener)
             {
                 _contextListeners.remove(contextScopeListener);
-                if (__context.get() != null)
-                    contextScopeListener.exitScope(__context.get(), null);
+                if (CURRENT_CONTEXT.get() != null)
+                    contextScopeListener.exitScope(CURRENT_CONTEXT.get(), null);
             }
             return true;
         }
@@ -728,12 +733,31 @@ protected void initializeDefaultsComplete()
     {
     }
 
+    /**
+     * <p>Enters the scope of the {@link Context}.</p>
+     * <p> Attempts to set the {@link Thread#setContextClassLoader(ClassLoader) thread's context classloader}
+     * to the {@link #getClassLoader() configured one} if a non-null one was set.</p>
+     *
+     * @param contextRequest the context's {@link Request}
+     * @return the previous {@link Thread#getContextClassLoader() thread's context classloader}
+     */
     protected ClassLoader enterScope(Request contextRequest)
     {
+        CURRENT_CONTEXT.set(_context);
+
         ClassLoader lastLoader = Thread.currentThread().getContextClassLoader();
-        __context.set(_context);
-        if (_classLoader != null)
-            Thread.currentThread().setContextClassLoader(_classLoader);
+        if (_classLoader != null && _classLoader != lastLoader)
+        {
+            try
+            {
+                Thread.currentThread().setContextClassLoader(_classLoader);
+            }
+            catch (Throwable x)
+            {
+                if (LOG.isDebugEnabled())
+                    LOG.debug("error setting a context classloader on thread {}", Thread.currentThread(), x);
+            }
+        }
         notifyEnterScope(contextRequest);
         return lastLoader;
     }
@@ -756,11 +780,20 @@ protected void notifyEnterScope(Request request)
         }
     }
 
-    protected void exitScope(Request request, Context lastContext, ClassLoader lastLoader)
+    /**
+     * <p>Exits the scope of the {@link Context}.</p>
+     *
+     * @param contextRequest the context's {@link Request}
+     * @param lastContext the previous context to restore as the current one.
+     * @param lastLoader the previous {@link Thread#getContextClassLoader() thread's context classloader} to restore
+     */
+    protected void exitScope(Request contextRequest, Context lastContext, ClassLoader lastLoader)
     {
-        notifyExitScope(request);
-        __context.set(lastContext);
-        Thread.currentThread().setContextClassLoader(lastLoader);
+        CURRENT_CONTEXT.set(lastContext);
+
+        notifyExitScope(contextRequest);
+        if (Thread.currentThread().getContextClassLoader() != lastLoader)
+            Thread.currentThread().setContextClassLoader(lastLoader);
     }
 
     /**
@@ -1588,7 +1621,7 @@ public List<String> getVirtualHosts()
 
         public void call(Invocable.Callable callable, Request request) throws Exception
         {
-            Context lastContext = __context.get();
+            Context lastContext = CURRENT_CONTEXT.get();
             if (lastContext == this)
                 callable.call();
             else
@@ -1607,7 +1640,7 @@ public void call(Invocable.Callable callable, Request request) throws Exception
 
         public <T> boolean test(Predicate<T> predicate, T t, Request request)
         {
-            Context lastContext = __context.get();
+            Context lastContext = CURRENT_CONTEXT.get();
             if (lastContext == this)
                 return predicate.test(t);
 
@@ -1624,7 +1657,7 @@ public <T> boolean test(Predicate<T> predicate, T t, Request request)
 
         public void accept(Consumer<Throwable> consumer, Throwable t, Request request)
         {
-            Context lastContext = __context.get();
+            Context lastContext = CURRENT_CONTEXT.get();
             if (lastContext == this)
                 consumer.accept(t);
             else
@@ -1649,7 +1682,7 @@ public void run(Runnable runnable)
 
         public void run(Runnable runnable, Request request)
         {
-            Context lastContext = __context.get();
+            Context lastContext = CURRENT_CONTEXT.get();
             if (lastContext == this)
                 runnable.run();
             else

jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java

@@ -19,6 +19,10 @@
 import java.net.URL;
 import java.net.URLClassLoader;
 import java.nio.ByteBuffer;
+import java.nio.channels.AsynchronousFileChannel;
+import java.nio.channels.CompletionHandler;
+import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -66,6 +70,7 @@
 import org.eclipse.jetty.server.internal.HttpChannelState;
 import org.eclipse.jetty.toolchain.test.FS;
 import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
+import org.eclipse.jetty.util.Blocker;
 import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.util.IO;
@@ -107,7 +112,7 @@ public class ContextHandlerTest
     AtomicBoolean _inContext;
 
     @BeforeEach
-    public void beforeEach() throws Exception
+    public void beforeEach()
     {
         _server = new Server();
         _loader = new URLClassLoader(new URL[0], this.getClass().getClassLoader());
@@ -170,6 +175,132 @@ public void testMiss() throws Exception
         assertThat(stream.getResponse().getStatus(), equalTo(404));
     }
 
+    @ParameterizedTest
+    @ValueSource(booleans = {true, false})
+    public void testThreadRefusingContextClassLoader(boolean withClassLoader) throws Exception
+    {
+        LocalConnector connector = new LocalConnector(_server);
+        _server.addConnector(connector);
+        _contextHandler.setClassLoader(withClassLoader ? _loader : null);
+        _contextHandler.setHandler(new Handler.Abstract()
+        {
+            @Override
+            public boolean handle(Request request, Response response, Callback callback) throws Exception
+            {
+                CountDownLatch latch = new CountDownLatch(1);
+                var t = new Thread()
+                {
+                    @Override
+                    public void run()
+                    {
+                        try (Blocker.Callback cb = Blocker.callback())
+                        {
+                            // When a classloader is configured, Response.write() tries to set it as the context classloader.
+                            response.write(true, ByteBuffer.allocate(32), cb);
+                            cb.block();
+                        }
+                        catch (IOException e)
+                        {
+                            throw new RuntimeException(e);
+                        }
+                        finally
+                        {
+                            latch.countDown();
+                        }
+                    }
+
+                    @Override
+                    public void setContextClassLoader(ClassLoader cl)
+                    {
+                        throw new ArithmeticException();
+                    }
+                };
+                t.start();
+                assertTrue(latch.await(5, TimeUnit.SECONDS));
+
+                callback.succeeded();
+                return true;
+            }
+        });
+        _server.start();
+
+        String rawRequest = """
+            GET /ctx/ HTTP/1.1
+            Host: local
+            Connection: close
+            
+            """;
+
+        String rawResponse = connector.getResponse(rawRequest);
+        HttpTester.Response response = HttpTester.parseResponse(rawResponse);
+        assertThat(response.getStatus(), is(200));
+    }
+
+    @ParameterizedTest
+    @ValueSource(booleans = {true, false})
+    public void testAIOAndClassLoader(boolean withClassLoader) throws Exception
+    {
+        LocalConnector connector = new LocalConnector(_server);
+        _server.addConnector(connector);
+        _contextHandler.setClassLoader(withClassLoader ? _loader : null);
+        _contextHandler.setHandler(new Handler.Abstract()
+        {
+            @Override
+            public boolean handle(Request request, Response response, Callback callback) throws Exception
+            {
+                Path tempFile = request.getContext().getTempDirectory().toPath().resolve("file.bin");
+                try (AsynchronousFileChannel afc = AsynchronousFileChannel.open(tempFile, StandardOpenOption.CREATE, StandardOpenOption.WRITE))
+                {
+                    CountDownLatch latch = new CountDownLatch(1);
+                    // In JDK 25, The CompletionHandler gets called by an InnocuousThread that throws when trying to set the context classloader.
+                    afc.write(ByteBuffer.allocate(32), 0L, null, new CompletionHandler<>()
+                    {
+                        @Override
+                        public void completed(Integer result, Object attachment)
+                        {
+                            try (Blocker.Callback cb = Blocker.callback())
+                            {
+                                // When a classloader is configured, Response.write() tries to set it as the context classloader.
+                                response.write(true, ByteBuffer.allocate(32), cb);
+                                cb.block();
+                            }
+                            catch (IOException e)
+                            {
+                                throw new RuntimeException(e);
+                            }
+                            finally
+                            {
+                                latch.countDown();
+                            }
+                        }
+
+                        @Override
+                        public void failed(Throwable exc, Object attachment)
+                        {
+                            latch.countDown();
+                        }
+                    });
+                    assertTrue(latch.await(5, TimeUnit.SECONDS));
+                }
+
+                callback.succeeded();
+                return true;
+            }
+        });
+        _server.start();
+
+        String rawRequest = """
+            GET /ctx/ HTTP/1.1
+            Host: local
+            Connection: close
+            
+            """;
+
+        String rawResponse = connector.getResponse(rawRequest);
+        HttpTester.Response response = HttpTester.parseResponse(rawResponse);
+        assertThat(response.getStatus(), is(200));
+    }
+
     @Test
     public void testSimpleGET() throws Exception
     {