Skip to content

Fix multipart decoder cleanup on abnormal termination #6088

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shblue21 wants to merge 4 commits into
base: master
Choose a base branch
from
+142 −23
Open

Fix multipart decoder cleanup on abnormal termination #6088

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f2271ce
Fix multipart decoder cleanup on abnormal termination
shblue21 Apr 28, 2026
adea3e5
Refine multipart decoder cleanup
shblue21 Apr 30, 2026
11dde21
Clarify multipart cleanup test sequencing
shblue21 May 4, 2026
f830830
Merge branch 'master' into fix/issue-6014-multipart-decoder-cleanup
shblue21 May 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 48 additions & 11 deletions vertx-core/src/main/java/io/vertx/core/http/impl/HttpServerRequestImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,17 +141,31 @@ public void handleHeaders(HttpRequestHead headers) {

private void handleException(Throwable cause) {
boolean notify;
HttpPostRequestDecoder decoderToCleanup;
synchronized (connection) {
notify = !ended;
decoderToCleanup = notify ? postRequestDecoder : null;
}
if (notify) {
notifyException(cause);
try {
if (notify) {
notifyException(cause);
}
response.handleException(cause);
} finally {
cleanupPostRequestDecoder(decoderToCleanup);
}
response.handleException(cause);
}

private void handleClosed(Void v) {
response.handleClose(v);
HttpPostRequestDecoder decoderToCleanup;
synchronized (connection) {
decoderToCleanup = postRequestDecoder;
}
try {
response.handleClose(v);
} finally {
cleanupPostRequestDecoder(decoderToCleanup);
}
}

private void notifyException(Throwable failure) {
Expand All @@ -178,8 +192,7 @@ public void handleData(Buffer data) {
} catch (HttpPostRequestDecoder.ErrorDataDecoderException |
HttpPostRequestDecoder.TooLongFormFieldException |
HttpPostRequestDecoder.TooManyFormFieldsException e) {
postRequestDecoder.destroy();
postRequestDecoder = null;
cleanupPostRequestDecoder();
handleException(e);
}
}
Expand Down Expand Up @@ -215,8 +228,7 @@ public void handleTrailers(MultiMap trailers) {
} catch (Exception e) {
handleException(e);
} finally {
postRequestDecoder.destroy();
postRequestDecoder = null;
cleanupPostRequestDecoder();
}
}
handler = eventHandler;
Expand All @@ -228,13 +240,38 @@ public void handleTrailers(MultiMap trailers) {

public void handleReset(long errorCode) {
boolean notify;
HttpPostRequestDecoder decoderToCleanup;
synchronized (connection) {
notify = !ended;
decoderToCleanup = postRequestDecoder;
}
try {
if (notify) {
notifyException(new StreamResetException(errorCode));
}
response.handleReset(errorCode);
} finally {
cleanupPostRequestDecoder(decoderToCleanup);
}
if (notify) {
notifyException(new StreamResetException(errorCode));
}

private void cleanupPostRequestDecoder() {
HttpPostRequestDecoder decoderToCleanup;
synchronized (connection) {
decoderToCleanup = postRequestDecoder;
}
cleanupPostRequestDecoder(decoderToCleanup);
}

private void cleanupPostRequestDecoder(HttpPostRequestDecoder decoderToCleanup) {
if (decoderToCleanup != null) {
synchronized (connection) {
if (postRequestDecoder == decoderToCleanup) {
postRequestDecoder = null;
}
}
decoderToCleanup.destroy();
}
response.handleReset(errorCode);
}

private void checkEnded() {
Expand Down
48 changes: 36 additions & 12 deletions vertx-core/src/main/java/io/vertx/core/http/impl/http1/Http1ServerRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -567,8 +567,7 @@ private void onData(Buffer data) {
} catch (HttpPostRequestDecoder.ErrorDataDecoderException |
HttpPostRequestDecoder.TooLongFormFieldException |
HttpPostRequestDecoder.TooManyFormFieldsException e) {
decoder.destroy();
decoder = null;
cleanupDecoder();
handleException(e);
}
}
Expand Down Expand Up @@ -617,43 +616,49 @@ private void reportRequestBegin() {
}

private void endDecode() {
Exception failure = null;
try {
decoder.offer(LastHttpContent.EMPTY_LAST_CONTENT);
while (decoder.hasNext()) {
while (failure == null && decoder.hasNext()) {
InterfaceHttpData data = decoder.next();
if (data instanceof Attribute) {
Attribute attr = (Attribute) data;
try {
attributes().add(attr.getName(), attr.getValue());
} catch (Exception e) {
// Will never happen, anyway handle it somehow just in case
handleException(e);
failure = e;
} finally {
attr.release();
}
}
}
if (failure != null) {
handleException(failure);
}
} catch (HttpPostRequestDecoder.ErrorDataDecoderException |
HttpPostRequestDecoder.TooLongFormFieldException |
HttpPostRequestDecoder.TooManyFormFieldsException e) {
handleException(e);
} catch (HttpPostRequestDecoder.EndOfDataDecoderException e) {
// ignore this as it is expected
} finally {
decoder.destroy();
decoder = null;
cleanupDecoder();
}
}

void handleException(Throwable t) {
HttpEventHandler handler = null;
Http1ServerResponse resp = null;
InterfaceHttpData upload = null;
HttpPostRequestDecoder decoderToCleanup = null;
synchronized (conn) {
if (!isEnded()) {
handler = eventHandler;
if (decoder != null) {
upload = decoder.currentPartialHttpData();
decoderToCleanup = decoder;
Comment thread
shblue21 marked this conversation as resolved.
decoder = null;
}
}
if (!response.ended()) {
Expand All @@ -663,14 +668,33 @@ void handleException(Throwable t) {
resp = response;
}
}
if (resp != null) {
resp.handleException(t);
try {
if (resp != null) {
resp.handleException(t);
}
if (upload instanceof NettyFileUpload) {
((NettyFileUpload) upload).handleException(t);
}
if (handler != null) {
handler.handleException(t);
}
} finally {
cleanupDecoder(decoderToCleanup);
}
if (upload instanceof NettyFileUpload) {
((NettyFileUpload) upload).handleException(t);
}

private void cleanupDecoder() {
HttpPostRequestDecoder decoderToCleanup;
synchronized (conn) {
decoderToCleanup = decoder;
decoder = null;
}
if (handler != null) {
handler.handleException(t);
cleanupDecoder(decoderToCleanup);
Comment thread
shblue21 marked this conversation as resolved.
}

private void cleanupDecoder(HttpPostRequestDecoder decoderToCleanup) {
if (decoderToCleanup != null) {
decoderToCleanup.destroy();
}
}

Expand Down
58 changes: 58 additions & 0 deletions vertx-core/src/test/java/io/vertx/tests/http/fileupload/HttpServerFileUploadTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import io.vertx.test.http.HttpConfig;
import io.vertx.test.http.HttpTestBase;
import io.vertx.test.http.SimpleHttpTest;
import org.junit.Assume;
import org.junit.Rule;
import org.junit.Assert;
import org.junit.Test;
Expand All @@ -33,6 +34,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.BooleanSupplier;
Expand All @@ -56,6 +58,62 @@ public void setUp() throws Exception {
testDir = testFolder.newFolder();
}

@Test
public void testClientResetMultipartUploadCleansDecoder() throws Exception {
Assume.assumeTrue(config.version() == HttpVersion.HTTP_1_1 || config.version() == HttpVersion.HTTP_2);
AtomicReference<HttpClientRequest> clientRequest = new AtomicReference<>();
AtomicBoolean completed = new AtomicBoolean();
server.requestHandler(req -> {
if (req.method() == HttpMethod.POST) {
Context context = Vertx.currentContext();
req.setExpectMultipart(true);
req.exceptionHandler(err -> {
if (!completed.compareAndSet(false, true)) {
return;
}
// Defer until handleException cleanup runs.
context.runOnContext(v -> {
req.headers().set(HttpHeaders.CONTENT_TYPE, "text/plain");
try {
req.setExpectMultipart(true);
Assert.fail("Should throw IllegalStateException");
} catch (IllegalStateException e) {
Assert.assertTrue(e.getMessage(), e.getMessage().contains("valid content-type"));
}
testComplete();
});
});
req.uploadHandler(upload -> {
upload.handler(buffer -> {});
HttpClientRequest request = clientRequest.get();
if (request != null) {
request.reset();
}
});
}
});
startServer(testAddress);

String boundary = "multipart-cleanup";
String body = "--" + boundary + "\r\n" +
"Content-Disposition: form-data; name=\"file\"; filename=\"tmp-0.txt\"\r\n" +
"Content-Type: text/plain\r\n" +
"\r\n" +
"content";

client.request(new RequestOptions(requestOptions)
.setMethod(HttpMethod.POST)
.setURI("/form")).onComplete(TestUtils.onSuccess(request -> {
clientRequest.set(request);
request
.putHeader(HttpHeaders.CONTENT_TYPE, "multipart/form-data; boundary=" + boundary)
.putHeader(HttpHeaders.CONTENT_LENGTH, "999999")
.response().onComplete(ar -> {});
request.write(body);
}));
await();
}

@Test
public void testFormUploadEmptyFile() {
testFormUploadFile("", false, false, false, false);
Expand Down
Loading