From 787a85f9f1b816ee901c1ec00348ae2007bb9d3b Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Wed, 30 Apr 2014 07:38:42 +0200 Subject: [PATCH] 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. --- .../ContinuationWebSocketFrame.java | 36 ++------------ .../codec/http/websocketx/UTF8Exception.java | 47 ------------------ .../{UTF8Output.java => Utf8Validator.java} | 49 +++++++++---------- .../websocketx/WebSocket08FrameDecoder.java | 40 +++++++-------- 4 files changed, 45 insertions(+), 127 deletions(-) delete mode 100644 codec-http/src/main/java/io/netty/handler/codec/http/websocketx/UTF8Exception.java rename codec-http/src/main/java/io/netty/handler/codec/http/websocketx/{UTF8Output.java => Utf8Validator.java} (81%) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/ContinuationWebSocketFrame.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/ContinuationWebSocketFrame.java index 9d590a5d02..fa452f546d 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/ContinuationWebSocketFrame.java +++ b/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 diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/UTF8Exception.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/UTF8Exception.java deleted file mode 100644 index 9580b72073..0000000000 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/UTF8Exception.java +++ /dev/null @@ -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 - * - * 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); - } -} diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/UTF8Output.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/Utf8Validator.java similarity index 81% rename from codec-http/src/main/java/io/netty/handler/codec/http/websocketx/UTF8Output.java rename to codec-http/src/main/java/io/netty/handler/codec/http/websocketx/Utf8Validator.java index fa9564b3ea..d4a612fcbf 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/UTF8Output.java +++ b/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; - - UTF8Output(ByteBuf buffer) { - stringBuilder = new StringBuilder(buffer.readableBytes()); - write(buffer); + public void check(ByteBuf buffer) { + checking = true; + buffer.forEachByte(this); } - public void write(ByteBuf buffer) { - for (int i = buffer.readerIndex(); i < buffer.writerIndex(); i++) { - write(buffer.getByte(i)); + public void finish() { + checking = false; + codep = 0; + if (state != UTF8_ACCEPT) { + state = UTF8_ACCEPT; + throw new CorruptedFrameException("bytes are not UTF-8"); } } - public void write(byte[] bytes) { - for (byte b : bytes) { - write(b); - } - } - - public void write(int b) { + @Override + public boolean process(byte b) throws Exception { byte type = TYPES[b & 0xFF]; codep = state != UTF8_ACCEPT ? b & 0x3f | codep << 6 : 0xff >> type & b; state = STATES[state + type]; - if (state == UTF8_ACCEPT) { - stringBuilder.append((char) codep); - } else if (state == UTF8_REJECT) { - throw new UTF8Exception("bytes are not UTF-8"); + if (state == UTF8_REJECT) { + checking = false; + throw new CorruptedFrameException("bytes are not UTF-8"); } + return true; } - @Override - public String toString() { - if (state != UTF8_ACCEPT) { - throw new UTF8Exception("bytes are not UTF-8"); - } - return stringBuilder.toString(); + public boolean isChecking() { + return checking; } } diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocket08FrameDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocket08FrameDecoder.java index e34a3a2dc4..846b12077a 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocket08FrameDecoder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocket08FrameDecoder.java @@ -81,9 +81,7 @@ public class WebSocket08FrameDecoder extends ReplayingDecoder