diff --git a/src/main/java/io/vertx/core/http/impl/Http1xServerResponse.java b/src/main/java/io/vertx/core/http/impl/Http1xServerResponse.java index 0f9e7e5156d..7a2fed16a4f 100644 --- a/src/main/java/io/vertx/core/http/impl/Http1xServerResponse.java +++ b/src/main/java/io/vertx/core/http/impl/Http1xServerResponse.java @@ -559,6 +559,19 @@ private void doSendFile(String filename, long offset, long length, Handler resultHandler.handle(Future.failedFuture(exception))); + } else { + log.error("Invalid offset: " + offset); + } + return; + } + bytesWritten = contentLength; if (!headers.contains(HttpHeaders.CONTENT_TYPE)) { String contentType = MimeMapping.getMimeTypeForFilename(filename); diff --git a/src/main/java/io/vertx/core/http/impl/Http2ServerResponse.java b/src/main/java/io/vertx/core/http/impl/Http2ServerResponse.java index c4245336e2f..71c6d075210 100644 --- a/src/main/java/io/vertx/core/http/impl/Http2ServerResponse.java +++ b/src/main/java/io/vertx/core/http/impl/Http2ServerResponse.java @@ -619,7 +619,16 @@ public HttpServerResponse sendFile(String filename, long offset, long length, Ha HttpUtils.resolveFile(stream.vertx, filename, offset, length, ar -> { if (ar.succeeded()) { AsyncFile file = ar.result(); - long contentLength = Math.min(length, file.getReadLength()); + long fileLength = file.getReadLength(); + long contentLength = Math.min(length, fileLength); + + // fail early before status code/headers are written to the response + if (contentLength < 0) { + Exception exception = new IllegalArgumentException("offset : " + offset + " is larger than the requested file length : " + fileLength); + h.handle(Future.failedFuture(exception)); + return; + } + if (headers.get(HttpHeaderNames.CONTENT_LENGTH) == null) { putHeader(HttpHeaderNames.CONTENT_LENGTH, String.valueOf(contentLength)); } diff --git a/src/test/java/io/vertx/core/http/Http2ServerTest.java b/src/test/java/io/vertx/core/http/Http2ServerTest.java index e8253128739..ef62504b770 100644 --- a/src/test/java/io/vertx/core/http/Http2ServerTest.java +++ b/src/test/java/io/vertx/core/http/Http2ServerTest.java @@ -58,7 +58,6 @@ import io.vertx.core.buffer.Buffer; import io.vertx.core.http.impl.Http1xOrH2CHandler; import io.vertx.core.http.impl.HttpUtils; -import io.vertx.core.http.impl.headers.HeadersMultiMap; import io.vertx.core.impl.Utils; import io.vertx.core.streams.ReadStream; import io.vertx.core.streams.WriteStream; @@ -74,9 +73,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.net.InetSocketAddress; -import java.nio.channels.ClosedChannelException; import java.nio.charset.StandardCharsets; -import java.util.ArrayList; import java.util.Arrays; import java.util.Base64; import java.util.Collections; diff --git a/src/test/java/io/vertx/core/http/HttpTest.java b/src/test/java/io/vertx/core/http/HttpTest.java index 9f369da2332..6aee9d8683c 100644 --- a/src/test/java/io/vertx/core/http/HttpTest.java +++ b/src/test/java/io/vertx/core/http/HttpTest.java @@ -2167,6 +2167,64 @@ public void testSendRangeFileFromClasspath() { await(); } + @Test + public void testSendZeroRangeFileFromClasspath() { + server.requestHandler(res -> { + // hosts_config.txt is 23 bytes + res.response().sendFile("hosts_config.txt", 23, 0); + }).listen(testAddress, onSuccess(res -> { + client.request(requestOptions) + .compose(HttpClientRequest::send) + .compose(resp -> { + assertEquals(String.valueOf(0), resp.headers().get("Content-Length")); + return resp.body(); + }) + .onComplete(onSuccess(body -> { + assertEquals("", body.toString()); + assertEquals(0, body.toString().length()); + testComplete(); + })); + })); + await(); + } + + @Test + public void testSendOffsetIsHigherThanFileLengthForFileFromClasspath() { + server.requestHandler(res -> { + try { + // hosts_config.txt is 23 bytes + res.response().sendFile("hosts_config.txt", 33, 10) + .onFailure(throwable -> { + try { + res.response().setStatusCode(500).end(); + } catch (IllegalStateException illegalStateException) { + // should not reach here, the response should not be sent if the offset is negative + fail(illegalStateException); + } + }); + } catch (Exception ex) { + // a route.route().failureHandler() would handle failures during + // handling of the request. Here we simulate that scenario, and during + // handling of failure we would like to set a status code for example. + try { + res.response().setStatusCode(505).end(); + fail("Should not reach here, failures should be handled by res.response().onFailure() above"); + } catch (IllegalStateException exceptionWhenTheResponseIsAlreadySent) { + // should not reach here, the response should not be sent if the offset is negative + fail(exceptionWhenTheResponseIsAlreadySent); + } + } + }).listen(testAddress, onSuccess(res -> { + client.request(requestOptions) + .compose(HttpClientRequest::send) + .onComplete(onSuccess(response -> { + assertEquals(500, response.statusCode()); + testComplete(); + })); + })); + await(); + } + @Test public void test100ContinueHandledAutomatically() { Buffer toSend = TestUtils.randomBuffer(1000);