From 01e8100496a0ddac8a2a7f576d69612849fb6237 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Wed, 4 Dec 2019 13:58:03 +0100
Subject: [PATCH] Prevent any leaks when HttpPostStandardRequestDecoder
 constructor throws (#9837)

Motivation:

HttpPostStandardRequestDecoder may throw multiple different exceptions in the constructor which could lead to memory leaks. We need to guard against this by explicit catch all of them and rethrow after we released any allocated memory.

Modifications:

- Catch, destroy and rethrow in any case
- Ensure we correctly wrap IllegalArgumentExceptions
- Add unit tests

Result:

Fixes https://github.com/netty/netty/issues/9829
---
 .../HttpPostStandardRequestDecoder.java       |  9 +++++++--
 .../multipart/HttpPostRequestDecoderTest.java | 20 +++++++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostStandardRequestDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostStandardRequestDecoder.java
index f5d3247ef0..904154b3b4 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostStandardRequestDecoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostStandardRequestDecoder.java
@@ -26,6 +26,7 @@ import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.EndOfDataDec
 import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.ErrorDataDecoderException;
 import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.MultiPartStatus;
 import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.NotEnoughDataDecoderException;
+import io.netty.util.internal.PlatformDependent;
 
 import java.io.IOException;
 import java.nio.charset.Charset;
@@ -158,9 +159,9 @@ public class HttpPostStandardRequestDecoder implements InterfaceHttpPostRequestD
                 undecodedChunk = buffer();
                 parseBody();
             }
-        } catch (HttpPostRequestDecoder.ErrorDataDecoderException e) {
+        } catch (Throwable e) {
             destroy();
-            throw e;
+            PlatformDependent.throwException(e);
         }
     }
 
@@ -487,6 +488,10 @@ public class HttpPostStandardRequestDecoder implements InterfaceHttpPostRequestD
             // error while decoding
             undecodedChunk.readerIndex(firstpos);
             throw new ErrorDataDecoderException(e);
+        } catch (IllegalArgumentException e) {
+            // error while decoding
+            undecodedChunk.readerIndex(firstpos);
+            throw new ErrorDataDecoderException(e);
         }
     }
 
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoderTest.java
index 2c3bfe7ed8..18df6267c0 100644
--- a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoderTest.java
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoderTest.java
@@ -736,6 +736,26 @@ public class HttpPostRequestDecoderTest {
         }
     }
 
+    @Test(expected = HttpPostRequestDecoder.ErrorDataDecoderException.class)
+    public void testNotLeakDirectBufferWhenWrapIllegalArgumentException() {
+        testNotLeakWhenWrapIllegalArgumentException(Unpooled.directBuffer());
+    }
+
+    @Test(expected = HttpPostRequestDecoder.ErrorDataDecoderException.class)
+    public void testNotLeakHeapBufferWhenWrapIllegalArgumentException() {
+        testNotLeakWhenWrapIllegalArgumentException(Unpooled.buffer());
+    }
+
+    private static void testNotLeakWhenWrapIllegalArgumentException(ByteBuf buf) {
+        buf.writeCharSequence("==", CharsetUtil.US_ASCII);
+        FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/", buf);
+        try {
+            new HttpPostStandardRequestDecoder(request);
+        } finally {
+            assertTrue(request.release());
+        }
+    }
+
     @Test
     public void testMultipartFormDataContentType() {
         HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/");