From 06a5173e8d700f5f431557f21f647f2199bb3d52 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Fri, 27 Dec 2019 09:14:50 +0100
Subject: [PATCH] Utf8FrameValidator must release buffer when validation fails
 (#9909)

Motivation:

Utf8FrameValidator must release the input buffer if the validation fails to ensure no memory leak happens

Modifications:

- Catch exception, release frame and rethrow
- Adjust unit test

Result:

Fixes https://github.com/netty/netty/issues/9906
---
 .../http/websocketx/Utf8FrameValidator.java   | 65 ++++++++++---------
 .../WebSocketUtf8FrameValidatorTest.java      |  4 +-
 2 files changed, 38 insertions(+), 31 deletions(-)

diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/Utf8FrameValidator.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/Utf8FrameValidator.java
index 5ce5ec369cb..7edf5cfa86e 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/Utf8FrameValidator.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/Utf8FrameValidator.java
@@ -35,42 +35,47 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
         if (msg instanceof WebSocketFrame) {
             WebSocketFrame frame = (WebSocketFrame) msg;
 
-            // Processing for possible fragmented messages for text and binary
-            // frames
-            if (((WebSocketFrame) msg).isFinalFragment()) {
-                // Final frame of the sequence. Apparently ping frames are
-                // allowed in the middle of a fragmented message
-                if (!(frame instanceof PingWebSocketFrame)) {
-                    fragmentedFramesCount = 0;
+            try {
+                // Processing for possible fragmented messages for text and binary
+                // frames
+                if (((WebSocketFrame) msg).isFinalFragment()) {
+                    // Final frame of the sequence. Apparently ping frames are
+                    // allowed in the middle of a fragmented message
+                    if (!(frame instanceof PingWebSocketFrame)) {
+                        fragmentedFramesCount = 0;
 
-                    // Check text for UTF8 correctness
-                    if ((frame instanceof TextWebSocketFrame) ||
-                            (utf8Validator != null && utf8Validator.isChecking())) {
-                        // Check UTF-8 correctness for this payload
-                        checkUTF8String(frame.content());
+                        // Check text for UTF8 correctness
+                        if ((frame instanceof TextWebSocketFrame) ||
+                                (utf8Validator != null && utf8Validator.isChecking())) {
+                            // Check UTF-8 correctness for this payload
+                            checkUTF8String(frame.content());
 
-                        // This does a second check to make sure UTF-8
-                        // correctness for entire text message
-                        utf8Validator.finish();
-                    }
-                }
-            } else {
-                // Not final frame so we can expect more frames in the
-                // fragmented sequence
-                if (fragmentedFramesCount == 0) {
-                    // First text or binary frame for a fragmented set
-                    if (frame instanceof TextWebSocketFrame) {
-                        checkUTF8String(frame.content());
+                            // This does a second check to make sure UTF-8
+                            // correctness for entire text message
+                            utf8Validator.finish();
+                        }
                     }
                 } else {
-                    // Subsequent frames - only check if init frame is text
-                    if (utf8Validator != null && utf8Validator.isChecking()) {
-                        checkUTF8String(frame.content());
+                    // Not final frame so we can expect more frames in the
+                    // fragmented sequence
+                    if (fragmentedFramesCount == 0) {
+                        // First text or binary frame for a fragmented set
+                        if (frame instanceof TextWebSocketFrame) {
+                            checkUTF8String(frame.content());
+                        }
+                    } else {
+                        // Subsequent frames - only check if init frame is text
+                        if (utf8Validator != null && utf8Validator.isChecking()) {
+                            checkUTF8String(frame.content());
+                        }
                     }
-                }
 
-                // Increment counter
-                fragmentedFramesCount++;
+                    // Increment counter
+                    fragmentedFramesCount++;
+                }
+            } catch (CorruptedWebSocketFrameException e) {
+                frame.release();
+                throw e;
             }
         }
 
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketUtf8FrameValidatorTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketUtf8FrameValidatorTest.java
index c3bb0ed7dd3..84428f3f39d 100644
--- a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketUtf8FrameValidatorTest.java
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketUtf8FrameValidatorTest.java
@@ -36,8 +36,9 @@ public void testCorruptedFrameExceptionInCheck() {
 
     private void assertCorruptedFrameExceptionHandling(byte[] data) {
         EmbeddedChannel channel = new EmbeddedChannel(new Utf8FrameValidator());
+        TextWebSocketFrame frame = new TextWebSocketFrame(Unpooled.copiedBuffer(data));
         try {
-            channel.writeInbound(new TextWebSocketFrame(Unpooled.copiedBuffer(data)));
+            channel.writeInbound(frame);
             Assert.fail();
         } catch (CorruptedFrameException e) {
             // expected exception
@@ -51,5 +52,6 @@ private void assertCorruptedFrameExceptionHandling(byte[] data) {
             buf.release();
         }
         Assert.assertNull(channel.readOutbound());
+        Assert.assertEquals(0, frame.refCnt());
     }
 }