Skip to content

Commit

Permalink
Validate content length early during HttpServerResponse#sendFile
Browse files Browse the repository at this point in the history
In `HttpServerResponse#sendFile`, when the offset is greater than the file length, the
calculated content length is negative. Currently, the value is not validated until after
the head (response status and other headers) is written. An application might have
downstream processing that would act on the `IllegalArgumentException` thrown when
validating the content length. However, it is too late for the downstream processor
to write a different response status (for example a 400 BAD REQUEST), to signal the
client that the request is not valid. The problem is exacerbated when the downstream
processor tries to write a status code, because the `IllegalStateException` exception
will be thrown because the `Response head is already sent`. This leaves the connection
hanging on the server side, while the client is expecting content.
  • Loading branch information
frankgh authored and vietj committed Feb 20, 2024
1 parent bee2f5d commit 7cd496b
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 4 deletions.
13 changes: 13 additions & 0 deletions src/main/java/io/vertx/core/http/impl/Http1xServerResponse.java
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,19 @@ private void doSendFile(String filename, long offset, long length, Handler<Async
}

long contentLength = Math.min(length, file.length() - offset);

// fail early before status code/headers are written to the response
if (contentLength < 0) {
if (resultHandler != null) {
ContextInternal ctx = vertx.getOrCreateContext();
Exception exception = new IllegalArgumentException("offset : " + offset + " is larger than the requested file length : " + file.length());
ctx.runOnContext((v) -> 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);
Expand Down
11 changes: 10 additions & 1 deletion src/main/java/io/vertx/core/http/impl/Http2ServerResponse.java
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
3 changes: 0 additions & 3 deletions src/test/java/io/vertx/core/http/Http2ServerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
58 changes: 58 additions & 0 deletions src/test/java/io/vertx/core/http/HttpTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 7cd496b

Please sign in to comment.