Issue #14403 - Defer FCGI application dispatch until the parser returns

jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/HttpStreamOverFCGI.java

@@ -103,17 +103,15 @@ else if (FCGI.Headers.HTTPS.equalsIgnoreCase(name))
             processField(field);
     }
 
-    public void onHeaders()
+    public Runnable onHeaders()
     {
         String pathQuery = URIUtil.addPathQuery(_path, _query);
         HttpScheme scheme = StringUtil.isEmpty(_secure) ? HttpScheme.HTTP : HttpScheme.HTTPS;
         MetaData.Request request = new MetaData.Request(_connection.getBeginNanoTime(), _method, scheme.asString(), hostPort, pathQuery, HttpVersion.fromString(_version), _headers, -1);
         Runnable task = _httpChannel.onRequest(request);
         _allHeaders.forEach(field -> _httpChannel.getRequest().setAttribute(field.getName(), field.getValue()));
-        // TODO: here we just execute the task.
-        //  However, we should really return all the way back to onFillable()
-        //  and feed the Runnable to an ExecutionStrategy.
-        execute(task);
+        // Return the task to dispatch it after ServerParser.parse() has returned.
+        return task;
     }
 
     private void processField(HttpField field)
@@ -358,11 +356,6 @@ public boolean onIdleTimeout(TimeoutException timeout)
         return !handlingRequest;
     }
 
-    private void execute(Runnable task)
-    {
-        _connection.getConnector().getExecutor().execute(task);
-    }
-
     private class DemandCallback implements Callback
     {
         @Override

jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java

@@ -59,6 +59,7 @@ public class ServerFCGIConnection extends AbstractMetaDataConnection implements
     private boolean useOutputDirectByteBuffers;
     private RetainableByteBuffer inputBuffer;
     private HttpStreamOverFCGI stream;
+    private Runnable onRequest;
 
     public ServerFCGIConnection(Connector connector, EndPoint endPoint, HttpConfiguration configuration, boolean sendStatus200)
     {
@@ -191,6 +192,10 @@ public void onFillable()
                     {
                         if (stream == null && inputBuffer.isEmpty())
                             releaseInputBuffer();
+                        Runnable task = onRequest;
+                        onRequest = null;
+                        if (task != null)
+                            getExecutor().execute(task);
                         return;
                     }
                 }
@@ -359,9 +364,8 @@ public boolean onHeaders(int request)
                 LOG.debug("Request {} headers on {}", request, stream);
             if (stream != null)
             {
-                stream.onHeaders();
-                // We have dispatched to the application,
-                // so we must stop the fill & parse loop.
+                onRequest = stream.onHeaders();
+                // Return to onFillable() before dispatching to the application.
                 return true;
             }
             return false;

jetty-core/jetty-fcgi/jetty-fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnectionTest.java

@@ -0,0 +1,85 @@
+//
+// ========================================================================
+// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.fcgi.server.internal;
+
+import java.nio.ByteBuffer;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.eclipse.jetty.fcgi.FCGI;
+import org.eclipse.jetty.fcgi.generator.ClientGenerator;
+import org.eclipse.jetty.fcgi.server.ServerFCGIConnectionFactory;
+import org.eclipse.jetty.http.HttpFields;
+import org.eclipse.jetty.http.HttpVersion;
+import org.eclipse.jetty.io.ArrayByteBufferPool;
+import org.eclipse.jetty.io.ByteArrayEndPoint;
+import org.eclipse.jetty.io.ByteBufferPool;
+import org.eclipse.jetty.server.Handler;
+import org.eclipse.jetty.server.HttpConfiguration;
+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.BufferUtil;
+import org.eclipse.jetty.util.Callback;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class ServerFCGIConnectionTest
+{
+    @Test
+    public void testApplicationDispatchedAfterParsing()
+    {
+        AtomicInteger handlersInvoked = new AtomicInteger();
+        Server server = new Server();
+        server.setHandler(new Handler.Abstract()
+        {
+            @Override
+            public boolean handle(Request request, Response response, Callback callback)
+            {
+                handlersInvoked.incrementAndGet();
+                callback.succeeded();
+                return true;
+            }
+        });
+
+        ArrayByteBufferPool.Tracking bufferPool = new ArrayByteBufferPool.Tracking();
+        ServerFCGIConnectionFactory connectionFactory = new ServerFCGIConnectionFactory(new HttpConfiguration());
+        ServerConnector connector = new ServerConnector(server, Runnable::run, null, bufferPool, 0, 1, connectionFactory);
+        ByteArrayEndPoint endPoint = new ByteArrayEndPoint(new byte[0], 64 * 1024);
+        ServerFCGIConnection connection = new ServerFCGIConnection(connector, endPoint, new HttpConfiguration(), false);
+
+        ClientGenerator generator = new ClientGenerator(ByteBufferPool.NON_POOLING);
+        ByteBufferPool.Accumulator accumulator = new ByteBufferPool.Accumulator();
+        HttpFields.Mutable params = HttpFields.build()
+            .put(FCGI.Headers.REQUEST_METHOD, "GET")
+            .put(FCGI.Headers.DOCUMENT_URI, "/")
+            .put(FCGI.Headers.QUERY_STRING, "")
+            .put(FCGI.Headers.SERVER_PROTOCOL, HttpVersion.HTTP_1_1.asString());
+        generator.generateRequestHeaders(accumulator, 1, params);
+        generator.generateRequestContent(accumulator, 1, BufferUtil.EMPTY_BUFFER, true);
+        List<ByteBuffer> buffers = accumulator.getByteBuffers();
+        ByteBuffer request = ByteBuffer.allocate((int)accumulator.getTotalLength());
+        buffers.forEach(request::put);
+        accumulator.release();
+        BufferUtil.flipToFlush(request, 0);
+
+        endPoint.addInput(request);
+        connection.onFillable();
+
+        assertEquals(1, handlersInvoked.get());
+        assertEquals(0, bufferPool.getLeaks().size(), bufferPool.dumpLeaks());
+    }
+}