Skip to content

Commit b95f9d6

Browse files
Response not ended on edge error cases (#2909) (#2915)
* Always end the response * Add tests * Reset connection in case of already written response * Test headWritten then reset the connction rather than catching an exception * Renaming * Fix imports * Fix style * Fix style * Fix style Co-authored-by: olivierayache <olivier.ayache@gmail.com>
1 parent b726dde commit b95f9d6

2 files changed

Lines changed: 47 additions & 2 deletions

File tree

vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextImplBase.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,14 +256,21 @@ protected void unhandledFailure(int statusCode, Throwable failure, RouterImpl ro
256256

257257
if (!response().ended() && !response().closed()) {
258258
try {
259-
response().setStatusCode(code);
259+
if (response().headWritten()) {
260+
response().reset(code);
261+
} else {
262+
response().setStatusCode(code);
263+
}
260264
} catch (IllegalArgumentException e) {
261265
// means that there are invalid chars in the status message
262266
response()
263267
.setStatusMessage(HttpResponseStatus.valueOf(code).reasonPhrase())
264268
.setStatusCode(code);
269+
} finally {
270+
if (!response().headWritten()) {
271+
response().end(response().getStatusMessage());
272+
}
265273
}
266-
response().end(response().getStatusMessage());
267274
}
268275
}
269276

vertx-web/src/test/java/io/vertx/ext/web/handler/BodyHandlerTest.java

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import io.vertx.core.buffer.Buffer;
2222
import io.vertx.core.http.HttpHeaders;
2323
import io.vertx.core.http.HttpMethod;
24+
import io.vertx.core.http.HttpServerResponse;
2425
import io.vertx.core.http.RequestOptions;
2526
import io.vertx.core.json.JsonObject;
2627
import io.vertx.ext.web.FileUpload;
@@ -39,6 +40,7 @@
3940
import java.util.Base64;
4041
import java.util.List;
4142
import java.util.concurrent.atomic.AtomicBoolean;
43+
import java.util.concurrent.atomic.AtomicInteger;
4244
import java.util.function.Function;
4345

4446
/**
@@ -432,6 +434,42 @@ private void sendFileUploadRequest(Buffer fileData,
432434
}, statusCode, statusMessage, null);
433435
}
434436

437+
@Test
438+
public void testRoutingContextFailedBeforeDownloaded() throws Exception {
439+
router.get("/failed-download")
440+
.handler((e) -> {
441+
HttpServerResponse response = e.response();
442+
response.setChunked(true);
443+
response.putHeader(HttpHeaders.CONTENT_TYPE, "application/octet-stream");
444+
AtomicInteger count = new AtomicInteger();
445+
vertx.setPeriodic(200, id -> {
446+
response.write(Buffer.buffer(new byte[1024]));
447+
if (count.incrementAndGet() == 5) {
448+
vertx.cancelTimer(id);
449+
e.fail(500, new RuntimeException("Download exception"));
450+
}
451+
});
452+
});
453+
454+
RequestOptions requestOptions = new RequestOptions()
455+
.setHost("localhost")
456+
.setPort(8080)
457+
.setMethod(HttpMethod.GET)
458+
.setURI("/failed-download");
459+
460+
client.request(requestOptions).onComplete(onSuccess(req -> {
461+
req.response(onSuccess(resp -> {
462+
assertEquals(200, resp.statusCode());
463+
resp.exceptionHandler(t -> {
464+
testComplete();
465+
});
466+
}));
467+
req.end();
468+
}));
469+
470+
await();
471+
}
472+
435473
@Test
436474
public void testRoutingContextFailedBeforeFileIsFullyUploaded() throws Exception {
437475
String uploadsDirectory = tempUploads.newFolder().getPath();

0 commit comments

Comments
 (0)