Jetty 12 - Cleanup Shutdown classes (#9201)

* Fixed ShutdownHandler in jetty-core
* Delete ee9 ShutdownHandler
* Rename GracefulShutdownHandler to just GracefulHandler
* Adding graceful Jetty module
* Improved Javadoc + Token encoding tests

jetty-core/jetty-server/src/main/config/etc/jetty-graceful.xml

@@ -0,0 +1,14 @@
+<?xml version="1.0"?>
+<!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "https://www.eclipse.org/jetty/configure_10_0.dtd">
+
+<!-- =============================================================== -->
+<!-- Mixin the Graceful Handler                                      -->
+<!-- =============================================================== -->
+
+<Configure id="Server" class="org.eclipse.jetty.server.Server">
+  <Call name="insertHandler">
+    <Arg>
+      <New id="GracefulHandler" class="org.eclipse.jetty.server.handler.GracefulHandler" />
+    </Arg>
+  </Call>
+</Configure>

jetty-core/jetty-server/src/main/config/modules/graceful.mod

@@ -0,0 +1,18 @@
+# DO NOT EDIT - See: https://www.eclipse.org/jetty/documentation/current/startup-modules.html
+
+[description]
+Enables Graceful processing of requests
+
+[tags]
+server
+
+[depend]
+server
+
+[xml]
+etc/jetty-graceful.xml
+
+[ini-template]
+
+## If the Graceful shutdown should wait for async requests as well as the currently dispatched ones.
+# jetty.statistics.gracefulShutdownWaitsForRequests=true

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

@@ -28,14 +28,14 @@ import org.slf4j.LoggerFactory;
 /**
  * Handler to track active requests and allow them to gracefully complete.
  */
-public class GracefulShutdownHandler extends Handler.Wrapper implements Graceful
+public class GracefulHandler extends Handler.Wrapper implements Graceful
 {
-    private static final Logger LOG = LoggerFactory.getLogger(GracefulShutdownHandler.class);
+    private static final Logger LOG = LoggerFactory.getLogger(GracefulHandler.class);
 
     private final LongAdder dispatchedStats = new LongAdder();
     private final Shutdown shutdown;
 
-    public GracefulShutdownHandler()
+    public GracefulHandler()
     {
         shutdown = new Shutdown(this)
         {

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

@@ -13,230 +13,200 @@
 
 package org.eclipse.jetty.server.handler;
 
-import java.io.IOException;
+import java.net.InetSocketAddress;
-import java.net.HttpURLConnection;
+import java.net.SocketAddress;
-import java.net.SocketException;
+import java.util.concurrent.CompletableFuture;
-import java.net.URL;
 
-import org.eclipse.jetty.server.Connector;
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.http.HttpStatus;
+import org.eclipse.jetty.io.Content;
 import org.eclipse.jetty.server.Handler;
-import org.eclipse.jetty.server.NetworkConnector;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.Response;
 import org.eclipse.jetty.util.Callback;
+import org.eclipse.jetty.util.Fields;
+import org.eclipse.jetty.util.StringUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
- * A handler that shuts the server down on a valid request. Used to do "soft" restarts from Java.
+ * <p>
- * If _exitJvm is set to true a hard System.exit() call is being made.
+ * A {@link Handler} that initiates a Shutdown of the Jetty Server it belongs to.
- * If _sendShutdownAtStart is set to true, starting the server will try to shut down an existing server at the same port.
+ * </p>
- * If _sendShutdownAtStart is set to true, make an http call to
- * "http://localhost:" + port + "/shutdown?token=" + shutdownCookie
- * in order to shut down the server.
  *
- * This handler is a contribution from Johannes Brodwall: https://bugs.eclipse.org/bugs/show_bug.cgi?id=357687
+ * <p>
+ * Used to trigger shutdown of a Jetty Server instance
+ * <ul>
+ *    <li>If {@code exitJvm} is set to true a hard {@link System#exit(int)} call will be performed.</li>
+ * </ul>
  *
- * Usage:
+ * Server Setup Example:
  *
- * <pre>
+ * <pre>{@code
  * Server server = new Server(8080);
- * ShutdownHandler shutdown = new ShutdownHandler(&quot;secret password&quot;, false, true) });
+ * String shutdownToken = "secret password";
- * server.setHandler(shutdown);
+ * boolean exitJvm = false;
+ * ShutdownHandler shutdown = new ShutdownHandler(shutdownToken, exitJvm));
  * shutdown.setHandler(someOtherHandler);
+ * server.setHandler(someOtherHandlers);
  * server.start();
- * </pre>
+ * }</pre>
  *
- * <pre>
+ * Client Triggering Example
- * public static void attemptShutdown(int port, String shutdownCookie) {
+ *
+ * <pre>{@code
+ * public static void attemptShutdown(int port, String shutdownToken) {
  *   try {
- * URL url = new URL("http://localhost:" + port + "/shutdown?token=" + shutdownCookie);
+ *     String encodedToken = URLEncoder.encode(shutdownToken);
- * HttpURLConnection connection = (HttpURLConnection)url.openConnection();
+ *     URI uri = URI.create("http://localhost:%d/shutdown?token=%s".formatted(port, shutdownCookie));
- * connection.setRequestMethod("POST");
+ *     HttpClient httpClient = HttpClient.newBuilder().build();
- * connection.getResponseCode();
+ *     HttpRequest httpRequest = HttpRequest.newBuilder(shutdownURI)
- * logger.info("Shutting down " + url + ": " + connection.getResponseMessage());
+ *         .POST(HttpRequest.BodyPublishers.noBody())
- * } catch (SocketException e) {
+ *         .build();
- * logger.debug("Not running");
+ *     HttpResponse<String> httpResponse = httpClient.send(httpRequest,
+ *         HttpResponse.BodyHandlers.ofString(StandardCharsets.UTF_8));
+ *     Assertions.assertEquals(200, httpResponse.statusCode());
+ *     System.out.println(httpResponse.body());
+ *     logger.info("Shutting down " + uri + ": " + httpResponse.body());
+ *   } catch (IOException | InterruptedException e) {
+ *     logger.debug("Shutdown Handler not available");
  *     // Okay - the server is not running
- * } catch (IOException e) {
  *     throw new RuntimeException(e);
  *   }
  * }
- * </pre>
+ * }</pre>
  */
 public class ShutdownHandler extends Handler.Wrapper
 {
     private static final Logger LOG = LoggerFactory.getLogger(ShutdownHandler.class);
 
+    private final String _shutdownPath;
     private final String _shutdownToken;
-    private boolean _sendShutdownAtStart;
     private boolean _exitJvm = false;
 
     /**
-     * Creates a listener that lets the server be shut down remotely (but only from localhost).
+     * Creates a Handler that lets the server be shut down remotely (but only from localhost).
      *
      * @param shutdownToken a secret password to avoid unauthorized shutdown attempts
      */
     public ShutdownHandler(String shutdownToken)
     {
-        this(shutdownToken, false, false);
+        this(null, shutdownToken, false);
     }
 
     /**
+     * Creates a Handler that lets the server be shut down remotely (but only from localhost).
+     *
      * @param shutdownToken a secret password to avoid unauthorized shutdown attempts
      * @param exitJVM If true, when the shutdown is executed, the handler class System.exit()
-     * @param sendShutdownAtStart If true, a shutdown is sent as an HTTP post
-     * during startup, which will shutdown any previously running instances of
-     * this server with an identically configured ShutdownHandler
      */
-    public ShutdownHandler(String shutdownToken, boolean exitJVM, boolean sendShutdownAtStart)
+    public ShutdownHandler(String shutdownToken, boolean exitJVM)
     {
+        this(null, shutdownToken, exitJVM);
+    }
+
+    /**
+     * Creates a Handler that lets the server be shut down remotely (but only from localhost).
+     *
+     * @param shutdownPath the path to respond to shutdown requests against (default is "{@code /shutdown}")
+     * @param shutdownToken a secret password to avoid unauthorized shutdown attempts
+     * @param exitJVM If true, when the shutdown is executed, the handler class System.exit()
+     */
+    public ShutdownHandler(String shutdownPath, String shutdownToken, boolean exitJVM)
+    {
+        this._shutdownPath = StringUtil.isBlank(shutdownPath) ? "/shutdown" : shutdownPath;
         this._shutdownToken = shutdownToken;
-        /* TODO
+        this._exitJvm = exitJVM;
-        setExitJvm(exitJVM);
-        setSendShutdownAtStart(sendShutdownAtStart);
-
-         */
-    }
-
-    public void sendShutdown() throws IOException
-    {
-        URL url = new URL(getServerUrl() + "/shutdown?token=" + _shutdownToken);
-        try
-        {
-            HttpURLConnection connection = (HttpURLConnection)url.openConnection();
-            connection.setRequestMethod("POST");
-            connection.getResponseCode();
-            LOG.info("Shutting down {}: {} {}", url, connection.getResponseCode(), connection.getResponseMessage());
-        }
-        catch (SocketException e)
-        {
-            LOG.debug("Not running");
-            // Okay - the server is not running
-        }
-        catch (IOException e)
-        {
-            throw new RuntimeException(e);
-        }
-    }
-
-    @SuppressWarnings("resource")
-    private String getServerUrl()
-    {
-        NetworkConnector connector = null;
-        for (Connector c : getServer().getConnectors())
-        {
-            if (c instanceof NetworkConnector)
-            {
-                connector = (NetworkConnector)c;
-                break;
-            }
-        }
-
-        if (connector == null)
-            return "http://localhost";
-
-        return "http://localhost:" + connector.getPort();
-    }
-
-    @Override
-    protected void doStart() throws Exception
-    {
-        super.doStart();
-        if (_sendShutdownAtStart)
-            sendShutdown();
     }
 
     @Override
     public boolean process(Request request, Response response, Callback callback) throws Exception
     {
-        return super.process(request, response, callback);
+        String fullPath = request.getHttpURI().getCanonicalPath();
-        /* TODO
+        ContextHandler contextHandler = ContextHandler.getContextHandler(request);
-        if (!target.equals("/shutdown"))
+        if (contextHandler != null)
         {
-            super.handle(target, baseRequest, request, response);
+            // We are operating in a context, so use it
-            return;
+            String pathInContext = contextHandler.getContext().getPathInContext(fullPath);
+            if (!pathInContext.startsWith(this._shutdownPath))
+            {
+                return super.process(request, response, callback);
+            }
+        }
+        else
+        {
+            // We are standalone
+            if (!fullPath.startsWith(this._shutdownPath))
+            {
+                return super.process(request, response, callback);
+            }
         }
 
         if (!request.getMethod().equals("POST"))
         {
-            response.sendError(HttpServletResponse.SC_BAD_REQUEST);
+            Response.writeError(request, response, callback, HttpStatus.BAD_REQUEST_400);
-            return;
+            return true;
         }
+
         if (!hasCorrectSecurityToken(request))
         {
-            LOG.warn("Unauthorized tokenless shutdown attempt from {}", request.getRemoteAddr());
+            LOG.warn("Unauthorized tokenless shutdown attempt from {}", request.getConnectionMetaData().getRemoteSocketAddress());
-            response.sendError(HttpServletResponse.SC_UNAUTHORIZED);
+            Response.writeError(request, response, callback, HttpStatus.UNAUTHORIZED_401);
-            return;
+            return true;
         }
-        if (!requestFromLocalhost(baseRequest))
+        if (!requestFromLocalhost(request))
         {
-            LOG.warn("Unauthorized non-loopback shutdown attempt from {}", request.getRemoteAddr());
+            LOG.warn("Unauthorized non-loopback shutdown attempt from {}", request.getConnectionMetaData().getRemoteSocketAddress());
-            response.sendError(HttpServletResponse.SC_UNAUTHORIZED);
+            Response.writeError(request, response, callback, HttpStatus.UNAUTHORIZED_401);
-            return;
+            return true;
         }
 
-        LOG.info("Shutting down by request from {}", request.getRemoteAddr());
+        LOG.info("Shutting down by request from {}", request.getConnectionMetaData().getRemoteSocketAddress());
-        doShutdown(baseRequest, response);
+        // Establish callback to trigger server shutdown when write of response is complete
-
+        Callback triggerShutdownCallback = Callback.from(() ->
-         */
-    }
-
-    /* TODO
-    protected void doShutdown(Request baseRequest, HttpServletResponse response) throws IOException
         {
-        for (Connector connector : getServer().getConnectors())
+            CompletableFuture.runAsync(this::shutdownServer);
-        {
+        });
-            connector.shutdown();
+        response.getHeaders().put(HttpHeader.CONTENT_TYPE, "text/plain, charset=utf-8");
-        }
+        String message = "Shutdown triggered";
-
+        Content.Sink.write(response, true, message, triggerShutdownCallback);
-        baseRequest.setHandled(true);
+        return true;
-        response.setStatus(200);
-        response.flushBuffer();
-
-        final Server server = getServer();
-        new Thread()
-        {
-            @Override
-            public void run()
-            {
-                try
-                {
-                    shutdownServer(server);
-                }
-                catch (InterruptedException e)
-                {
-                    LOG.trace("IGNORED", e);
-                }
-                catch (Exception e)
-                {
-                    throw new RuntimeException("Shutting down server", e);
-                }
-            }
-        }.start();
     }
 
     private boolean requestFromLocalhost(Request request)
     {
-        InetSocketAddress addr = request.getRemoteInetSocketAddress();
+        SocketAddress socketAddress = request.getConnectionMetaData().getRemoteSocketAddress();
-        if (addr == null)
+        if (socketAddress == null)
-        {
+            return false;
+
+        if (socketAddress instanceof InetSocketAddress addr)
+            return addr.getAddress().isLoopbackAddress();
+
         return false;
     }
-        return addr.getAddress().isLoopbackAddress();
-    }
 
-    private boolean hasCorrectSecurityToken(HttpServletRequest request)
+    private boolean hasCorrectSecurityToken(Request request)
     {
-        String tok = request.getParameter("token");
+        Fields fields = Request.extractQueryParameters(request);
+        String tok = fields.getValue("token");
         if (LOG.isDebugEnabled())
             LOG.debug("Token: {}", tok);
         return _shutdownToken.equals(tok);
     }
 
-    private void shutdownServer(Server server) throws Exception
+    private void shutdownServer()
     {
-        server.stop();
+        try
+        {
+            // Let server stop normally.
+            // Order of stop is controlled by server.
+            // Graceful stop can even be configured at the Server level
+            getServer().stop();
+        }
+        catch (Exception e)
+        {
+            LOG.warn("Unable to stop server", e);
+        }
 
         if (_exitJvm)
         {
@@ -249,25 +219,8 @@ public class ShutdownHandler extends Handler.Wrapper
         this._exitJvm = exitJvm;
     }
 
-    public boolean isSendShutdownAtStart()
-    {
-        return _sendShutdownAtStart;
-    }
-
-    public void setSendShutdownAtStart(boolean sendShutdownAtStart)
-    {
-        _sendShutdownAtStart = sendShutdownAtStart;
-    }
-
-    public String getShutdownToken()
-    {
-        return _shutdownToken;
-    }
-
     public boolean isExitJvm()
     {
         return _exitJvm;
     }
-
-     */
 }

jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/GracefulHandlerTest.java

@@ -29,7 +29,7 @@ import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.HttpTester;
 import org.eclipse.jetty.io.Content;
 import org.eclipse.jetty.logging.StacklessLogging;
-import org.eclipse.jetty.server.handler.GracefulShutdownHandler;
+import org.eclipse.jetty.server.handler.GracefulHandler;
 import org.eclipse.jetty.util.Blocker;
 import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.util.NanoTime;
@@ -48,9 +48,9 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-public class GracefulShutdownTest
+public class GracefulHandlerTest
 {
-    private static final Logger LOG = LoggerFactory.getLogger(GracefulShutdownTest.class);
+    private static final Logger LOG = LoggerFactory.getLogger(GracefulHandlerTest.class);
     private Server server;
     private ServerConnector connector;
 
@@ -113,13 +113,13 @@ public class GracefulShutdownTest
     /**
      * Test for when a Handler throws an unhandled Exception from {@link Handler#process(Request, Response, Callback)}
      * when in normal mode (not during graceful mode).  This test exists to ensure that the Callback management of
-     * the {@link GracefulShutdownHandler} doesn't mess with normal operations of requests.
+     * the {@link GracefulHandler} doesn't mess with normal operations of requests.
      */
     @Test
     public void testHandlerNormalUnhandledException() throws Exception
     {
-        GracefulShutdownHandler gracefulShutdownHandler = new GracefulShutdownHandler();
+        GracefulHandler gracefulHandler = new GracefulHandler();
-        gracefulShutdownHandler.setHandler(new Handler.Abstract()
+        gracefulHandler.setHandler(new Handler.Abstract()
         {
             @Override
             public boolean process(Request request, Response response, Callback callback) throws Exception
@@ -127,7 +127,7 @@ public class GracefulShutdownTest
                 throw new RuntimeException("Intentional Exception");
             }
         });
-        server = createServer(gracefulShutdownHandler);
+        server = createServer(gracefulHandler);
         server.setStopTimeout(10000);
         server.start();
 
@@ -175,8 +175,8 @@ public class GracefulShutdownTest
     public void testHandlerGracefulUnhandledException() throws Exception
     {
         CountDownLatch dispatchLatch = new CountDownLatch(1);
-        GracefulShutdownHandler gracefulShutdownHandler = new GracefulShutdownHandler();
+        GracefulHandler gracefulHandler = new GracefulHandler();
-        gracefulShutdownHandler.setHandler(new Handler.Abstract()
+        gracefulHandler.setHandler(new Handler.Abstract()
         {
             @Override
             public boolean process(Request request, Response response, Callback callback) throws Exception
@@ -185,11 +185,11 @@ public class GracefulShutdownTest
                 // let main thread know that we've reach this handler
                 dispatchLatch.countDown();
                 // now wait for graceful stop to begin
-                await().atMost(5, TimeUnit.SECONDS).until(() -> gracefulShutdownHandler.isShutdown());
+                await().atMost(5, TimeUnit.SECONDS).until(() -> gracefulHandler.isShutdown());
                 throw new RuntimeException("Intentional Failure");
             }
         });
-        server = createServer(gracefulShutdownHandler);
+        server = createServer(gracefulHandler);
         server.setStopTimeout(10000);
         server.start();
 
@@ -234,14 +234,14 @@ public class GracefulShutdownTest
 
     /**
      * Test for when a Handler uses {@link Callback#failed(Throwable)} when in normal mode (not during graceful mode).
-     * This test exists to ensure that the Callback management of the {@link GracefulShutdownHandler} doesn't
+     * This test exists to ensure that the Callback management of the {@link GracefulHandler} doesn't
      * mess with normal operations of requests.
      */
     @Test
     public void testHandlerNormalCallbackFailure() throws Exception
     {
-        GracefulShutdownHandler gracefulShutdownHandler = new GracefulShutdownHandler();
+        GracefulHandler gracefulHandler = new GracefulHandler();
-        gracefulShutdownHandler.setHandler(new Handler.Abstract()
+        gracefulHandler.setHandler(new Handler.Abstract()
         {
             @Override
             public boolean process(Request request, Response response, Callback callback) throws Exception
@@ -250,7 +250,7 @@ public class GracefulShutdownTest
                 return true;
             }
         });
-        server = createServer(gracefulShutdownHandler);
+        server = createServer(gracefulHandler);
         server.setStopTimeout(10000);
         server.start();
 
@@ -298,20 +298,20 @@ public class GracefulShutdownTest
     public void testHandlerGracefulCallbackFailure() throws Exception
     {
         CountDownLatch dispatchLatch = new CountDownLatch(1);
-        GracefulShutdownHandler gracefulShutdownHandler = new GracefulShutdownHandler();
+        GracefulHandler gracefulHandler = new GracefulHandler();
-        gracefulShutdownHandler.setHandler(new Handler.Abstract()
+        gracefulHandler.setHandler(new Handler.Abstract()
         {
             @Override
             public boolean process(Request request, Response response, Callback callback) throws Exception
             {
                 dispatchLatch.countDown();
                 // wait for graceful to kick in
-                await().atMost(5, TimeUnit.SECONDS).until(() -> gracefulShutdownHandler.isShutdown());
+                await().atMost(5, TimeUnit.SECONDS).until(() -> gracefulHandler.isShutdown());
                 callback.failed(new RuntimeException("Intentional Failure"));
                 return true;
             }
         });
-        server = createServer(gracefulShutdownHandler);
+        server = createServer(gracefulHandler);
         server.setStopTimeout(10000);
         server.start();
 
@@ -357,14 +357,14 @@ public class GracefulShutdownTest
     /**
      * Test for when a Handler returns false from {@link Handler#process(Request, Response, Callback)}
      * when in normal mode (not during graceful mode).
-     * This test exists to ensure that the Callback management of the {@link GracefulShutdownHandler} doesn't
+     * This test exists to ensure that the Callback management of the {@link GracefulHandler} doesn't
      * mess with normal operations of requests.
      */
     @Test
     public void testHandlerNormalProcessingFalse() throws Exception
     {
-        GracefulShutdownHandler gracefulShutdownHandler = new GracefulShutdownHandler();
+        GracefulHandler gracefulHandler = new GracefulHandler();
-        gracefulShutdownHandler.setHandler(new Handler.Abstract()
+        gracefulHandler.setHandler(new Handler.Abstract()
         {
             @Override
             public boolean process(Request request, Response response, Callback callback) throws Exception
@@ -372,7 +372,7 @@ public class GracefulShutdownTest
                 return false;
             }
         });
-        server = createServer(gracefulShutdownHandler);
+        server = createServer(gracefulHandler);
         server.setStopTimeout(10000);
         server.start();
 
@@ -416,18 +416,18 @@ public class GracefulShutdownTest
     public void testHandlerGracefulProcessingFalse() throws Exception
     {
         AtomicReference<CompletableFuture<Long>> stopFuture = new AtomicReference<>();
-        GracefulShutdownHandler gracefulShutdownHandler = new GracefulShutdownHandler();
+        GracefulHandler gracefulHandler = new GracefulHandler();
-        gracefulShutdownHandler.setHandler(new Handler.Abstract()
+        gracefulHandler.setHandler(new Handler.Abstract()
         {
             @Override
             public boolean process(Request request, Response response, Callback callback) throws Exception
             {
                 stopFuture.set(runAsyncServerStop());
-                await().atMost(5, TimeUnit.SECONDS).until(() -> gracefulShutdownHandler.isShutdown());
+                await().atMost(5, TimeUnit.SECONDS).until(() -> gracefulHandler.isShutdown());
                 return false;
             }
         });
-        server = createServer(gracefulShutdownHandler);
+        server = createServer(gracefulHandler);
         server.setStopTimeout(10000);
         server.start();
 
@@ -471,8 +471,8 @@ public class GracefulShutdownTest
     public void testHandlerGracefulBlocked() throws Exception
     {
         CountDownLatch dispatchedToHandlerLatch = new CountDownLatch(1);
-        GracefulShutdownHandler gracefulShutdownHandler = new GracefulShutdownHandler();
+        GracefulHandler gracefulHandler = new GracefulHandler();
-        gracefulShutdownHandler.setHandler(new BlockingReadHandler()
+        gracefulHandler.setHandler(new BlockingReadHandler()
         {
             @Override
             protected void onBeforeRead(Request request, Response response)
@@ -480,7 +480,7 @@ public class GracefulShutdownTest
                 dispatchedToHandlerLatch.countDown();
             }
         });
-        server = createServer(gracefulShutdownHandler);
+        server = createServer(gracefulHandler);
         server.setStopTimeout(10000);
         server.start();
 
@@ -508,7 +508,7 @@ public class GracefulShutdownTest
         CompletableFuture<Long> stopFuture = runAsyncServerStop();
 
         // Wait till we enter graceful mode
-        await().atMost(5, TimeUnit.SECONDS).until(() -> gracefulShutdownHandler.isShutdown());
+        await().atMost(5, TimeUnit.SECONDS).until(() -> gracefulHandler.isShutdown());
 
         // Send rest of data
         output0.write("67890".getBytes(StandardCharsets.UTF_8));
@@ -544,8 +544,8 @@ public class GracefulShutdownTest
     public void testHandlerGracefulBlockedEarlyCommit() throws Exception
     {
         CountDownLatch dispatchedToHandlerLatch = new CountDownLatch(1);
-        GracefulShutdownHandler gracefulShutdownHandler = new GracefulShutdownHandler();
+        GracefulHandler gracefulHandler = new GracefulHandler();
-        gracefulShutdownHandler.setHandler(new BlockingReadHandler()
+        gracefulHandler.setHandler(new BlockingReadHandler()
         {
             @Override
             protected void onBeforeRead(Request request, Response response) throws Exception
@@ -560,7 +560,7 @@ public class GracefulShutdownTest
                 dispatchedToHandlerLatch.countDown();
             }
         });
-        server = createServer(gracefulShutdownHandler);
+        server = createServer(gracefulHandler);
         server.setStopTimeout(10000);
         server.start();
 
@@ -588,7 +588,7 @@ public class GracefulShutdownTest
         CompletableFuture<Long> stopFuture = runAsyncServerStop();
 
         // Wait till we enter graceful mode
-        await().atMost(5, TimeUnit.SECONDS).until(() -> gracefulShutdownHandler.isShutdown());
+        await().atMost(5, TimeUnit.SECONDS).until(() -> gracefulHandler.isShutdown());
 
         // Send rest of data
         output0.write("67890".getBytes(StandardCharsets.UTF_8));
@@ -614,15 +614,15 @@ public class GracefulShutdownTest
     }
 
     /**
-     * Test of how the {@link GracefulShutdownHandler} should behave if it
+     * Test of how the {@link GracefulHandler} should behave if it
      * receives a request on an active connection after graceful starts.
      */
     @Test
     public void testRequestAfterGraceful() throws Exception
     {
-        GracefulShutdownHandler gracefulShutdownHandler = new GracefulShutdownHandler();
+        GracefulHandler gracefulHandler = new GracefulHandler();
-        gracefulShutdownHandler.setHandler(new BlockingReadHandler());
+        gracefulHandler.setHandler(new BlockingReadHandler());
-        server = createServer(gracefulShutdownHandler);
+        server = createServer(gracefulHandler);
         server.setStopTimeout(10000);
         server.start();
 
@@ -655,7 +655,7 @@ public class GracefulShutdownTest
         CompletableFuture<Long> stopFuture = runAsyncServerStop();
 
         // Wait till we enter graceful mode
-        await().atMost(5, TimeUnit.SECONDS).until(() -> gracefulShutdownHandler.isShutdown());
+        await().atMost(5, TimeUnit.SECONDS).until(() -> gracefulHandler.isShutdown());
 
         // Send another request on same connection
         output0.write(rawRequest.formatted(2).getBytes(StandardCharsets.UTF_8));

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

@@ -13,53 +13,69 @@
 
 package org.eclipse.jetty.server.handler;
 
-import java.io.IOException;
+import java.io.InputStream;
 import java.io.OutputStream;
+import java.net.InetSocketAddress;
 import java.net.Socket;
+import java.net.SocketAddress;
+import java.net.URI;
+import java.net.URLEncoder;
 import java.nio.charset.StandardCharsets;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.HttpTester;
+import org.eclipse.jetty.server.ConnectionMetaData;
 import org.eclipse.jetty.server.Handler;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.Response;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.ServerConnector;
+import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.util.component.AbstractLifeCycle;
 import org.eclipse.jetty.util.component.LifeCycle;
-import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-@Disabled // TODO
 public class ShutdownHandlerTest
 {
     private Server server;
-    private ServerConnector connector;
-    private String shutdownToken = "asdlnsldgnklns";
 
-    public void start(Handler.Wrapper wrapper) throws Exception
+    public void createServer(Handler handler) throws Exception
     {
         server = new Server();
-        connector = new ServerConnector(server);
+        ServerConnector connector = new ServerConnector(server);
+        connector.setPort(0);
         server.addConnector(connector);
-        Handler shutdown = new ShutdownHandler(shutdownToken);
-        Handler handler = shutdown;
-        if (wrapper != null)
-        {
-            wrapper.setHandler(shutdown);
-            handler = wrapper;
-        }
         server.setHandler(handler);
         server.start();
     }
 
-    @Test
+    @AfterEach
-    public void testShutdownServerWithCorrectTokenAndIP() throws Exception
+    public void teardown()
     {
-        start(null);
+        LifeCycle.stop(server);
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {"abcdefg", "a token with space", "euro-€-token"})
+    public void testShutdownServerWithCorrectTokenAndFromLocalhost(String shutdownToken) throws Exception
+    {
+        ShutdownHandler shutdownHandler = new ShutdownHandler(shutdownToken);
+        shutdownHandler.setHandler(new EchoHandler());
+
+        InetSocketAddress fakeRemoteAddr = new InetSocketAddress("127.0.0.1", 22033);
+        Handler.Wrapper fakeRemoteAddressHandler = new FakeRemoteAddressHandlerWrapper(fakeRemoteAddr);
+        fakeRemoteAddressHandler.setHandler(shutdownHandler);
+
+        createServer(fakeRemoteAddressHandler);
+        server.start();
 
         CountDownLatch stopLatch = new CountDownLatch(1);
         server.addEventListener(new AbstractLifeCycle.AbstractLifeCycleListener()
@@ -71,7 +87,7 @@ public class ShutdownHandlerTest
             }
         });
 
-        HttpTester.Response response = shutdown(shutdownToken);
+        HttpTester.Response response = sendShutdownRequest(shutdownToken);
         assertEquals(HttpStatus.OK_200, response.getStatus());
 
         assertTrue(stopLatch.await(5, TimeUnit.SECONDS));
@@ -81,9 +97,13 @@ public class ShutdownHandlerTest
     @Test
     public void testWrongToken() throws Exception
     {
-        start(null);
+        String shutdownToken = "abcdefg";
+        ShutdownHandler shutdownHandler = new ShutdownHandler(shutdownToken);
+        shutdownHandler.setHandler(new EchoHandler());
+        createServer(shutdownHandler);
+        server.start();
 
-        HttpTester.Response response = shutdown("wrongToken");
+        HttpTester.Response response = sendShutdownRequest("wrongToken");
         assertEquals(HttpStatus.UNAUTHORIZED_401, response.getStatus());
 
         Thread.sleep(1000);
@@ -93,40 +113,103 @@ public class ShutdownHandlerTest
     @Test
     public void testShutdownRequestNotFromLocalhost() throws Exception
     {
-        /* TODO
+        String shutdownToken = "abcdefg";
-        start(new Handler.Wrapper()
-        {
-            @Override
-            public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
-            {
-                baseRequest.setRemoteAddr(new InetSocketAddress("192.168.0.1", 12345));
-                super.handle(target, baseRequest, request, response);
-            }
-        });
 
-         */
+        ShutdownHandler shutdownHandler = new ShutdownHandler(shutdownToken);
+        shutdownHandler.setHandler(new EchoHandler());
 
-        HttpTester.Response response = shutdown(shutdownToken);
+        InetSocketAddress fakeRemoteAddr = new InetSocketAddress("192.168.0.1", 12345);
+        Handler.Wrapper fakeRemoteAddressHandler = new FakeRemoteAddressHandlerWrapper(fakeRemoteAddr);
+        fakeRemoteAddressHandler.setHandler(shutdownHandler);
+
+        createServer(fakeRemoteAddressHandler);
+        server.start();
+
+        HttpTester.Response response = sendShutdownRequest(shutdownToken);
         assertEquals(HttpStatus.UNAUTHORIZED_401, response.getStatus());
 
         Thread.sleep(1000);
         assertEquals(AbstractLifeCycle.STARTED, server.getState());
     }
 
-    private HttpTester.Response shutdown(String shutdownToken) throws IOException
+    private HttpTester.Response sendShutdownRequest(String shutdownToken) throws Exception
     {
-        try (Socket socket = new Socket("localhost", connector.getLocalPort()))
+        URI shutdownUri = server.getURI().resolve("/shutdown?token=" + URLEncoder.encode(shutdownToken, StandardCharsets.UTF_8));
+        try (Socket client = new Socket(shutdownUri.getHost(), shutdownUri.getPort());
+             OutputStream output = client.getOutputStream();
+             InputStream input = client.getInputStream())
         {
-            String request =
+            String rawRequest = """
-                "POST /shutdown?token=" + shutdownToken + " HTTP/1.1\r\n" +
+                POST %s?%s HTTP/1.1
-                    "Host: localhost\r\n" +
+                Host: %s:%d
-                    "\r\n";
+                Connection: close
-            OutputStream output = socket.getOutputStream();
+                Content-Length: 0
-            output.write(request.getBytes(StandardCharsets.UTF_8));
+                                
+                """.formatted(shutdownUri.getRawPath(), shutdownUri.getRawQuery(), shutdownUri.getHost(), shutdownUri.getPort());
+
+            output.write(rawRequest.getBytes(StandardCharsets.UTF_8));
             output.flush();
 
-            HttpTester.Input input = HttpTester.from(socket.getInputStream());
+            HttpTester.Response response = HttpTester.parseResponse(input);
-            return HttpTester.parseResponse(input);
+            return response;
+        }
+    }
+
+    static class FakeRemoteAddressHandlerWrapper extends Handler.Wrapper
+    {
+        private final InetSocketAddress fakeRemoteAddress;
+
+        public FakeRemoteAddressHandlerWrapper(InetSocketAddress fakeRemoteAddress)
+        {
+            super();
+            this.fakeRemoteAddress = fakeRemoteAddress;
+        }
+
+        @Override
+        public boolean process(Request request, Response response, Callback callback) throws Exception
+        {
+            Request fakedRequest = FakeRemoteAddressRequest.from(request, this.fakeRemoteAddress);
+            return super.process(fakedRequest, response, callback);
+        }
+    }
+
+    static class FakeRemoteAddressConnectionMetadata extends ConnectionMetaData.Wrapper
+    {
+        private final InetSocketAddress fakeRemoteAddress;
+
+        public FakeRemoteAddressConnectionMetadata(ConnectionMetaData wrapped, InetSocketAddress fakeRemoteAddress)
+        {
+            super(wrapped);
+            this.fakeRemoteAddress = fakeRemoteAddress;
+        }
+
+        @Override
+        public SocketAddress getRemoteSocketAddress()
+        {
+            return this.fakeRemoteAddress;
+        }
+    }
+
+    static class FakeRemoteAddressRequest extends Request.Wrapper
+    {
+        private final ConnectionMetaData fakeConnectionMetaData;
+
+        public static Request from(Request request, InetSocketAddress fakeRemoteAddress)
+        {
+            ConnectionMetaData fakeRemoteConnectionMetadata = new FakeRemoteAddressConnectionMetadata(request.getConnectionMetaData(), fakeRemoteAddress);
+            return new FakeRemoteAddressRequest(request, fakeRemoteConnectionMetadata);
+        }
+
+        public FakeRemoteAddressRequest(Request wrapped, ConnectionMetaData fakeRemoteConnectionMetadata)
+        {
+            super(wrapped);
+            this.fakeConnectionMetaData = fakeRemoteConnectionMetadata;
+        }
+
+        @Override
+        public ConnectionMetaData getConnectionMetaData()
+        {
+            return this.fakeConnectionMetaData;
         }
     }
 }