HttpClient fails when response is chunked, but no body is sent

[paulbakker]

I'm using HttpClient to invoke a third party REST API. Using a curl on the resource in question gives the following:

HTTP/1.1 204 No Content
Transfer-Encoding: chunked

Vert.x doesn't like the combination of chunked and "no content", which can be reproduced with the test below:

public class VertxTest {
    public static void main(String[] args) {
        Vertx vertx = Vertx.vertx();
        vertx.createHttpServer().requestHandler(r -> {
            r.response().setChunked(true).setStatusCode(204).end();
        }).listen(9888, (status) -> {
            vertx.createHttpClient().postAbs("http://localhost:9888").handler(r -> {
                System.out.println(r);
            }).end();

        });
    }
}

The following exception is thrown.

SEVERE: Unhandled exception
java.lang.IllegalStateException: No response handler
    at io.vertx.core.http.impl.ClientConnection.handleResponse(ClientConnection.java:275)
    at io.vertx.core.http.impl.HttpClientImpl$ClientHandler.doMessageReceived(HttpClientImpl.java:869)
    at io.vertx.core.http.impl.HttpClientImpl$ClientHandler.doMessageReceived(HttpClientImpl.java:847)
    at io.vertx.core.http.impl.VertxHttpHandler.lambda$channelRead$20(VertxHttpHandler.java:80)
    at io.vertx.core.http.impl.VertxHttpHandler$$Lambda$21/288971877.run(Unknown Source)
    at io.vertx.core.impl.ContextImpl.lambda$wrapTask$18(ContextImpl.java:333)
    at io.vertx.core.impl.ContextImpl$$Lambda$9/752209249.run(Unknown Source)
    at io.vertx.core.impl.ContextImpl.executeFromIO(ContextImpl.java:225)
    at io.vertx.core.http.impl.VertxHttpHandler.channelRead(VertxHttpHandler.java:80)
    at io.vertx.core.net.impl.VertxHandler.channelRead(VertxHandler.java:124)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:318)
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:304)
    at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:276)
    at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:263)
    at io.netty.channel.CombinedChannelDuplexHandler.channelRead(CombinedChannelDuplexHandler.java:147)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:318)
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:304)
    at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:846)
    at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:131)
    at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:511)
    at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:468)
    at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:382)
    at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:354)
    at io.netty.util.concurrent.SingleThreadEventExecutor$2.run(SingleThreadEventExecutor.java:112)
    at java.lang.Thread.run(Thread.java:745)

Switching setChunked to false fixes the problem.

[vietj]

hi, can you check with current Vert.x 3.3 snapshot ?

[paulbakker]

Just retested with the snapshot in https://oss.sonatype.org/content/repositories/snapshots. Still the same problem.

[vietj]

this response is invalid and it confuses the Netty parser.

For Netty a 204 response cannot be chunked so it thinks there is no content whatsoever and the following bytes (the empty chunked message) is interpreted by Netty as an invalid response.

To me the server you access is misbehaving.

I will try to see if there is a workaround to this, i.e tolerate an 204 response with chunked content header.

[vietj]

see in HttpObjectDecoder#isContentAlwaysEmpty:

    protected boolean isContentAlwaysEmpty(HttpMessage msg) {
        if (msg instanceof HttpResponse) {
            HttpResponse res = (HttpResponse) msg;
            int code = res.status().code();

            // Correctly handle return codes of 1xx.
            //
            // See:
            //     - http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html Section 4.4
            //     - https://github.com/netty/netty/issues/222
            if (code >= 100 && code < 200) {
                // One exception: Hixie 76 websocket handshake response
                return !(code == 101 && !res.headers().contains(HttpHeaderNames.SEC_WEBSOCKET_ACCEPT)
                         && res.headers().contains(HttpHeaderNames.UPGRADE, HttpHeaderValues.WEBSOCKET, true));
            }

            switch (code) {
            case 204: case 205: case 304:
                return true;
            }
        }
        return false;
    }

[paulbakker]

@vietj FYI I agree with that the server is misbehaving, already created an issue in that project: Graylog2/graylog2-server#2276

Would be nice at least get a easier to debug error though, it's not very obvious. (I understand this is mostly Netty's fault)

[vietj]

and it's not possible to override this behavior in Netty as the class HttpClientCodec is final

[vietj]

so finally I think the problem we can solve is that Vert.x is confused by this behavior because it receives two responses (one is an error response created by invalidMessage) and we should try to make Vert.x handle properly this case at least

[vietj]

my opinion is that in such case, we should mark the connection as "stale" and try to handle the potential pipelined responses but not put in back in the connection pool.

[vietj]

will contribute to that tomorrow!

[paulbakker]

Great, no hurries though, it's not blocking at all. Good improvement to avoid the same issue for others though :)

[vietj]

cf also in HttpResponseDecoder:

    @Override
    protected HttpMessage createInvalidMessage() {
        return new DefaultFullHttpResponse(HttpVersion.HTTP_1_0, UNKNOWN_STATUS, validateHeaders);
    }

that generates this new extra response instead of doing a failure.

if I remember correctly this behavior has been debated in a Netty issue.

[vietj]

so when vertx gets such error it should just ignore it, specially if it's a HTTP_1_1 connection. HTTP_1_0 does not do keep-alive, so there is no ambiguity.

[vietj]

hi, here is a fix for this error: https://github.com/eclipse/vert.x/tree/invalid-http-response

in your case you should not receive any error on the request/response as it happens after the http response is received and it should be logged.

if you are using pipelining, the pipelined requests should error.

in all cases the connection will be closed as considered as unusable.

[paulbakker]

Running the same test now gives SEVERE: java.lang.IllegalArgumentException: invalid version format: 0

[vietj]

yes, this is expected, as I said before "it should be logged".

if you use pipelining, the next request will get the error.

[vietj]

one idea could be to have this error flow in the HttpConnection handler as it would be interesting to notify as this place too, before logging