From 91aaee43a799b4febab47b106e644edaf2f1a8cf Mon Sep 17 00:00:00 2001 From: mansal Date: Mon, 20 Jan 2025 06:00:18 -0500 Subject: [PATCH 1/3] Initial commit, debugging preload test --- .../bootstrap.properties | 2 +- .../driver/war/servlets/PushPromiseTests.java | 2 +- .../test/war/servlets/H2PushPromise.java | 53 ++++++++++--------- 3 files changed, 29 insertions(+), 28 deletions(-) diff --git a/dev/com.ibm.ws.transport.http2_fat/publish/servers/com.ibm.ws.transport.http2.fat/bootstrap.properties b/dev/com.ibm.ws.transport.http2_fat/publish/servers/com.ibm.ws.transport.http2.fat/bootstrap.properties index fbbf0add12c2..c9953cd376e2 100644 --- a/dev/com.ibm.ws.transport.http2_fat/publish/servers/com.ibm.ws.transport.http2.fat/bootstrap.properties +++ b/dev/com.ibm.ws.transport.http2_fat/publish/servers/com.ibm.ws.transport.http2.fat/bootstrap.properties @@ -1,4 +1,4 @@ -com.ibm.ws.logging.trace.specification=*=info=enabled +com.ibm.ws.logging.trace.specification=*=info=enabled:com.ibm.ws.http*=all:io.openliberty.http* com.ibm.ws.logging.max.file.size=0 ds.loglevel=debug diff --git a/dev/com.ibm.ws.transport.http2_fat/test-applications/H2FATDriver.war/src/http2/test/driver/war/servlets/PushPromiseTests.java b/dev/com.ibm.ws.transport.http2_fat/test-applications/H2FATDriver.war/src/http2/test/driver/war/servlets/PushPromiseTests.java index d30362c49974..f53702a69991 100644 --- a/dev/com.ibm.ws.transport.http2_fat/test-applications/H2FATDriver.war/src/http2/test/driver/war/servlets/PushPromiseTests.java +++ b/dev/com.ibm.ws.transport.http2_fat/test-applications/H2FATDriver.war/src/http2/test/driver/war/servlets/PushPromiseTests.java @@ -156,7 +156,7 @@ public void onPushPromiseReceived(FramePushPromise receivedFrame) { } }; - byte[] testBytes = new byte[] { 'l', 'i', 'b', 'e', 'r', 't', 'y' }; + byte[] testBytes = new byte[] { 'l', 'i', 'b', 'e', 'r', 't', 'y','!' }; FramePing expectedPing = new FramePing(0, testBytes, false); expectedPing.setAckFlag(); h2Client.addExpectedFrame(expectedPing); diff --git a/dev/com.ibm.ws.transport.http2_fat/test-applications/H2TestModule.war/src/http2/test/war/servlets/H2PushPromise.java b/dev/com.ibm.ws.transport.http2_fat/test-applications/H2TestModule.war/src/http2/test/war/servlets/H2PushPromise.java index 26ca75878655..5485c49ecc57 100644 --- a/dev/com.ibm.ws.transport.http2_fat/test-applications/H2TestModule.war/src/http2/test/war/servlets/H2PushPromise.java +++ b/dev/com.ibm.ws.transport.http2_fat/test-applications/H2TestModule.war/src/http2/test/war/servlets/H2PushPromise.java @@ -34,9 +34,7 @@ public class H2PushPromise extends HttpServlet { public void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { PrintWriter pw = response.getWriter(); - - String test = null; - test = request.getParameter("test"); + String test = request.getParameter("test"); if ((test == null) || (test == (new String("")))) { // No special parm, this is the pushed request @@ -45,24 +43,26 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) thro response.setDateHeader("Date", time); pw.println("Response to push"); + pw.flush(); pw.close(); + response.flushBuffer(); - } else if (test.toLowerCase().equals(new String("preload"))) { + } else if ("preload".equalsIgnoreCase(test)) { // Test the header link rel=preload path pw.println("push_promise link header rel=preload"); - long time = System.currentTimeMillis(); - try { - Thread.sleep(250); - } catch (Exception x) { - } - response.setDateHeader("Date", time); + + try {Thread.sleep(250);} catch (Exception x) {/*Ignore Interrupt*/} + + response.setDateHeader("Date", System.currentTimeMillis()); response.addHeader("Link", "; rel=preload;"); response.addHeader("Link", "; rel=preload;"); + pw.flush(); pw.close(); + response.flushBuffer(); - } else if ((test.toLowerCase().equals(new String("pushbuilder")))) { + } else if ("pushbulder".equalsIgnoreCase(test)) { // Test the pushbuilder path pw.println("push_promise PushBuilder"); @@ -72,10 +72,7 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) thro String name = reqHeaderNames.nextElement(); pw.println("Req Header : " + name + ":" + request.getHeader(name)); } - try { - Thread.sleep(250); - } catch (Exception x) { - } + try {Thread.sleep(250);} catch (Exception x) {} PushBuilder pb = request.newPushBuilder(); if (pb != null) { pb.path("/H2TestModule/H2PushPromise"); @@ -87,21 +84,25 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) thro pw.println("FAIL : pb.push() threw an ISE : " + exc.getMessage()); } } + pw.flush(); pw.close(); - } else if ((test.toLowerCase().equals(new String("delay")))) { + response.flushBuffer(); + } else if ("delay".equalsIgnoreCase(test)) { - try { - Thread.sleep(5000); - } catch (Exception x) { - } - - } + try {Thread.sleep(5000);} catch (Exception x) {} + response.setDateHeader("Date", System.currentTimeMillis()); + pw.println("Delayed response complete"); + pw.flush(); + pw.close(); + response.flushBuffer(); - try { - Thread.yield(); - } catch (Exception x) { + }else{ + response.setDateHeader("Date", System.currentTimeMillis()); + pw.println("Unknown test param: " + test); + pw.flush(); + pw.close(); + response.flushBuffer(); } - } /* From 05ee8c9606e84116fc5f673e36be3697f89d55c8 Mon Sep 17 00:00:00 2001 From: mansal Date: Tue, 21 Jan 2025 09:43:27 -0500 Subject: [PATCH 2/3] Adjusting test makeup to try to avoid race conditions --- .../driver/war/servlets/PushPromiseTests.java | 193 +++++++----------- 1 file changed, 76 insertions(+), 117 deletions(-) diff --git a/dev/com.ibm.ws.transport.http2_fat/test-applications/H2FATDriver.war/src/http2/test/driver/war/servlets/PushPromiseTests.java b/dev/com.ibm.ws.transport.http2_fat/test-applications/H2FATDriver.war/src/http2/test/driver/war/servlets/PushPromiseTests.java index f53702a69991..e96e378b71c0 100644 --- a/dev/com.ibm.ws.transport.http2_fat/test-applications/H2FATDriver.war/src/http2/test/driver/war/servlets/PushPromiseTests.java +++ b/dev/com.ibm.ws.transport.http2_fat/test-applications/H2FATDriver.war/src/http2/test/driver/war/servlets/PushPromiseTests.java @@ -15,8 +15,11 @@ import java.io.IOException; import java.text.SimpleDateFormat; import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; import java.util.List; import java.util.Locale; +import java.util.Set; import java.util.TimeZone; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -49,139 +52,87 @@ public class PushPromiseTests extends H2FATDriverServlet { private static final Logger LOGGER = Logger.getLogger(PushPromiseTests.class.getName()); private static final String SERVLET_PUSH_PROMISE = "/H2TestModule/H2PushPromise"; + private FrameHeadersClient createFrameHeaders(int streamId, boolean usingNetty) { + return new FrameHeadersClient(streamId, null, 0, 0, usingNetty ? 15 : 0, // Weight depends on Netty usage + true, true, false, usingNetty, false, false // Priority and other flags depend on Netty usage + ); + } + public void testPushPromisePreload(HttpServletRequest request, HttpServletResponse response) throws Exception { - CountDownLatch blockUntilConnectionIsDone = new CountDownLatch(1); + CountDownLatch pushPromiseLatch = new CountDownLatch(3); String testName = "testPushPromisePreload"; if (LOGGER.isLoggable(Level.INFO)) { LOGGER.logp(Level.INFO, this.getClass().getName(), testName, "request: " + request); LOGGER.logp(Level.INFO, this.getClass().getName(), testName, "hostName: " + request.getParameter("hostName")); LOGGER.logp(Level.INFO, this.getClass().getName(), testName, "port: " + request.getParameter("port")); } - Http2Client h2Client = new Http2Client(request.getParameter("hostName"), Integer.parseInt(request.getParameter("port")), blockUntilConnectionIsDone, defaultTimeoutToSendFrame); + Http2Client h2Client = new Http2Client(request.getParameter("hostName"), Integer.parseInt(request.getParameter("port")), pushPromiseLatch, defaultTimeoutToSendFrame); boolean testFailed = false; StringBuilder message = new StringBuilder("The following exceptions were found: "); - // We should be expecting - // some setting frames - // a push promise frame on stream 1 - // a second push promise frame on stream 1 - // a headers frame on stream 1 to satisfy the original request - // a headers frame on stream 2 to satisfy the pushed request - // a headers frame on stream 4 to satisfy the second pushed request - SimpleDateFormat date = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss z", Locale.US); date.setTimeZone(TimeZone.getTimeZone("GMT")); + // We should be expecting some setting frames: + // Setting frame(s) on stream 0 h2Client.addExpectedFrame(DEFAULT_SERVER_SETTINGS_FRAME); - // PushPromiseFrame on stream 1 - List pushPromiseHeadersReceived = new ArrayList(); - pushPromiseHeadersReceived.add(new H2HeaderField(":method", "GET")); - pushPromiseHeadersReceived.add(new H2HeaderField(":path", SERVLET_PUSH_PROMISE)); - pushPromiseHeadersReceived.add(new H2HeaderField(":authority", "127.0.0.1")); - pushPromiseHeadersReceived.add(new H2HeaderField(":scheme", "http")); - byte[] headerBlockFragment = new byte[0]; - FramePushPromiseClient pushPromise = new FramePushPromiseClient(1, headerBlockFragment, 2, 0, true, false, false); - h2Client.addExpectedFrame(pushPromise); - - // Seconds PushPromiseFrame on stream 1 - List pushPromiseHeadersReceived2 = new ArrayList(); - pushPromiseHeadersReceived2.add(new H2HeaderField(":method", "GET")); - pushPromiseHeadersReceived2.add(new H2HeaderField(":path", SERVLET_PUSH_PROMISE)); - pushPromiseHeadersReceived2.add(new H2HeaderField(":authority", "127.0.0.1")); - pushPromiseHeadersReceived2.add(new H2HeaderField(":scheme", "http")); - byte[] headerBlockFragment2 = new byte[0]; - FramePushPromiseClient pushPromise2 = new FramePushPromiseClient(1, headerBlockFragment2, 4, 0, true, false, false); - h2Client.addExpectedFrame(pushPromise2); - - // Headers frame with results from the original request on stream 1 - List firstHeadersReceived = new ArrayList(); - firstHeadersReceived.add(new H2HeaderField(":status", "200")); - firstHeadersReceived.add(new H2HeaderField("x-powered-by", "Servlet/4.0")); - firstHeadersReceived.add(new H2HeaderField("date", ".*")); //regex because date will vary - FrameHeadersClient frameHeaders; - if (USING_NETTY) - frameHeaders = new FrameHeadersClient(1, null, 0, 0, 15, true, true, false, true, false, false); - else - frameHeaders = new FrameHeadersClient(1, null, 0, 0, 0, true, true, false, false, false, false); - frameHeaders.setHeaderFields(firstHeadersReceived); - h2Client.addExpectedFrame(frameHeaders); - - CountDownLatch blockUntilPushPromiseReceived = new CountDownLatch(2); - - PushPromiseListener pushPromiseListener = new PushPromiseListener() { - + //A Push Promise Frame on Stream 1 (Promised Stream 2) + FramePushPromiseClient pushPromise1 = new FramePushPromiseClient( + 1, new byte[0], 2, 0, true, false, false); + pushPromise1.setHeaderFields(Arrays.asList( + new H2HeaderField(":method", "GET"), + new H2HeaderField(":path", SERVLET_PUSH_PROMISE), + new H2HeaderField(":authority", "127.0.0.1"), + new H2HeaderField(":scheme", "http"))); + + // Define PUSH_PROMISE 2 (stream 1 -> promised stream 4) + FramePushPromiseClient pushPromise2 = new FramePushPromiseClient( + 1, new byte[0], 4, 0, true, false, false); + pushPromise2.setHeaderFields(Arrays.asList( + new H2HeaderField(":method", "GET"), + new H2HeaderField(":path", SERVLET_PUSH_PROMISE), + new H2HeaderField(":authority", "127.0.0.1"), + new H2HeaderField(":scheme", "http"))); + + // A Headers Frame on Stream 1 to satisfy the original request + FrameHeadersClient originalRequest = createFrameHeaders(1, USING_NETTY); + originalRequest.setHeaderFields(Arrays.asList( + new H2HeaderField(":status", "200"), + new H2HeaderField("x-powered-by", "Servlet/4.0"), + new H2HeaderField("date",".*"))); + + + h2Client.addExpectedFrame(originalRequest); + + // Set a PushPromiseListener to dynamically add expected headers for promised streams + // Expected Header Frames on Streams 2 and 4 + Set promisedStreams = new HashSet<>(); // To track promised streams dynamically + h2Client.setPushPromiseListener(new PushPromiseListener() { @Override public void onPushPromiseReceived(FramePushPromise receivedFrame) { - int promisedStreamId = receivedFrame.getPromisedStreamId(); - // First time - if (blockUntilPushPromiseReceived.getCount() > 1) { - // Headers frame with results from pushed resource request on specified StreamID - List secondHeadersReceived = new ArrayList(); - secondHeadersReceived.add(new H2HeaderField(":status", "200")); - secondHeadersReceived.add(new H2HeaderField("x-powered-by", "Servlet/4.0")); - secondHeadersReceived.add(new H2HeaderField("date", ".*")); //regex because date will vary - FrameHeadersClient secondFrameHeaders; - secondFrameHeaders = new FrameHeadersClient(promisedStreamId, null, 0, 0, 15, false, true, false, true, false, false); - secondFrameHeaders.setHeaderFields(secondHeadersReceived); - try { - h2Client.addExpectedFrame(secondFrameHeaders); - blockUntilPushPromiseReceived.countDown(); - } catch (CompressionException | IOException | ExpectedPushPromiseDoesNotIncludeLinkHeaderException e) { - if (LOGGER.isLoggable(Level.INFO)) { - LOGGER.logp(Level.INFO, this.getClass().getName(), testName, "Caught exception attempting to add expected Push Promise Headers!" + e); - } + LOGGER.info("Received PUSH_PROMISE: " + receivedFrame); + try { + // Check if headers for the promised stream already exist + int promisedStreamId = receivedFrame.getPromisedStreamId(); + if (!promisedStreams.contains(promisedStreamId)) { + promisedStreams.add(promisedStreamId); + FrameHeadersClient expectedHeaders = createFrameHeaders(promisedStreamId, USING_NETTY); + expectedHeaders.setHeaderFields(Arrays.asList( + new H2HeaderField(":status", "200"), + new H2HeaderField("x-powered-by", "Servlet/4.0"), + new H2HeaderField("date", ".*") + )); + h2Client.addExpectedFrame(expectedHeaders); + pushPromiseLatch.countDown(); } - } else if (blockUntilPushPromiseReceived.getCount() == 1) { - // Headers frame with results from pushed resource request on specified StreamID - List thirdHeadersReceived = new ArrayList(); - thirdHeadersReceived.add(new H2HeaderField(":status", "200")); - thirdHeadersReceived.add(new H2HeaderField("x-powered-by", "Servlet/4.0")); - thirdHeadersReceived.add(new H2HeaderField("date", ".*")); //regex because date will vary - FrameHeadersClient thirdFrameHeaders = new FrameHeadersClient(promisedStreamId, null, 0, 0, 15, false, true, false, true, false, false); - thirdFrameHeaders.setHeaderFields(thirdHeadersReceived); - try { - h2Client.addExpectedFrame(thirdFrameHeaders); - blockUntilPushPromiseReceived.countDown(); - } catch (CompressionException | IOException | ExpectedPushPromiseDoesNotIncludeLinkHeaderException e) { - if (LOGGER.isLoggable(Level.INFO)) { - LOGGER.logp(Level.INFO, this.getClass().getName(), testName, "Caught exception attempting to add expected Push Promise Headers!" + e); - } - } - } else { - throw new IllegalArgumentException("Should not already be count down!!"); + } catch (Exception e) { + LOGGER.severe("Error adding expected frame for PUSH_PROMISE: " + e); } } - }; - - byte[] testBytes = new byte[] { 'l', 'i', 'b', 'e', 'r', 't', 'y','!' }; - FramePing expectedPing = new FramePing(0, testBytes, false); - expectedPing.setAckFlag(); - h2Client.addExpectedFrame(expectedPing); - - if (USING_NETTY) { - h2Client.setPushPromiseListener(pushPromiseListener); - } else { - // Headers frame with results from pushed resource request on stream 2 - List secondHeadersReceived = new ArrayList(); - secondHeadersReceived.add(new H2HeaderField(":status", "200")); - secondHeadersReceived.add(new H2HeaderField("x-powered-by", "Servlet/4.0")); - secondHeadersReceived.add(new H2HeaderField("date", ".*")); //regex because date will vary - FrameHeadersClient secondFrameHeaders = new FrameHeadersClient(2, null, 0, 0, 0, true, true, false, false, false, false); - secondFrameHeaders.setHeaderFields(secondHeadersReceived); - h2Client.addExpectedFrame(secondFrameHeaders); - - // Headers frame with results from pushed resource request on stream 4 - List thirdHeadersReceived = new ArrayList(); - thirdHeadersReceived.add(new H2HeaderField(":status", "200")); - thirdHeadersReceived.add(new H2HeaderField("x-powered-by", "Servlet/4.0")); - thirdHeadersReceived.add(new H2HeaderField("date", ".*")); //regex because date will vary - FrameHeadersClient thirdFrameHeaders = new FrameHeadersClient(4, null, 0, 0, 0, true, true, false, false, false, false); - thirdFrameHeaders.setHeaderFields(thirdHeadersReceived); - h2Client.addExpectedFrame(thirdFrameHeaders); - } + }); h2Client.sendUpgradeHeader(SERVLET_PUSH_PROMISE + new String("?test=preload")); @@ -189,15 +140,23 @@ public void onPushPromiseReceived(FramePushPromise receivedFrame) { //If the this fails, the test needs to fail as well because the H2 protocol was not established successfully. h2Client.sendClientPrefaceFollowedBySettingsFrame(EMPTY_SETTINGS_FRAME); - if (USING_NETTY) - blockUntilPushPromiseReceived.await(defaultTimeoutToSendFrame, TimeUnit.MILLISECONDS); + // Wait for frames in the expected order + h2Client.waitFor(originalRequest); - FramePing ping = new FramePing(0, testBytes, false); - h2Client.sendFrame(ping); + if (!pushPromiseLatch.await(defaultTimeoutToSendFrame, TimeUnit.MILLISECONDS)) { + throw new AssertionError("Timeout waiting for PUSH_PROMISE frames"); + } - h2Client.waitFor(expectedPing); + // Wait for the headers on the promised streams dynamically + for (int promisedStreamId : promisedStreams) { + FrameHeadersClient expectedHeaders = createFrameHeaders(promisedStreamId, USING_NETTY); + h2Client.waitFor(expectedHeaders); + } - blockUntilConnectionIsDone.await(); + // Wait for connection to complete + pushPromiseLatch.await(); + + // Handle errors handleErrors(h2Client, testName); } From 1e2315a8ada6919dcb2dca5c17351a8df4d29975 Mon Sep 17 00:00:00 2001 From: mansal Date: Tue, 21 Jan 2025 11:01:17 -0500 Subject: [PATCH 3/3] Do writePushPromise within the Netty loop --- .../channel/internal/HttpServiceContextImpl.java | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/dev/com.ibm.ws.transport.http/src/com/ibm/ws/http/channel/internal/HttpServiceContextImpl.java b/dev/com.ibm.ws.transport.http/src/com/ibm/ws/http/channel/internal/HttpServiceContextImpl.java index 5472c0effcb6..997486f7f818 100644 --- a/dev/com.ibm.ws.transport.http/src/com/ibm/ws/http/channel/internal/HttpServiceContextImpl.java +++ b/dev/com.ibm.ws.transport.http/src/com/ibm/ws/http/channel/internal/HttpServiceContextImpl.java @@ -3021,18 +3021,14 @@ private void handleNettyPreload(String uri) { + headers); } - ChannelFuture promise = handler.encoder().writePushPromise(nettyContext, currentStreamId, nextPromisedStreamId, headers, 0, - new VoidChannelPromise(this.nettyContext.channel(), true)); - promise.addListener(future -> { - if (future.isSuccess()){ - - } - else { - future.cause().printStackTrace(); - } + nettyContext.channel().eventLoop().execute(()-> { + handler.encoder().writePushPromise(nettyContext, currentStreamId, nextPromisedStreamId, headers, 0, + new VoidChannelPromise(this.nettyContext.channel(), true)); }); + + try { DefaultFullHttpRequest newRequest = new DefaultFullHttpRequest(nettyRequest.protocolVersion(), HttpMethod.GET, uri); newRequest.headers().set(HttpConversionUtil.ExtensionHeaderNames.STREAM_ID.text(), nextPromisedStreamId);