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
This commit is contained in:
Norman Maurer 2019-12-04 13:58:03 +01:00
parent 5223217121
commit 01e8100496
2 changed files with 27 additions and 2 deletions

View File

@ -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);
}
}

View File

@ -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, "/");