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
This commit is contained in:
Norman Maurer 2019-12-27 09:14:50 +01:00
parent 1e4f0e6a09
commit 961362f43f
2 changed files with 38 additions and 31 deletions

View File

@ -35,42 +35,47 @@ public class Utf8FrameValidator implements ChannelInboundHandler {
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;
}
}

View File

@ -36,8 +36,9 @@ public class WebSocketUtf8FrameValidatorTest {
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 @@ public class WebSocketUtf8FrameValidatorTest {
buf.release();
}
Assert.assertNull(channel.readOutbound());
Assert.assertEquals(0, frame.refCnt());
}
}