HttpPostStandardRequestDecoder leaks memory when constructor throws ErrorDataDecoderException. (#9517)

Motivation:

Currently when HttpPostStandardRequestDecoder throws a ErrorDataDecoderException during construction we leak memory. We need to ensure all is released correctly.

Modifications:

- Call destroy() if parseBody() throws and rethrow the ErrorDataDecoderException
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/9513.
This commit is contained in:
Norman Maurer 2019-08-28 10:27:38 +02:00
parent 88e247ee45
commit 64eb392d70
2 changed files with 23 additions and 7 deletions

View File

@ -149,13 +149,18 @@ public class HttpPostStandardRequestDecoder implements InterfaceHttpPostRequestD
this.request = requireNonNull(request, "request"); this.request = requireNonNull(request, "request");
this.charset = requireNonNull(charset, "charset"); this.charset = requireNonNull(charset, "charset");
this.factory = requireNonNull(factory, "factory"); this.factory = requireNonNull(factory, "factory");
if (request instanceof HttpContent) { try {
// Offer automatically if the given request is als type of HttpContent if (request instanceof HttpContent) {
// See #1089 // Offer automatically if the given request is als type of HttpContent
offer((HttpContent) request); // See #1089
} else { offer((HttpContent) request);
undecodedChunk = buffer(); } else {
parseBody(); undecodedChunk = buffer();
parseBody();
}
} catch (HttpPostRequestDecoder.ErrorDataDecoderException e) {
destroy();
throw e;
} }
} }

View File

@ -723,4 +723,15 @@ public class HttpPostRequestDecoderTest {
decoder.destroy(); decoder.destroy();
assertEquals(1, req.refCnt()); assertEquals(1, req.refCnt());
} }
@Test(expected = HttpPostRequestDecoder.ErrorDataDecoderException.class)
public void testNotLeak() {
FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/",
Unpooled.copiedBuffer("a=1&&b=2", CharsetUtil.US_ASCII));
try {
new HttpPostStandardRequestDecoder(request);
} finally {
assertTrue(request.release());
}
}
} }