Don't keep slices around that could be overwritten

Author: yawkat

multipart-core/src/main/java/io/netty/contrib/multipart/MultipartDecoder.java

@@ -204,7 +204,7 @@ public Event next() {
                         if (normal == 0) {
                             return null;
                         }
-                        undecodedPartData = buffer.readRetainedSlice(normal);
+                        undecodedPartData = buffer.readBytes(normal);
                         addReceivedLength(normal);
                         return Event.CONTENT;
                     }

multipart-core/src/main/java/io/netty/contrib/multipart/UrlEncodedDecoder.java

@@ -130,7 +130,7 @@ public Event next() {
                         } else if (valueEnd == buffer.readerIndex()) {
                             return null;
                         } else {
-                            undecodedContent = buffer.readRetainedSlice(valueEnd - buffer.readerIndex());
+                            undecodedContent = buffer.readBytes(valueEnd - buffer.readerIndex());
                         }
                         if (quirks.contains(DecoderQuirk.EARLY_CRLF_CHECK) && buffer != null) {
                             earlyEolCheck(buffer.readerIndex());

multipart-core/src/test/java/io/netty/contrib/multipart/MultipartDecoderTest.java

@@ -21,7 +21,14 @@
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
 import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ThreadLocalRandom;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 
 class MultipartDecoderTest {
 
@@ -193,4 +200,56 @@ private static int findDelimiter(int readerOffset, String input, String delimite
             b.release();
         }
     }
+
+    @Test
+    public void bufferCompaction() throws IOException {
+        byte[] fullData = new byte[10 * 1024 * 1024];
+        ThreadLocalRandom.current().nextBytes(fullData);
+
+        bufferCompaction(PostBodyDecoder.builder().forMultipartBoundary("a"), "--a\r\n\r\n", fullData, "\r\n--a--");
+    }
+
+    static void bufferCompaction(PostBodyDecoder decoder, String before, byte[] fullData, String after) throws IOException {
+        // this test verifies that buffers returned by decodedContent remain unchanged over time. This tests for a bug
+        // caused by incorrect discardSomeReadBytes calls
+
+        List<ByteBuf> readData = new ArrayList<>();
+
+        try (decoder) {
+            decoder.add(Unpooled.copiedBuffer(before, StandardCharsets.UTF_8));
+
+            for (int i = 0; i < fullData.length / 1024; i++) {
+                decoder.add(Unpooled.copiedBuffer(fullData, i * 1024, 1024));
+
+                while (true) {
+                    PostBodyDecoder.Event event = decoder.next();
+                    if (event == null) {
+                        break;
+                    } else if (event == PostBodyDecoder.Event.CONTENT) {
+                        readData.add(decoder.decodedContent());
+                    }
+                }
+            }
+
+            decoder.add(Unpooled.copiedBuffer(after, StandardCharsets.UTF_8));
+            decoder.endInput();
+
+            while (true) {
+                PostBodyDecoder.Event event = decoder.next();
+                if (event == null) {
+                    break;
+                } else if (event == PostBodyDecoder.Event.CONTENT) {
+                    readData.add(decoder.decodedContent());
+                }
+            }
+        }
+
+        ByteArrayOutputStream combined = new ByteArrayOutputStream(fullData.length);
+        for (ByteBuf buf : readData) {
+            buf.readBytes(combined, buf.readableBytes());
+            buf.release();
+        }
+
+        assertArrayEquals(fullData, combined.toByteArray());
+    }
 }

multipart-core/src/test/java/io/netty/contrib/multipart/UrlEncodedDecoderTest.java

@@ -5,7 +5,9 @@
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
+import java.io.IOException;
 import java.nio.charset.StandardCharsets;
+import java.util.concurrent.ThreadLocalRandom;
 
 class UrlEncodedDecoderTest {
     private void expectField(PostBodyDecoder decoder, String name, String value) {
@@ -101,4 +103,19 @@ public void spacesValue() {
             Assertions.assertNull(decoder.next());
         }
     }
+
+    @Test
+    public void bufferCompaction() throws IOException {
+        byte[] fullData = new byte[10 * 1024 * 1024];
+        for (int i = 0; i < fullData.length; i++) {
+            // avoid *valid* escape sequences, but we still need some invalid ones to trigger buffering behavior
+            int c = ThreadLocalRandom.current().nextInt('f', 'z' + 1);
+            if (c == 'f') {
+                c = '%';
+            }
+            fullData[i] = (byte) c;
+        }
+
+        MultipartDecoderTest.bufferCompaction(PostBodyDecoder.builder().forUrlEncodedData(), "xyz=", fullData, "");
+    }
 }