Say what the shared body view aliases

Author: hyperxpro

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.
      *
      * <p>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.
      *
      * <p>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

client/src/main/java/org/asynchttpclient/netty/NettyResponse.java

@@ -196,6 +196,8 @@ public byte[] getResponseBodyAsBytes() {
     }
 
     /**
+     * {@inheritDoc}
+     * <p>
      * 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.
      * <p>
-     * 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();

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<HttpResponseBodyPart> 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<HttpResponseBodyPart> 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<HttpResponseBodyPart> 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);