Skip to content

Commit

Permalink
HttpPostStandardRequestDecoder leaks memory when constructor throws E…
Browse files Browse the repository at this point in the history
…rrorDataDecoderException.

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 #9513.
  • Loading branch information
normanmaurer committed Aug 27, 2019
1 parent 4c14fa5 commit 2a0f3c0
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,18 @@ public HttpPostStandardRequestDecoder(HttpDataFactory factory, HttpRequest reque
this.request = checkNotNull(request, "request");
this.charset = checkNotNull(charset, "charset");
this.factory = checkNotNull(factory, "factory");
if (request instanceof HttpContent) {
// Offer automatically if the given request is als type of HttpContent
// See #1089
offer((HttpContent) request);
} else {
undecodedChunk = buffer();
parseBody();
try {
if (request instanceof HttpContent) {
// Offer automatically if the given request is als type of HttpContent
// See #1089
offer((HttpContent) request);
} else {
undecodedChunk = buffer();
parseBody();
}
} catch (HttpPostRequestDecoder.ErrorDataDecoderException e) {
destroy();
throw e;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -723,4 +723,15 @@ public void testMultipartRequest() throws Exception {
decoder.destroy();
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());
}
}
}

0 comments on commit 2a0f3c0

Please sign in to comment.