Remove ContinuationWebSocketFrame.aggregatedText()

Motivation:
Before we aggregated the full text in the WebSocket08FrameDecoder just to fill in the ContinuationWebSocketFrame.aggregatedText(). The problem was that there was no upper-limit and so it would be possible to see an OOME if the remote peer sends a TextWebSocketFrame + a never ending stream of ContinuationWebSocketFrames. Furthermore the aggregation does not really belong in the WebSocket08FrameDecoder, as we provide an extra ChannelHandler for this anyway (WebSocketFrameAggregator).

Modification:
Remove the ContinuationWebSocketFrame.aggregatedText() method and corresponding constructor. Also refactored WebSocket08FrameDecoder a bit to me more efficient which is now possible as we not need to aggregate here.

Result:
No more risk of OOME because of frames.

codec-http/src/main/java/io/netty/handler/codec/http/websocketx/ContinuationWebSocketFrame.java

@@ -25,13 +25,11 @@ import io.netty.util.CharsetUtil;
  */
 public class ContinuationWebSocketFrame extends WebSocketFrame {
 
-    private String aggregatedText;
-
     /**
      * Creates a new empty continuation frame.
      */
     public ContinuationWebSocketFrame() {
-        super(Unpooled.buffer(0));
+        this(Unpooled.buffer(0));
     }
 
     /**
@@ -58,25 +56,6 @@ public class ContinuationWebSocketFrame extends WebSocketFrame {
         super(finalFragment, rsv, binaryData);
     }
 
-    /**
-     * Creates a new continuation frame with the specified binary data
-     *
-     * @param finalFragment
-     *            flag indicating if this frame is the final fragment
-     * @param rsv
-     *            reserved bits used for protocol extensions
-     * @param binaryData
-     *            the content of the frame.
-     * @param aggregatedText
-     *            Aggregated text set by decoder on the final continuation frame of a fragmented
-     *            text message
-     */
-    public ContinuationWebSocketFrame(
-            boolean finalFragment, int rsv, ByteBuf binaryData, String aggregatedText) {
-        super(finalFragment, rsv, binaryData);
-        this.aggregatedText = aggregatedText;
-    }
-
     /**
      * Creates a new continuation frame with the specified text data
      *
@@ -88,7 +67,7 @@ public class ContinuationWebSocketFrame extends WebSocketFrame {
      *            text content of the frame.
      */
     public ContinuationWebSocketFrame(boolean finalFragment, int rsv, String text) {
-        this(finalFragment, rsv, fromText(text), null);
+        this(finalFragment, rsv, fromText(text));
     }
 
     /**
@@ -112,21 +91,14 @@ public class ContinuationWebSocketFrame extends WebSocketFrame {
         }
     }
 
-    /**
-     * Aggregated text returned by decoder on the final continuation frame of a fragmented text message
-     */
-    public String aggregatedText() {
-        return aggregatedText;
-    }
-
     @Override
     public ContinuationWebSocketFrame copy() {
-        return new ContinuationWebSocketFrame(isFinalFragment(), rsv(), content().copy(), aggregatedText());
+        return new ContinuationWebSocketFrame(isFinalFragment(), rsv(), content().copy());
     }
 
     @Override
     public ContinuationWebSocketFrame duplicate() {
-        return new ContinuationWebSocketFrame(isFinalFragment(), rsv(), content().duplicate(), aggregatedText());
+        return new ContinuationWebSocketFrame(isFinalFragment(), rsv(), content().duplicate());
     }
 
     @Override

codec-http/src/main/java/io/netty/handler/codec/http/websocketx/UTF8Exception.java

@@ -1,47 +0,0 @@
-/*
- * Copyright 2012 The Netty Project
- *
- * The Netty Project licenses this file to you under the Apache License,
- * version 2.0 (the "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at:
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
- * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
- * License for the specific language governing permissions and limitations
- * under the License.
- */
-/*
- * Adaptation of http://bjoern.hoehrmann.de/utf-8/decoder/dfa/
- *
- * Copyright (c) 2008-2009 Bjoern Hoehrmann <bjoern@hoehrmann.de>
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy of this software
- * and associated documentation files (the "Software"), to deal in the Software without restriction,
- * including without limitation the rights to use, copy, modify, merge, publish, distribute,
- * sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in all copies or
- * substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING
- * BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
- * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
- */
-package io.netty.handler.codec.http.websocketx;
-
-/**
- * Invalid UTF8 bytes encountered
- */
-final class UTF8Exception extends RuntimeException {
-    private static final long serialVersionUID = 1L;
-
-    UTF8Exception(String reason) {
-        super(reason);
-    }
-}

codec-http/src/main/java/io/netty/handler/codec/http/websocketx/Utf8Validator.java

@@ -36,11 +36,13 @@
 package io.netty.handler.codec.http.websocketx;
 
 import io.netty.buffer.ByteBuf;
+import io.netty.buffer.ByteBufProcessor;
+import io.netty.handler.codec.CorruptedFrameException;
 
 /**
- * Checks UTF8 bytes for validity before converting it into a string
+ * Checks UTF8 bytes for validity
  */
-final class UTF8Output {
+final class Utf8Validator implements ByteBufProcessor {
     private static final int UTF8_ACCEPT = 0;
     private static final int UTF8_REJECT = 12;
 
@@ -65,45 +67,38 @@ final class UTF8Output {
     @SuppressWarnings("RedundantFieldInitialization")
     private int state = UTF8_ACCEPT;
     private int codep;
+    private boolean checking;
 
-    private final StringBuilder stringBuilder;
+    public void check(ByteBuf buffer) {
-
+        checking = true;
-    UTF8Output(ByteBuf buffer) {
+        buffer.forEachByte(this);
-        stringBuilder = new StringBuilder(buffer.readableBytes());
-        write(buffer);
     }
 
-    public void write(ByteBuf buffer) {
+    public void finish() {
-        for (int i = buffer.readerIndex(); i < buffer.writerIndex(); i++) {
+        checking = false;
-            write(buffer.getByte(i));
+        codep = 0;
+        if (state != UTF8_ACCEPT) {
+            state = UTF8_ACCEPT;
+            throw new CorruptedFrameException("bytes are not UTF-8");
         }
     }
 
-    public void write(byte[] bytes) {
+    @Override
-        for (byte b : bytes) {
+    public boolean process(byte b) throws Exception {
-            write(b);
-        }
-    }
-
-    public void write(int b) {
         byte type = TYPES[b & 0xFF];
 
         codep = state != UTF8_ACCEPT ? b & 0x3f | codep << 6 : 0xff >> type & b;
 
         state = STATES[state + type];
 
-        if (state == UTF8_ACCEPT) {
+        if (state == UTF8_REJECT) {
-            stringBuilder.append((char) codep);
+            checking = false;
-        } else if (state == UTF8_REJECT) {
+            throw new CorruptedFrameException("bytes are not UTF-8");
-            throw new UTF8Exception("bytes are not UTF-8");
         }
+        return true;
     }
 
-    @Override
+    public boolean isChecking() {
-    public String toString() {
+        return checking;
-        if (state != UTF8_ACCEPT) {
-            throw new UTF8Exception("bytes are not UTF-8");
-        }
-        return stringBuilder.toString();
     }
 }

codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocket08FrameDecoder.java

@@ -92,7 +92,10 @@ public class WebSocket08FrameDecoder extends ByteToMessageDecoder
     private static final byte OPCODE_PING = 0x9;
     private static final byte OPCODE_PONG = 0xA;
 
-    private UTF8Output fragmentedFramesText;
+    private final long maxFramePayloadLength;
+    private final boolean allowExtensions;
+    private final boolean maskedPayload;
+
     private int fragmentedFramesCount;
     private boolean frameFinalFlag;
     private int frameRsv;
@@ -101,11 +104,7 @@ public class WebSocket08FrameDecoder extends ByteToMessageDecoder
     private byte[] maskingKey;
     private int framePayloadLen1;
     private boolean receivedClosingHandshake;
-
+    private Utf8Validator utf8Validator;
-    private final long maxFramePayloadLength;
-    private final boolean allowExtensions;
-    private final boolean maskedPayload;
-
     private State state = State.READING_FIRST;
 
     /**
@@ -311,7 +310,6 @@ public class WebSocket08FrameDecoder extends ByteToMessageDecoder
 
                         // Processing for possible fragmented messages for text and binary
                         // frames
-                        String aggregatedText = null;
                         if (frameFinalFlag) {
                             // Final frame of the sequence. Apparently ping frames are
                             // allowed in the middle of a fragmented message
@@ -319,15 +317,14 @@ public class WebSocket08FrameDecoder extends ByteToMessageDecoder
                                 fragmentedFramesCount = 0;
 
                                 // Check text for UTF8 correctness
-                                if (frameOpcode == OPCODE_TEXT || fragmentedFramesText != null) {
+                                if (frameOpcode == OPCODE_TEXT ||
+                                        (utf8Validator != null && utf8Validator.isChecking())) {
                                     // Check UTF-8 correctness for this payload
                                     checkUTF8String(ctx, payloadBuffer);
 
                                     // This does a second check to make sure UTF-8
                                     // correctness for entire text message
-                                    aggregatedText = fragmentedFramesText.toString();
+                                    utf8Validator.finish();
-
-                                    fragmentedFramesText = null;
                                 }
                             }
                         } else {
@@ -335,13 +332,12 @@ public class WebSocket08FrameDecoder extends ByteToMessageDecoder
                             // fragmented sequence
                             if (fragmentedFramesCount == 0) {
                                 // First text or binary frame for a fragmented set
-                                fragmentedFramesText = null;
                                 if (frameOpcode == OPCODE_TEXT) {
                                     checkUTF8String(ctx, payloadBuffer);
                                 }
                             } else {
                                 // Subsequent frames - only check if init frame is text
-                                if (fragmentedFramesText != null) {
+                                if (utf8Validator != null && utf8Validator.isChecking()) {
                                     checkUTF8String(ctx, payloadBuffer);
                                 }
                             }
@@ -361,7 +357,7 @@ public class WebSocket08FrameDecoder extends ByteToMessageDecoder
                             return;
                         } else if (frameOpcode == OPCODE_CONT) {
                             out.add(new ContinuationWebSocketFrame(frameFinalFlag, frameRsv,
-                                    payloadBuffer, aggregatedText));
+                                    payloadBuffer));
                             payloadBuffer = null;
                             return;
                         } else {
@@ -392,11 +388,15 @@ public class WebSocket08FrameDecoder extends ByteToMessageDecoder
     }
 
     private void protocolViolation(ChannelHandlerContext ctx, String reason) {
+        protocolViolation(ctx, new CorruptedFrameException(reason));
+    }
+
+    private void protocolViolation(ChannelHandlerContext ctx, CorruptedFrameException ex) {
         state = State.CORRUPT;
         if (ctx.channel().isActive()) {
             ctx.writeAndFlush(Unpooled.EMPTY_BUFFER).addListener(ChannelFutureListener.CLOSE);
         }
-        throw new CorruptedFrameException(reason);
+        throw ex;
     }
 
     private static int toFrameLength(long l) {
@@ -409,13 +409,12 @@ public class WebSocket08FrameDecoder extends ByteToMessageDecoder
 
     private void checkUTF8String(ChannelHandlerContext ctx, ByteBuf buffer) {
         try {
-            if (fragmentedFramesText == null) {
+            if (utf8Validator == null) {
-                fragmentedFramesText = new UTF8Output(buffer);
+                utf8Validator = new Utf8Validator();
-            } else {
-                fragmentedFramesText.write(buffer);
             }
-        } catch (UTF8Exception ex) {
+            utf8Validator.check(buffer);
-            protocolViolation(ctx, "invalid UTF-8 bytes");
+        } catch (CorruptedFrameException ex) {
+            protocolViolation(ctx, ex);
         }
     }
 
@@ -443,9 +442,9 @@ public class WebSocket08FrameDecoder extends ByteToMessageDecoder
         // May have UTF-8 message
         if (buffer.isReadable()) {
             try {
-                new UTF8Output(buffer);
+                new Utf8Validator().check(buffer);
-            } catch (UTF8Exception ex) {
+            } catch (CorruptedFrameException ex) {
-                protocolViolation(ctx, "Invalid close frame reason text. Invalid UTF-8 bytes");
+                protocolViolation(ctx, ex);
             }
         }