From 996c5aa16379b16453f3c39d7cc8a58c4fa74533 Mon Sep 17 00:00:00 2001 From: Gavin Bunney Date: Wed, 8 Oct 2025 13:06:59 -0700 Subject: [PATCH 1/3] Only warn when writing to client before LastContent when a body is expected --- .../netty/server/ClientResponseWriter.java | 5 +- .../server/ClientResponseWriterTest.java | 49 +++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/zuul-core/src/main/java/com/netflix/zuul/netty/server/ClientResponseWriter.java b/zuul-core/src/main/java/com/netflix/zuul/netty/server/ClientResponseWriter.java index 2cb4862e7e..4e8839714c 100644 --- a/zuul-core/src/main/java/com/netflix/zuul/netty/server/ClientResponseWriter.java +++ b/zuul-core/src/main/java/com/netflix/zuul/netty/server/ClientResponseWriter.java @@ -118,9 +118,10 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception channel.attr(ClientRequestReceiver.ATTR_ZUUL_RESP).set(zuulResponse); if (channel.isActive()) { - // Track if this is happening. + // track if response is being written before receiving LastContent for requests with a body if (!ClientRequestReceiver.isLastContentReceivedForChannel(channel) - && !shouldAllowPreemptiveResponse(channel)) { + && !shouldAllowPreemptiveResponse(channel) + && zuulResponse.getInboundRequest().hasBody()) { responseBeforeReceivedLastContentCounter.increment(); logger.warn( "Writing response to client channel before have received the LastContent of request! {}," diff --git a/zuul-core/src/test/java/com/netflix/zuul/netty/server/ClientResponseWriterTest.java b/zuul-core/src/test/java/com/netflix/zuul/netty/server/ClientResponseWriterTest.java index 264c3ba5dc..233b50f4fd 100644 --- a/zuul-core/src/test/java/com/netflix/zuul/netty/server/ClientResponseWriterTest.java +++ b/zuul-core/src/test/java/com/netflix/zuul/netty/server/ClientResponseWriterTest.java @@ -19,6 +19,9 @@ import static org.assertj.core.api.Assertions.assertThat; import com.netflix.netty.common.HttpLifecycleChannelHandler; +import com.netflix.spectator.api.Counter; +import com.netflix.spectator.api.DefaultRegistry; +import com.netflix.spectator.api.Registry; import com.netflix.zuul.BasicRequestCompleteHandler; import com.netflix.zuul.context.CommonContextKeys; import com.netflix.zuul.context.SessionContext; @@ -30,6 +33,7 @@ import com.netflix.zuul.stats.status.StatusCategory; import com.netflix.zuul.stats.status.StatusCategoryUtils; import com.netflix.zuul.stats.status.ZuulStatusCategory; +import io.netty.buffer.Unpooled; import io.netty.channel.Channel; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelOutboundHandlerAdapter; @@ -43,6 +47,8 @@ import java.util.concurrent.atomic.AtomicReference; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.junit.jupiter.MockitoExtension; @ExtendWith(MockitoExtension.class) @@ -134,4 +140,47 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise) HttpLifecycleChannelHandler.CompleteReason.SESSION_COMPLETE, null, nettyResp.get())); assertThat(responseWriter.getZuulResponse()).isNull(); } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void warningOnlyForRequestsWithBody(boolean hasBodyChunk) { + Registry registry = new DefaultRegistry(); + Counter warningCounter = registry.counter("server.http.requests.responseBeforeReceivedLastContent"); + ClientResponseWriter responseWriter = new ClientResponseWriter(new BasicRequestCompleteHandler(), registry); + EmbeddedChannel channel = new EmbeddedChannel(responseWriter); + + SessionContext ctx = new SessionContext(); + HttpRequestMessage request = new HttpRequestBuilder(ctx).build(); + + if (hasBodyChunk) { + request.bufferBodyContents(new io.netty.handler.codec.http.DefaultHttpContent( + Unpooled.copiedBuffer("body", java.nio.charset.StandardCharsets.UTF_8))); + } + + request.storeInboundRequest(); + + HttpResponseMessageImpl response = new HttpResponseMessageImpl(ctx, request, 200); + response.setHeaders(new Headers()); + + channel.attr(ClientRequestReceiver.ATTR_ZUUL_REQ).set(request); + DefaultHttpRequest nettyRequest = new DefaultHttpRequest( + HttpVersion.HTTP_1_1, + hasBodyChunk ? HttpMethod.POST : HttpMethod.GET, + hasBodyChunk ? "/api" : "/favicon.ico"); + ctx.set(CommonContextKeys.NETTY_HTTP_REQUEST, nettyRequest); + + channel.pipeline().fireUserEventTriggered(new HttpLifecycleChannelHandler.StartEvent(nettyRequest)); + channel.writeInbound(response); + + assertThat(responseWriter.getZuulResponse()).isNotNull(); + assertThat(request.hasBody()).isEqualTo(hasBodyChunk); + + if (hasBodyChunk) { + assertThat(warningCounter.count()).as("should warn as is body expected").isEqualTo(1); + } else { + assertThat(warningCounter.count()).as("should not warn as no body expected").isEqualTo(0); + } + + channel.close(); + } } From a68adf3488a89a772ca4615e5af1d5b9da3ec0c4 Mon Sep 17 00:00:00 2001 From: Gavin Bunney Date: Wed, 8 Oct 2025 14:05:00 -0700 Subject: [PATCH 2/3] fmt --- .../zuul/netty/server/ClientResponseWriterTest.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/zuul-core/src/test/java/com/netflix/zuul/netty/server/ClientResponseWriterTest.java b/zuul-core/src/test/java/com/netflix/zuul/netty/server/ClientResponseWriterTest.java index 233b50f4fd..24f458fb7c 100644 --- a/zuul-core/src/test/java/com/netflix/zuul/netty/server/ClientResponseWriterTest.java +++ b/zuul-core/src/test/java/com/netflix/zuul/netty/server/ClientResponseWriterTest.java @@ -176,9 +176,13 @@ void warningOnlyForRequestsWithBody(boolean hasBodyChunk) { assertThat(request.hasBody()).isEqualTo(hasBodyChunk); if (hasBodyChunk) { - assertThat(warningCounter.count()).as("should warn as is body expected").isEqualTo(1); + assertThat(warningCounter.count()) + .as("should warn as is body expected") + .isEqualTo(1); } else { - assertThat(warningCounter.count()).as("should not warn as no body expected").isEqualTo(0); + assertThat(warningCounter.count()) + .as("should not warn as no body expected") + .isEqualTo(0); } channel.close(); From 953d18ca39b70af943ea0e912a2abeb455536186 Mon Sep 17 00:00:00 2001 From: Gavin Bunney Date: Wed, 8 Oct 2025 15:13:06 -0700 Subject: [PATCH 3/3] Dispose body in test --- .../com/netflix/zuul/netty/server/ClientResponseWriterTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/zuul-core/src/test/java/com/netflix/zuul/netty/server/ClientResponseWriterTest.java b/zuul-core/src/test/java/com/netflix/zuul/netty/server/ClientResponseWriterTest.java index 24f458fb7c..897e971fa2 100644 --- a/zuul-core/src/test/java/com/netflix/zuul/netty/server/ClientResponseWriterTest.java +++ b/zuul-core/src/test/java/com/netflix/zuul/netty/server/ClientResponseWriterTest.java @@ -185,6 +185,7 @@ void warningOnlyForRequestsWithBody(boolean hasBodyChunk) { .isEqualTo(0); } + request.disposeBufferedBody(); channel.close(); } }