From 2850db13e216def95f1b8c13322373ca78382b0c Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 1 Jun 2022 13:12:40 -0500 Subject: [PATCH] Fixes #7818 - Regression: allow HttpChannel.Listener.onResponseBegin to modify response headers (#7850) * allow HttpChannel.Listener.onResponseBegin to modify response headers Signed-off-by: Joakim Erdfelt --- .../org/eclipse/jetty/server/HttpChannel.java | 3 +- .../jetty/server/HttpChannelEventTest.java | 59 ++++++++++++++++++- 2 files changed, 58 insertions(+), 4 deletions(-) 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 69b2795d1ac..f89fdcd5297 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 @@ public abstract class HttpChannel implements Runnable, HttpOutput.Interceptor 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 informational 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 0ba68e298e3..48f3fd1df50 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 @@ package org.eclipse.jetty.server; 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.ServletInputStream; 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.AfterEach; 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; @@ -65,7 +69,7 @@ public class HttpChannelEventTest 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(); @@ -107,7 +111,7 @@ public class HttpChannelEventTest 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); } @@ -163,13 +167,61 @@ public class HttpChannelEventTest assertTrue(latch.await(5, TimeUnit.SECONDS)); } + @Test + public void testResponseBeginModifyHeaders() throws Exception + { + start(new TestHandler() + { + @Override + protected void handle(HttpServletRequest request, HttpServletResponse response) + { + 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)); + + 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 { 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); @@ -235,6 +287,7 @@ public class HttpChannelEventTest assertThat(elapsed.get(), Matchers.greaterThan(0L)); } + @SuppressWarnings("deprecation") @Test public void testTransientListener() throws Exception {