Fix fetching body content after buffer has been read (#1778)

Netflix · Jun 13, 2024 · 2f95d29 · 2f95d29
1 parent f189e61
commit 2f95d29
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 12 deletions.
diff --git a/zuul-core/src/main/java/com/netflix/zuul/message/ZuulMessage.java b/zuul-core/src/main/java/com/netflix/zuul/message/ZuulMessage.java
@@ -59,13 +59,15 @@ public interface ZuulMessage extends Cloneable {
     void setHasBody(boolean hasBody);
 
     /**
-     * Returns the message body.  If there is no message body, this returns {@code null}.
+     * Returns the message body.
+     * This is the entire buffered body, regardless of whether the underlying body chunks have been read or not.
+     * If there is no message body, this returns {@code null}.
      */
     @Nullable
     byte[] getBody();
 
     /**
-     * Returns the length of the message body, or {@code 0} if there isn't a message present.
+     * Returns the length of the entire buffered message body, or {@code 0} if there isn't a message present.
      */
     int getBodyLength();
 

diff --git a/zuul-core/src/main/java/com/netflix/zuul/message/ZuulMessageImpl.java b/zuul-core/src/main/java/com/netflix/zuul/message/ZuulMessageImpl.java
@@ -147,16 +147,13 @@ public byte[] getBody() {
             return null;
         }
 
-        int size = 0;
-        for (final HttpContent chunk : bodyChunks) {
-            size += chunk.content().readableBytes();
-        }
+        int size = this.getBodyLength();
         final byte[] body = new byte[size];
         int offset = 0;
         for (final HttpContent chunk : bodyChunks) {
             final ByteBuf content = chunk.content();
-            final int len = content.readableBytes();
-            content.getBytes(content.readerIndex(), body, offset, len);
+            final int len = content.writerIndex(); // writer idx tracks the total readable bytes in the buffer
+            content.getBytes(0, body, offset, len);
             offset += len;
         }
         return body;
@@ -166,7 +163,8 @@ public byte[] getBody() {
     public int getBodyLength() {
         int size = 0;
         for (final HttpContent chunk : bodyChunks) {
-            size += chunk.content().readableBytes();
+            // writer index tracks the total number of bytes written to the buffer regardless of buffer reads
+            size += chunk.content().writerIndex();
         }
         return size;
     }

diff --git a/zuul-core/src/test/java/com/netflix/zuul/message/ZuulMessageImplTest.java b/zuul-core/src/test/java/com/netflix/zuul/message/ZuulMessageImplTest.java
@@ -16,15 +16,17 @@
 
 package com.netflix.zuul.message;
 
-import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import com.netflix.zuul.context.SessionContext;
 import io.netty.buffer.Unpooled;
 import io.netty.handler.codec.http.DefaultHttpContent;
 import io.netty.handler.codec.http.DefaultLastHttpContent;
 import io.netty.handler.codec.http.HttpContent;
+import java.nio.charset.StandardCharsets;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.junit.jupiter.MockitoExtension;
@@ -179,8 +181,49 @@ void testResettingBodyReaderIndex() {
             c.content().readerIndex(c.content().capacity());
         }
 
-        assertArrayEquals(new byte[0], msg.getBody(), "body should be empty as readerIndex at end of buffers");
+        for (HttpContent c : msg.getBodyContents()) {
+            assertFalse(c.content().isReadable());
+            assertEquals(0, c.content().readableBytes());
+        }
+
         msg.resetBodyReader();
-        assertEquals("Hello World!", new String(msg.getBody()));
+
+        for (HttpContent c : msg.getBodyContents()) {
+            assertTrue(c.content().isReadable());
+            assertTrue(c.content().readableBytes() > 0);
+        }
+    }
+
+    @Test
+    void testFetchingBodyReturnsEntireBuffer() {
+        final ZuulMessage msg = new ZuulMessageImpl(new SessionContext(), new Headers());
+        msg.bufferBodyContents(new DefaultHttpContent(Unpooled.copiedBuffer("Hello ".getBytes())));
+        msg.bufferBodyContents(new DefaultLastHttpContent(Unpooled.copiedBuffer("World!".getBytes())));
+
+        // move the reader indexes to the end of the content buffers
+        for (HttpContent c : msg.getBodyContents()) {
+            c.content().readerIndex(c.content().capacity());
+        }
+
+        // ensure body returns entire chunk content irregardless of reader index movement above
+        assertEquals(12, msg.getBodyLength());
+        assertEquals("Hello World!", new String(msg.getBody(), StandardCharsets.UTF_8));
+
+        // buffer more content and ensure body returns entire chunk content
+        msg.bufferBodyContents(new DefaultLastHttpContent(Unpooled.copiedBuffer(" Bye".getBytes())));
+
+        assertEquals(16, msg.getBodyLength());
+        assertEquals("Hello World! Bye", new String(msg.getBody(), StandardCharsets.UTF_8));
+    }
+
+    @Test
+    void testFetchingEmptyBody() {
+        final ZuulMessage msg = new ZuulMessageImpl(new SessionContext(), new Headers());
+        assertEquals(0, msg.getBodyLength());
+        assertNull(msg.getBody());
+
+        msg.bufferBodyContents(new DefaultHttpContent(Unpooled.copiedBuffer("".getBytes())));
+        assertEquals(0, msg.getBodyLength());
+        assertEquals(0, msg.getBody().length);
     }
 }