From 48938e2d99d51c8226f60b045b9a7a4520ed65da Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Mon, 22 Jun 2020 13:54:20 +0200
Subject: [PATCH] Fix memory leak in AbstractDiskHttpData when CompositeByteBuf
 is used (#10360)

Motivation:

AbstractDiskHttpData may cause a memory leak when a CompositeByteBuf is used. This happened because we may call copy() but actually never release the newly created ByteBuf.

Modifications:

- Remove copy() call and just use ByteBuf.getBytes(...) which will internally handle the writing to the FileChannel without any extra copies that need to be released later on.
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/10354
---
 .../http/multipart/AbstractDiskHttpData.java  |  5 +--
 .../http/multipart/DiskFileUploadTest.java    | 37 +++++++++++++++++++
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractDiskHttpData.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractDiskHttpData.java
index cf7ba5eaf9..3eac4ff99d 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractDiskHttpData.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractDiskHttpData.java
@@ -156,7 +156,6 @@ public abstract class AbstractDiskHttpData extends AbstractHttpData {
                     throw new IOException("Out of size: " + (size + localsize) +
                             " > " + definedSize);
                 }
-                ByteBuffer byteBuffer = buffer.nioBufferCount() == 1 ? buffer.nioBuffer() : buffer.copy().nioBuffer();
                 int written = 0;
                 if (file == null) {
                     file = tempFile();
@@ -165,9 +164,7 @@ public abstract class AbstractDiskHttpData extends AbstractHttpData {
                     RandomAccessFile accessFile = new RandomAccessFile(file, "rw");
                     fileChannel = accessFile.getChannel();
                 }
-                while (written < localsize) {
-                    written += fileChannel.write(byteBuffer);
-                }
+                buffer.getBytes(buffer.readerIndex(), fileChannel, fileChannel.position(), localsize);
                 size += localsize;
                 buffer.readerIndex(buffer.readerIndex() + written);
             } finally {
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DiskFileUploadTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DiskFileUploadTest.java
index 7a9429ba30..8fd665dbb4 100644
--- a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DiskFileUploadTest.java
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DiskFileUploadTest.java
@@ -17,6 +17,7 @@ package io.netty.handler.codec.http.multipart;
 
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.ByteBufInputStream;
+import io.netty.buffer.ByteBufUtil;
 import io.netty.buffer.Unpooled;
 import io.netty.util.CharsetUtil;
 import io.netty.util.internal.PlatformDependent;
@@ -182,6 +183,42 @@ public class DiskFileUploadTest {
         }
     }
 
+    @Test
+    public void testAddContentFromByteBuf() throws Exception {
+        testAddContentFromByteBuf0(false);
+    }
+
+    @Test
+    public void testAddContentFromCompositeByteBuf() throws Exception {
+        testAddContentFromByteBuf0(true);
+    }
+
+    private static void testAddContentFromByteBuf0(boolean composite) throws Exception {
+        DiskFileUpload f1 = new DiskFileUpload("file3", "file3", "application/json", null, null, 0);
+        try {
+            byte[] bytes = new byte[4096];
+            PlatformDependent.threadLocalRandom().nextBytes(bytes);
+
+            final ByteBuf buffer;
+
+            if (composite) {
+                buffer = Unpooled.compositeBuffer()
+                        .addComponent(true, Unpooled.wrappedBuffer(bytes, 0 , bytes.length / 2))
+                        .addComponent(true, Unpooled.wrappedBuffer(bytes, bytes.length / 2, bytes.length / 2));
+            } else {
+                buffer = Unpooled.wrappedBuffer(bytes);
+            }
+            f1.addContent(buffer, true);
+            ByteBuf buf = f1.getByteBuf();
+            assertEquals(buf.readerIndex(), 0);
+            assertEquals(buf.writerIndex(), bytes.length);
+            assertArrayEquals(bytes, ByteBufUtil.getBytes(buf));
+        } finally {
+            //release the ByteBuf
+            f1.delete();
+        }
+    }
+
     private static byte[] doReadFile(File file, int maxRead) throws Exception {
         FileInputStream fis = new FileInputStream(file);
         try {