eclipse-vertx / eclipse-vertx/vert.x

HttpClient.send doesn't fail on ReadStream failure

Open
#5,550 7 comments 0 reactions 1 assignee Claimed by @vietj View on GitHub
bug
Dominant language
Java
Stars
14.7k
Forks
2.1k
Avg merge
2d 7h
Merged PRs (30d)
28

Description

### Version

I'm using Vert.x 4.5.14.

### Context

When sending an HTTP Request using a `ReadStream` as input of the request's body, then on `ReadStream` failure the `httpClient.send(readStream)` method ends up in success. Hiding that the request body was corrupted.

### Do you have a reproducer?
Here is a Junit unit test which reproduce the issue:

```java

import java.util.concurrent.CountDownLatch;

import org.junit.jupiter.api.Test;

import io.vertx.codegen.annotations.Nullable;
import io.vertx.core.Handler;
import io.vertx.core.Vertx;
import io.vertx.core.buffer.Buffer;
import io.vertx.core.http.HttpMethod;
import io.vertx.core.http.HttpServer;
import io.vertx.core.streams.ReadStream;

class MyTest {

@Test
void test() throws Exception {
HttpServer server = consumeIncomingRequestAndReturnOk();
CountDownLatch latch = new CountDownLatch(1);
try {
Vertx.vertx()
.createHttpClient()
.request(HttpMethod.POST, server.actualPort(), "localhost", "/")
.compose(request -> request.send(new FailingReadStream()))
.onComplete(response -> System.out.println("Worked although it should not"),
Throwable::printStackTrace)
.onComplete(h -> latch.countDown());
latch.await(); // As the request.send never completes, this hangs forever.
} finally {
server.close();
}
}

private HttpServer consumeIncomingRequestAndReturnOk() throws Exception {
CountDownLatch latch = new CountDownLatch(1);
HttpServer server = io.vertx.core.Vertx.vertx().createHttpServer()
.requestHandler(req ->
req.bodyHandler(buff -> req.response()
.setStatusCode(200)
.end()));
server.listen(0).onSuccess(startedServer -> latch.countDown());
latch.await();
return server;
}

private static final class FailingReadStream implements ReadStream {
Handler exceptionHandler;

@Override
public ReadStream exceptionHandler(@Nullable final Handler handler) {
this.exceptionHandler = handler;
return this;
}

@Override
public ReadStream handler(@Nullable final Handler handler) {
return this;
}

@Override
public ReadStream pause() {
return this;
}

@Override
public ReadStream resume() {
exceptionHandler.handle(new IllegalStateException("Boom"));
return this;
}

@Override
public ReadStream fetch(final long amount) {
exceptionHandler.handle(new IllegalStateException("Boom"));
return this;
}

@Override
public ReadStream endHandler(@Nullable final Handler endHandler) {
return this;
}
}
}

```

### Investigation

If I understood correctly, the issue is caused by [this line](https://github.com/eclipse-vertx/vert.x/blob/25128445394d6306ad8e053527a024387359d7ec/vertx-core/src/main/java/io/vertx/core/http/HttpClientRequest.java#L365) that ignores the result of pipeTo future (which is a failed future):
```java
default Future send(ReadStream body) {
MultiMap headers = headers();
if (headers == null || !headers.contains(HttpHeaders.CONTENT_LENGTH)) {
setChunked(true);
}
body.pipeTo(this); // <--- Here
return response();
}
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.