From eb2206fe1f02f589b3d40016ea69f9f881822607 Mon Sep 17 00:00:00 2001 From: Aayush Atharva Date: Thu, 17 Sep 2026 19:57:07 +0000 Subject: [PATCH 1/3] Return an empty buffer for a bodyless response --- .../org/asynchttpclient/netty/NettyResponse.java | 4 +++- .../netty/NettyAsyncResponseTest.java | 13 +++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/client/src/main/java/org/asynchttpclient/netty/NettyResponse.java b/client/src/main/java/org/asynchttpclient/netty/NettyResponse.java index 0c080aee6..2f595d01f 100755 --- a/client/src/main/java/org/asynchttpclient/netty/NettyResponse.java +++ b/client/src/main/java/org/asynchttpclient/netty/NettyResponse.java @@ -228,7 +228,9 @@ public ByteBuffer getResponseBodyAsByteBuffer() { @Override public ByteBuf getResponseBodyAsByteBuf() { - CompositeByteBuf compositeByteBuf = ByteBufAllocator.DEFAULT.compositeBuffer(bodyParts.size()); + // At least one component: CompositeByteBuf rejects a maxNumComponents of 0, so a bodyless response + // would otherwise throw here rather than return an empty buffer. + CompositeByteBuf compositeByteBuf = ByteBufAllocator.DEFAULT.compositeBuffer(Math.max(1, bodyParts.size())); for (HttpResponseBodyPart part : bodyParts) { compositeByteBuf.addComponent(true, part.getBodyByteBuf()); } diff --git a/client/src/test/java/org/asynchttpclient/netty/NettyAsyncResponseTest.java b/client/src/test/java/org/asynchttpclient/netty/NettyAsyncResponseTest.java index 0b6320d4f..ec927480f 100644 --- a/client/src/test/java/org/asynchttpclient/netty/NettyAsyncResponseTest.java +++ b/client/src/test/java/org/asynchttpclient/netty/NettyAsyncResponseTest.java @@ -185,6 +185,19 @@ public void testGetResponseBodyAsBytesViewReturnsEmptyArray() { assertArrayEquals(new byte[0], response.getResponseBodyAsBytesView()); } + @Test + public void testBodylessResponseHasAnEmptyByteBuf() { + // A CompositeByteBuf rejects a maxNumComponents of 0, so a HEAD, a 204 or a 304 threw from here. + NettyResponse response = new NettyResponse(new NettyResponseStatus(null, null, null), null, new LinkedList<>()); + + ByteBuf body = response.getResponseBodyAsByteBuf(); + try { + assertEquals(0, body.readableBytes()); + } finally { + body.release(); + } + } + @Test public void testGetResponseBodyAsBytesViewDefaultImplementationDelegates() throws Throwable { byte[] expected = "Hello World".getBytes(StandardCharsets.UTF_8); From 58cdc60d78b666ca4bf91f6a890a5976f8d06f01 Mon Sep 17 00:00:00 2001 From: Aayush Atharva Date: Thu, 17 Sep 2026 19:57:12 +0000 Subject: [PATCH 2/3] Copy the body parts when building a response --- .../java/org/asynchttpclient/Response.java | 4 +- .../asynchttpclient/ResponseBuilderTest.java | 75 +++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 client/src/test/java/org/asynchttpclient/ResponseBuilderTest.java diff --git a/client/src/main/java/org/asynchttpclient/Response.java b/client/src/main/java/org/asynchttpclient/Response.java index 3fc871b49..9f31093e0 100644 --- a/client/src/main/java/org/asynchttpclient/Response.java +++ b/client/src/main/java/org/asynchttpclient/Response.java @@ -253,7 +253,9 @@ public void accumulate(HttpResponseBodyPart bodyPart) { * @return a {@link Response} instance */ public @Nullable Response build() { - return status == null ? null : new NettyResponse(status, headers, bodyParts); + // A copy: reset() clears this list, and a builder may build more than once, so a response that held + // the builder's own list would lose its body or take on the next response's. + return status == null ? null : new NettyResponse(status, headers, new ArrayList<>(bodyParts)); } /** diff --git a/client/src/test/java/org/asynchttpclient/ResponseBuilderTest.java b/client/src/test/java/org/asynchttpclient/ResponseBuilderTest.java new file mode 100644 index 000000000..fd2a7295c --- /dev/null +++ b/client/src/test/java/org/asynchttpclient/ResponseBuilderTest.java @@ -0,0 +1,75 @@ +/* + * Copyright (c) 2026 AsyncHttpClient Project. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.asynchttpclient; + +import io.netty.buffer.Unpooled; +import org.asynchttpclient.netty.EagerResponseBodyPart; +import org.asynchttpclient.netty.NettyResponseStatus; +import org.junit.jupiter.api.Test; + +import java.nio.charset.StandardCharsets; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class ResponseBuilderTest { + + private static void accumulateBody(Response.ResponseBuilder builder, String body) { + builder.accumulate(new NettyResponseStatus(null, null, null)); + builder.accumulate(new EagerResponseBodyPart(Unpooled.wrappedBuffer(body.getBytes(StandardCharsets.UTF_8)), true)); + } + + @Test + public void testAResponseKeepsItsBodyWhenTheBuilderIsReused() { + // AsyncCompletionHandler resets its builder on every onStatusReceived, so a handler instance reused for + // a second request refills the same list. A response holding that list would take on the second body. + Response.ResponseBuilder builder = new Response.ResponseBuilder(); + accumulateBody(builder, "first"); + Response first = builder.build(); + + builder.reset(); + accumulateBody(builder, "second"); + Response second = builder.build(); + + assertEquals("first", first.getResponseBody(StandardCharsets.UTF_8)); + assertEquals("second", second.getResponseBody(StandardCharsets.UTF_8)); + } + + @Test + public void testAResponseKeepsItsBodyWhenTheBuilderAccumulatesAgain() { + // Without a reset in between, a further part must not appear in a response that was already built. + Response.ResponseBuilder builder = new Response.ResponseBuilder(); + accumulateBody(builder, "first"); + Response first = builder.build(); + + builder.accumulate(new EagerResponseBodyPart(Unpooled.wrappedBuffer(" second".getBytes(StandardCharsets.UTF_8)), true)); + + assertEquals("first", first.getResponseBody(StandardCharsets.UTF_8)); + assertEquals("first second", builder.build().getResponseBody(StandardCharsets.UTF_8)); + } + + @Test + public void testAResponseKeepsItsBodyWhenTheBuilderIsResetAndNotRebuilt() { + Response.ResponseBuilder builder = new Response.ResponseBuilder(); + accumulateBody(builder, "first"); + Response first = builder.build(); + + builder.reset(); + + assertTrue(first.hasResponseBody()); + assertEquals("first", first.getResponseBody(StandardCharsets.UTF_8)); + } +} From 3a24bee147e167fa265238a389dbca77068e0777 Mon Sep 17 00:00:00 2001 From: Aayush Atharva Date: Thu, 17 Sep 2026 19:57:17 +0000 Subject: [PATCH 3/3] Say what the shared body view aliases --- .../java/org/asynchttpclient/Response.java | 14 +++++---- .../asynchttpclient/netty/NettyResponse.java | 10 ++++--- .../netty/NettyAsyncResponseTest.java | 29 ++++++++++++++++--- 3 files changed, 40 insertions(+), 13 deletions(-) diff --git a/client/src/main/java/org/asynchttpclient/Response.java b/client/src/main/java/org/asynchttpclient/Response.java index 9f31093e0..804dd1660 100644 --- a/client/src/main/java/org/asynchttpclient/Response.java +++ b/client/src/main/java/org/asynchttpclient/Response.java @@ -63,15 +63,19 @@ public interface Response { * body part's own array, it is the array reachable from the part handed to * {@link AsyncHandler#onBodyPartReceived}, the one each * {@link org.asynchttpclient.handler.TransferListener} is given by a - * {@link org.asynchttpclient.handler.TransferCompletionHandler}, and the one + * {@link org.asynchttpclient.handler.TransferCompletionHandler}, the one + * {@link HttpResponseBodyPart#getBodyByteBuffer()} wraps for a + * {@link org.asynchttpclient.handler.resumable.ResumableListener}, and the one * {@link #getResponseBodyAsByteBuf()} wraps. Writing to it changes what all of those see, and a write * through any of them changes what this returns. * *

Whether anything is shared at all is not something to rely on. It depends on how the body happened to - * arrive - how the origin chunked it, whether a proxy re-chunked it, whether it was compressed - and on the - * body parts the implementation was given, none of which is visible from here. The same body from the - * same server may be shared on one response and copied on the next. No array identity is guaranteed between - * calls either. + * arrive - how the origin chunked it, whether a proxy re-chunked it, whether it was compressed - on how the + * client happened to read it off the wire, which varies with connection age and client configuration, and on + * the body parts the implementation was given. The same body from the same server may be shared on one + * response and copied on the next. Which of the two a caller gets is the peer's choice rather than the + * caller's, so code that writes to the array can behave one way against a friendly server and another way + * against a hostile one. No array identity is guaranteed between calls either. * *

A caller that needs an array it may modify should copy what it receives. {@link * #getResponseBodyAsBytes()} is the accessor to reach for first, but it is implemented by whoever implements diff --git a/client/src/main/java/org/asynchttpclient/netty/NettyResponse.java b/client/src/main/java/org/asynchttpclient/netty/NettyResponse.java index 2f595d01f..95838699b 100755 --- a/client/src/main/java/org/asynchttpclient/netty/NettyResponse.java +++ b/client/src/main/java/org/asynchttpclient/netty/NettyResponse.java @@ -196,6 +196,8 @@ public byte[] getResponseBodyAsBytes() { } /** + * {@inheritDoc} + *

* Returns a lone body part's array; concatenates into one of its own when there are several, or an empty * array when there are none. Which of those a given response takes is not a property of the body: see * {@link Response#getResponseBodyAsBytesView()}, whose contract is deliberately weaker than this. @@ -251,15 +253,15 @@ public String getResponseBody() { * documents it as read-only and names the other holders. {@link #getResponseBodyAsBytes()} stays the * copying accessor for callers who want an array of their own. *

- * Private, and called directly by the accessors below rather than through + * Private, and called directly by the string accessors rather than through * {@link #getResponseBodyAsBytesView()}, so that overriding the view does not silently change what this * response's text says as well. */ private byte[] sharedBodyBytes() { if (bodyParts.isEmpty()) { - // A HEAD, a 204 or a 304 otherwise walks the aggregating path to allocate an empty array and a - // buffer to wrap it, on every call. Nothing can be written through a zero-length array, so one - // shared instance serves every empty body. + // A response with no body - a HEAD, a 204, a 304, a discarded CONNECT failure, one aborted from a + // handler - otherwise walks the aggregating path to allocate an empty array and a buffer to wrap it, + // on every call. Nothing can be written through a zero-length array, so one instance serves them all. return EMPTY_BODY; } return bodyParts.size() == 1 ? bodyParts.get(0).getBodyPartBytes() : getResponseBodyAsBytes(); diff --git a/client/src/test/java/org/asynchttpclient/netty/NettyAsyncResponseTest.java b/client/src/test/java/org/asynchttpclient/netty/NettyAsyncResponseTest.java index ec927480f..100900b4b 100644 --- a/client/src/test/java/org/asynchttpclient/netty/NettyAsyncResponseTest.java +++ b/client/src/test/java/org/asynchttpclient/netty/NettyAsyncResponseTest.java @@ -109,13 +109,15 @@ public void testGetResponseBodyDecodesOnePartAndSplitPartsIdentically() { // 0xC3 0xA9 encodes U+00E9; split between its two bytes so neither half decodes on its own int split = 4; + // Clones onto the wire, so that utf8 stays an oracle rather than becoming a part's own storage. List onePart = new LinkedList<>(); - onePart.add(new EagerResponseBodyPart(Unpooled.wrappedBuffer(utf8), true)); + onePart.add(new EagerResponseBodyPart(Unpooled.wrappedBuffer(utf8.clone()), true)); NettyResponse single = new NettyResponse(new NettyResponseStatus(null, null, null), null, onePart); + byte[] wire = utf8.clone(); List splitParts = new LinkedList<>(); - splitParts.add(new EagerResponseBodyPart(Unpooled.wrappedBuffer(utf8, 0, split), false)); - splitParts.add(new EagerResponseBodyPart(Unpooled.wrappedBuffer(utf8, split, utf8.length - split), true)); + splitParts.add(new EagerResponseBodyPart(Unpooled.wrappedBuffer(wire, 0, split), false)); + splitParts.add(new EagerResponseBodyPart(Unpooled.wrappedBuffer(wire, split, wire.length - split), true)); NettyResponse multiple = new NettyResponse(new NettyResponseStatus(null, null, null), null, splitParts); assertEquals(expected, single.getResponseBody(StandardCharsets.UTF_8)); @@ -182,7 +184,10 @@ public void testGetResponseBodyAsBytesDoesNotShareTheBodyPartArray() { public void testGetResponseBodyAsBytesViewReturnsEmptyArray() { NettyResponse response = new NettyResponse(new NettyResponseStatus(null, null, null), null, new LinkedList<>()); - assertArrayEquals(new byte[0], response.getResponseBodyAsBytesView()); + byte[] view = response.getResponseBodyAsBytesView(); + assertArrayEquals(new byte[0], view); + // One shared instance rather than an allocation per call, which is the whole point of the branch. + assertSame(view, response.getResponseBodyAsBytesView()); } @Test @@ -198,6 +203,22 @@ public void testBodylessResponseHasAnEmptyByteBuf() { } } + @Test + public void testOverridingTheViewLeavesTheResponseTextAlone() { + // The string accessors go through a private helper rather than the overridable view, so a subclass that + // hardens the view cannot silently change what this response's text says. + List bodyParts = new LinkedList<>(); + bodyParts.add(new EagerResponseBodyPart(Unpooled.wrappedBuffer("Hello World".getBytes(StandardCharsets.UTF_8)), true)); + NettyResponse response = new NettyResponse(new NettyResponseStatus(null, null, null), null, bodyParts) { + @Override + public byte[] getResponseBodyAsBytesView() { + return "Goodbye".getBytes(StandardCharsets.UTF_8); + } + }; + + assertEquals("Hello World", response.getResponseBody(StandardCharsets.UTF_8)); + } + @Test public void testGetResponseBodyAsBytesViewDefaultImplementationDelegates() throws Throwable { byte[] expected = "Hello World".getBytes(StandardCharsets.UTF_8);