From 625414e06648931ca6dfe685dbe577a66b912213 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 6 Apr 2022 11:46:38 -0500 Subject: [PATCH 1/3] Fixes #7818 - allow HttpChannel.Listener.onResponseBegin to modify response headers Signed-off-by: Joakim Erdfelt --- .../org/eclipse/jetty/server/HttpChannel.java | 3 +- .../jetty/server/HttpChannelEventTest.java | 53 +++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java index 6e13f885faf1..94946086a773 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java @@ -1037,11 +1037,12 @@ protected boolean sendResponse(MetaData.Response response, ByteBuffer content, b if (committing) { + // Let HttpChannel.Listeners modify the response before commit + _combinedListener.onResponseBegin(_request); // We need an info to commit if (response == null) response = _response.newResponseMetaData(); commit(response); - _combinedListener.onResponseBegin(_request); _request.onResponseCommit(); // wrap callback to process 100 responses diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelEventTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelEventTest.java index 0ba68e298e34..c7ac4d44d6e8 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelEventTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelEventTest.java @@ -15,6 +15,7 @@ import java.io.IOException; import java.nio.ByteBuffer; +import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; @@ -23,8 +24,10 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpTester; +import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.io.EndPoint; import org.eclipse.jetty.server.handler.AbstractHandler; import org.hamcrest.Matchers; @@ -32,6 +35,7 @@ import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -163,6 +167,55 @@ public void onComplete(Request request) assertTrue(latch.await(5, TimeUnit.SECONDS)); } + @Test + public void testResponseBeginModifyHeaders() throws Exception + { + start(new TestHandler() + { + @Override + protected void handle(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + response.setCharacterEncoding("utf-8"); + response.setContentType("text/plain"); + // Intentionally add two values for a header + response.addHeader("X-Header", "foo"); + response.addHeader("X-Header", "bar"); + } + }); + + CountDownLatch latch = new CountDownLatch(1); + connector.addBean(new HttpChannel.Listener() + { + @Override + public void onResponseBegin(Request request) + { + Response response = request.getResponse(); + // Eliminate all "X-Header" values from Handler, and force it to be the one value "zed" + response.getHttpFields().computeField("X-Header", (n, f) -> new HttpField(n, "zed")); + } + + @Override + public void onComplete(Request request) + { + latch.countDown(); + } + }); + + HttpTester.Request request = HttpTester.newRequest(); + request.setVersion(HttpVersion.HTTP_1_1); + request.setHeader("Host", "localhost"); + request.setHeader("Connection", "close"); + + HttpTester.Response response = HttpTester.parseResponse(connector.getResponse(request.toString(), 5, TimeUnit.SECONDS)); + System.out.printf("Response: %s%n", response); + + assertTrue(latch.await(5, TimeUnit.SECONDS)); + + List xheaders = response.getFields("X-Header"); + assertThat("X-Header count", xheaders.size(), is(1)); + assertThat("X-Header[0].value", xheaders.get(0).getValue(), is("zed")); + } + @Test public void testResponseFailure() throws Exception { From 25fb76630d98da1da33bf339034dc4c11cf5820d Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 6 Apr 2022 11:48:35 -0500 Subject: [PATCH 2/3] Fixes #7818 - Eliminate noise from test Signed-off-by: Joakim Erdfelt --- .../test/java/org/eclipse/jetty/server/HttpChannelEventTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelEventTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelEventTest.java index c7ac4d44d6e8..3d4fc9699c82 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelEventTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelEventTest.java @@ -207,7 +207,6 @@ public void onComplete(Request request) request.setHeader("Connection", "close"); HttpTester.Response response = HttpTester.parseResponse(connector.getResponse(request.toString(), 5, TimeUnit.SECONDS)); - System.out.printf("Response: %s%n", response); assertTrue(latch.await(5, TimeUnit.SECONDS)); From 07355081808de6588cf8f02dcfc933dcc4daafb2 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 6 Apr 2022 13:39:10 -0500 Subject: [PATCH 3/3] Fixes #7818 - Fixing warnings on HttpChannelEventTest Signed-off-by: Joakim Erdfelt --- .../org/eclipse/jetty/server/HttpChannelEventTest.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelEventTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelEventTest.java index 3d4fc9699c82..48f3fd1df505 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelEventTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelEventTest.java @@ -69,7 +69,7 @@ public void testRequestContentSlice() throws Exception start(new TestHandler() { @Override - protected void handle(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + protected void handle(HttpServletRequest request, HttpServletResponse response) throws IOException { ServletInputStream input = request.getInputStream(); int content = input.read(); @@ -111,7 +111,7 @@ public void testResponseContentSlice() throws Exception start(new TestHandler() { @Override - protected void handle(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + protected void handle(HttpServletRequest request, HttpServletResponse response) throws IOException { response.getOutputStream().write(data); } @@ -173,7 +173,7 @@ public void testResponseBeginModifyHeaders() throws Exception start(new TestHandler() { @Override - protected void handle(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + protected void handle(HttpServletRequest request, HttpServletResponse response) { response.setCharacterEncoding("utf-8"); response.setContentType("text/plain"); @@ -221,7 +221,7 @@ public void testResponseFailure() throws Exception start(new TestHandler() { @Override - protected void handle(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + protected void handle(HttpServletRequest request, HttpServletResponse response) { // Closes all connections, response will fail. connector.getConnectedEndPoints().forEach(EndPoint::close); @@ -287,6 +287,7 @@ public void onComplete(Request request) assertThat(elapsed.get(), Matchers.greaterThan(0L)); } + @SuppressWarnings("deprecation") @Test public void testTransientListener() throws Exception {