Jetty 12.0.x ee9 serverpush tck (#9902)

* Make a test case to resemble a failing tck test

* Add more test cases more like the tck test

* Fixed processing of EOF frames for pushed requests.

Before, the EOF frame was not added to the HTTP2Stream queue.
When the pushed request was completing it tried to consume the available content, but it was not finding EOF and so was emitting a reset frame, causing the TCK failure.

Now the EOF frame is always added to the HTTP2Stream queue, so it is properly consumed when completing the pushed request.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>

* Fix H2 trailers frames

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

---------

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
Co-authored-by: Simone Bordet <simone.bordet@gmail.com>
Co-authored-by: Ludovic Orban <lorban@bitronix.be>

jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java

@@ -437,16 +437,12 @@ public class HTTP2Stream implements Stream, Attachable, Closeable, Callback, Dum
             }
         }
 
-        if (getListener() != null)
+        boolean listenerPresent = getListener() != null;
-        {
+        boolean endStream = data.frame().isEndStream();
-            if (offer(data))
+        if ((listenerPresent || endStream) && offer(data))
-                processData();
+            processData();
-        }
+        if (!listenerPresent && updateClose(endStream, CloseState.Event.RECEIVED))
-        else
+            session.removeStream(this);
-        {
-            if (updateClose(data.frame().isEndStream(), CloseState.Event.RECEIVED))
-                session.removeStream(this);
-        }
     }
 
     private boolean offer(Data data)

jetty-ee9/jetty-ee9-tests/jetty-ee9-test-client-transports/src/test/java/org/eclipse/jetty/ee9/test/client/transport/AbstractTest.java

@@ -19,6 +19,7 @@ import java.nio.file.Path;
 import java.util.Collection;
 import java.util.EnumSet;
 import java.util.List;
+import java.util.Objects;
 
 import jakarta.servlet.http.HttpServlet;
 import org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory;
@@ -112,6 +113,11 @@ public class AbstractTest
     }
 
     protected void prepareServer(Transport transport, HttpServlet servlet) throws Exception
+    {
+        prepareServer(transport, servlet, "/");
+    }
+
+    protected void prepareServer(Transport transport, HttpServlet servlet, String path) throws Exception
     {
         if (transport == Transport.UNIX_DOMAIN)
         {
@@ -126,11 +132,16 @@ public class AbstractTest
         connector = newConnector(transport, server);
         server.addConnector(connector);
         servletContextHandler = new ServletContextHandler();
-        servletContextHandler.setContextPath("/");
+        addServlet(servlet, path);
+        server.setHandler(servletContextHandler);
+    }
+
+    protected void addServlet(HttpServlet servlet, String path) throws Exception
+    {
+        Objects.requireNonNull(servletContextHandler);
         ServletHolder holder = new ServletHolder(servlet);
         holder.setAsyncSupported(true);
-        servletContextHandler.addServlet(holder, "/*");
+        servletContextHandler.getServletHandler().addServletWithMapping(holder, path);
-        server.setHandler(servletContextHandler);
     }
 
     protected Server newServer()

jetty-ee9/jetty-ee9-tests/jetty-ee9-test-client-transports/src/test/java/org/eclipse/jetty/ee9/test/client/transport/PushedResourcesTest.java

@@ -14,6 +14,8 @@
 package org.eclipse.jetty.ee9.test.client.transport;
 
 import java.io.IOException;
+import java.net.URI;
+import java.nio.file.Path;
 import java.util.Random;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
@@ -21,15 +23,20 @@ import java.util.concurrent.TimeUnit;
 import jakarta.servlet.http.HttpServlet;
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
+import jakarta.servlet.http.PushBuilder;
 import org.eclipse.jetty.client.BufferingResponseListener;
 import org.eclipse.jetty.client.ContentResponse;
 import org.eclipse.jetty.client.Result;
+import org.eclipse.jetty.ee9.servlet.DefaultServlet;
 import org.eclipse.jetty.http.HttpStatus;
+import org.eclipse.jetty.server.NetworkConnector;
+import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.MethodSource;
 
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 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 PushedResourcesTest extends AbstractTest
@@ -105,6 +112,227 @@ public class PushedResourcesTest extends AbstractTest
         assertTrue(latch2.await(5, TimeUnit.SECONDS));
     }
 
+    @ParameterizedTest
+    @MethodSource("transportsWithPushSupport")
+    public void testPushedResourcesSomewhatLikeTCK(Transport transport) throws Exception
+    {
+        Random random = new Random();
+        byte[] bytes = new byte[512];
+        random.nextBytes(bytes);
+        byte[] pushBytes1 = new byte[1024];
+        random.nextBytes(pushBytes1);
+        byte[] pushBytes2 = new byte[2048];
+        random.nextBytes(pushBytes2);
+
+        String path1 = "/secondary1";
+        String path2 = "/secondary2";
+        start(transport, new HttpServlet()
+        {
+            @Override
+            protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException
+            {
+                String target = request.getRequestURI();
+                if (target.equals(path1))
+                {
+                    response.getOutputStream().write(pushBytes1);
+                }
+                else if (target.equals(path2))
+                {
+                    response.getOutputStream().write(pushBytes2);
+                }
+                else
+                {
+                    try
+                    {
+                        PushBuilder pb = request.newPushBuilder();
+                        pb.push();
+                    }
+                    catch (Exception e)
+                    {
+                        System.err.println("Expected error empty push builder");
+                    }
+
+                    PushBuilder pb1 = request.newPushBuilder();
+                    pb1.path(path1);
+                    pb1.push();
+                    PushBuilder pb2 = request.newPushBuilder();
+                    pb2.path(path2);
+                    pb2.push();
+
+                    try
+                    {
+                        pb2.push();
+                    }
+                    catch (Exception e)
+                    {
+                        System.err.println("Expected error no path reset");
+                    }
+
+                    PushBuilder pb3 = request.newPushBuilder();
+                    try
+                    {
+                        pb3.method(null);
+                    }
+                    catch (Exception e)
+                    {
+                        System.err.println("Expected error null method");
+                    }
+
+                    String[] methods = {
+                        "", "POST", "PUT", "DELETE",
+                        "CONNECT", "OPTIONS", "TRACE"
+                    };
+
+                    for (String m : methods)
+                    {
+                        try
+                        {
+                            pb3.method(m);
+                            System.err.println("Fail " + m);
+                        }
+                        catch (Exception e)
+                        {
+                            System.err.println("Pass " + m);
+                        }
+                    }
+                    response.getOutputStream().write(bytes);
+                }
+            }
+        });
+
+        CountDownLatch latch1 = new CountDownLatch(1);
+        CountDownLatch latch2 = new CountDownLatch(1);
+        ContentResponse response = client.newRequest(newURI(transport))
+            .onPush((mainRequest, pushedRequest) -> new BufferingResponseListener()
+            {
+                @Override
+                public void onComplete(Result result)
+                {
+                    assertTrue(result.isSucceeded());
+                    if (pushedRequest.getPath().equals(path1))
+                    {
+                        assertArrayEquals(pushBytes1, getContent());
+                        latch1.countDown();
+                    }
+                    else if (pushedRequest.getPath().equals(path2))
+                    {
+                        assertArrayEquals(pushBytes2, getContent());
+                        latch2.countDown();
+                    }
+                }
+            })
+            .timeout(5, TimeUnit.SECONDS)
+            .send();
+
+        assertEquals(HttpStatus.OK_200, response.getStatus());
+        assertArrayEquals(bytes, response.getContent());
+        assertTrue(latch1.await(5, TimeUnit.SECONDS));
+        assertTrue(latch2.await(5, TimeUnit.SECONDS));
+    }
+
+    @ParameterizedTest
+    @MethodSource("transportsWithPushSupport")
+    public void testPushedResourcesLikeTCK(Transport transport) throws Exception
+    {
+        String path1 = "/secondary1.html";
+
+        prepareServer(transport, new DefaultServlet());
+        Path staticDir = MavenTestingUtils.getTestResourcePath("serverpushtck");
+        assertNotNull(staticDir);
+        servletContextHandler.setBaseResourceAsPath(staticDir);
+        addServlet(
+            new HttpServlet()
+            {
+                @Override
+                protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException
+                {
+                    try
+                    {
+                        PushBuilder pb = request.newPushBuilder();
+                        pb.push();
+                    }
+                    catch (Exception e)
+                    {
+                        System.err.println("Expected error empty push builder");
+                    }
+
+                    PushBuilder pb1 = request.newPushBuilder();
+                    pb1.path(path1);
+                    pb1.push();
+
+                    try
+                    {
+                        pb1.push();
+                    }
+                    catch (Exception e)
+                    {
+                        System.err.println("Expected error no path reset");
+                    }
+
+                    PushBuilder pb3 = request.newPushBuilder();
+                    try
+                    {
+                        pb3.method(null);
+                    }
+                    catch (Exception e)
+                    {
+                        System.err.println("Expected error null method");
+                    }
+
+                    String[] methods = {
+                        "", "POST", "PUT", "DELETE",
+                        "CONNECT", "OPTIONS", "TRACE"
+                    };
+
+                    for (String m : methods)
+                    {
+                        try
+                        {
+                            pb3.method(m);
+                            System.err.println("Fail " + m);
+                        }
+                        catch (Exception e)
+                        {
+                            System.err.println("Pass " + m);
+                        }
+                    }
+                    response.getWriter().println("TEST FINISHED");
+                }
+            },
+            "/serverpushtck/*");
+
+        server.start();
+        startClient(transport);
+        CountDownLatch latch1 = new CountDownLatch(1);
+
+        String scheme = transport.isSecure() ? "https" : "http";
+        String uri = scheme + "://localhost";
+        if (connector instanceof NetworkConnector networkConnector)
+            uri += ":" + networkConnector.getLocalPort();
+        URI theURI = URI.create(uri + "/serverpushtck/foo");
+
+        ContentResponse response = client.newRequest(theURI)
+            .onPush((mainRequest, pushedRequest) -> new BufferingResponseListener()
+            {
+                @Override
+                public void onComplete(Result result)
+                {
+                    assertTrue(result.isSucceeded());
+                    if (pushedRequest.getPath().equals(path1))
+                    {
+                        assertTrue(getContentAsString().contains("SECONDARY 1"));
+                        latch1.countDown();
+                    }
+                }
+            })
+            .timeout(5, TimeUnit.SECONDS)
+            .send();
+
+        assertEquals(HttpStatus.OK_200, response.getStatus());
+        assertTrue(response.getContentAsString().contains("TEST FINISHED"));
+        assertTrue(latch1.await(5, TimeUnit.SECONDS));
+    }
+
     @ParameterizedTest
     @MethodSource("transportsWithPushSupport")
     public void testPushedResourceRedirect(Transport transport) throws Exception
@@ -132,7 +360,7 @@ public class PushedResourcesTest extends AbstractTest
         });
 
         CountDownLatch latch = new CountDownLatch(1);
-        ;
+
         ContentResponse response = client.newRequest(newURI(transport))
             .onPush((mainRequest, pushedRequest) -> new BufferingResponseListener()
             {

jetty-ee9/jetty-ee9-tests/jetty-ee9-test-client-transports/src/test/resources/serverpushtck/secondary1.html

@@ -0,0 +1,3 @@
+<html>
+<h1>SECONDARY 1</h1>
+</html>