quarkus (i.e.Vertx) does not return the correct values for http HEAD

[rsearls]

I have a test that performs a GET and then a HEAD on endpoint,

@GET
@Produces("text/plain")
public String get() {
return "hello";
}

Undertow returns this for GET
HttpResponseProxy{HTTP/1.1 200 OK [
Connection: keep-alive,
Content-Type: text/plain;charset=UTF-8,
Content-Length: 5,
Date: Mon, 17 Feb 2020 20:48:12 GMT]
ResponseEntityProxy{[
Content-Type: text/plain;charset=UTF-8,
Content-Length: 5,
Chunked: false]}}

and it returns this for HEAD
HttpResponseProxy{HTTP/1.1 200 OK [
Connection: keep-alive,
Content-Type: text/plain;charset=UTF-8,
Content-Length: 5,
Date: Mon, 17 Feb 2020 20:50:26 GMT]}

Quarkus returns this for GET
HttpResponseProxy{HTTP/1.1 200 OK [
Content-Type: text/plain;charset=UTF-8,
Content-Length: 5]
ResponseEntityProxy{[
Content-Type: text/plain;charset=UTF-8,
Content-Length: 5,
Chunked: false]}}

and it returns this for HEAD (note "Content-Length: 0" it should be 5)
HttpResponseProxy{HTTP/1.1 200 OK [
Content-Type: text/plain;charset=UTF-8,
Content-Length: 0]}

This project demonstrates the issue https://github.com/rsearls/quarkus-http-HEAD.git
See the project README file for details for running the project

Execution env.
jdk 11.0.2
mvn 3.6.0
quarkus 999-SNAPSHOT

[Karm]

I can confirm that this is a valid problem on the current master; violation of https://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html#sec9.4

@vietj Is this a Vert.x specific thing or are we using Vert.x wrong from within Quarkus?

[Karm]

I tried with plain Vert.x without Quarkus. This is a Vert.x specific behavior, not a Quarkus specific one.

public class MainVerticle extends AbstractVerticle {

  @Override
  public void start(Promise<Void> startPromise) throws Exception {
    vertx.createHttpServer().requestHandler(req -> {
      req.response()
        .putHeader("content-type", "text/plain")
        .end("Hello from Vert.x!");
    }).listen(8888, http -> {
      if (http.succeeded()) {
        startPromise.complete();
        System.out.println("HTTP server started on port 8888");
      } else {
        startPromise.fail(http.cause());
      }
    });
  }
}

$ curl http://localhost:8888/ -i
HTTP/1.1 200 OK
content-type: text/plain
content-length: 18

Hello from Vert.x!

And here it should have been the same except for the body:

$ curl http://localhost:8888/ --head
HTTP/1.1 200 OK
content-type: text/plain

[sberyozkin]

Hi, this is what RestEasy should control, right ? if no @Head method is available then if @GET should be checked in order to construct a Head response as per the JAX-RS spec, as far as I recall

[Karm]

@sberyozkin I don't think it has anything to do with RestEeasy...why should it?

It is caused by a deliberate decision in Vert.x Core here:

https://github.com/eclipse-vertx/vert.x/blob/3.8.5/src/main/java/io/vertx/core/http/impl/HttpServerResponseImpl.java#L639

[Karm]

@sberyozkin

public class MainVerticle extends AbstractVerticle {

  @Override
  public void start(Promise<Void> startPromise) throws Exception {
    vertx.createHttpServer().requestHandler(req -> {
      req.response()
        .putHeader("content-type", "text/plain")
        .end("Hello from Vert.x!");
    }).listen(8888, http -> {
      if (http.succeeded()) {
        startPromise.complete();
        System.out.println("HTTP server started on port 8888");
      } else {
        startPromise.fail(http.cause());
      }
    });
  }
}

Unpatched Vert.x 3.8.5:

$ curl -X HEAD http://localhost:8888/ --insecure --head
HTTP/1.1 200 OK
content-type: text/plain

Patched Vert.x 3.8.5.karm:

+++ b/src/main/java/io/vertx/core/http/impl/HttpServerResponseImpl.java
@@ -635,8 +635,8 @@ public class HttpServerResponseImpl implements HttpServerResponse {
     } else if (version == HttpVersion.HTTP_1_1 && !keepAlive) {
       headers.set(HttpHeaders.CONNECTION, HttpHeaders.CLOSE);
     }
-    if (head || status == HttpResponseStatus.NOT_MODIFIED) {
-      // For HEAD request or NOT_MODIFIED response
+    if (status == HttpResponseStatus.NOT_MODIFIED) {
+      // For NOT_MODIFIED response
       // don't set automatically the content-length
       // and remove the transfer-encoding
       headers.remove(HttpHeaders.TRANSFER_ENCODING);

$ curl -X HEAD http://localhost:8888/ --insecure --head
HTTP/1.1 200 OK
content-type: text/plain
content-length: 18

So...I'll open it with Vert.x Github repo and see what they say...

This is really not about any optimization. All the body content is pushed all the way to Netty buffers and processed; it is just not returned (which is fine for HEAD). Since we know the length anyway, there is IMHO no reason not to spit out the Content-Length.

[sberyozkin]

HI @Karm Thanks for all this information.

So what would happen if one has:

@GET
@Produces("text/plain")
public String get() {
return "hello";
}
@HEAD
public String head() {
    return "hello";
}

? Will this method be even invoked with Vertx now supporting Quarkus ? The other thing I mentioned earlier, the JAX-RS spec requires that if no @HEAD method exists but @GET on the same path is available, then that @GET response minus any payload is what should be returned, I'm pretty sure a TCK test exists to check it.
In other words, it is the JAX-RS application which controls the HEAD response. There should be some spec text around it too.
Thanks

[Karm]

@sberyozkin Lemme probe that.

[Karm]

@sberyozkin

Vert.x http server used from Quarkus

With patched Vert.x core:

+++ b/src/main/java/io/vertx/core/http/impl/HttpServerResponseImpl.java
@@ -635,8 +635,8 @@ public class HttpServerResponseImpl implements HttpServerResponse {
     } else if (version == HttpVersion.HTTP_1_1 && !keepAlive) {
       headers.set(HttpHeaders.CONNECTION, HttpHeaders.CLOSE);
     }
-    if (head || status == HttpResponseStatus.NOT_MODIFIED) {
-      // For HEAD request or NOT_MODIFIED response
+    if (status == HttpResponseStatus.NOT_MODIFIED) {
+      // For NOT_MODIFIED response
       // don't set automatically the content-length
       // and remove the transfer-encoding
       headers.remove(HttpHeaders.TRANSFER_ENCODING);

This test passes just fine HTTPCasesTest.java

It fails with vanilla 3.8.5, that is used in the current Quarkus.

JAX-RS used from Quarkus

This is what JAX-RS 2 spec tells us:

3.3.5 HEAD and OPTIONS

HEAD and OPTIONS requests receive additional automated support. On receipt of a HEAD request an implementation MUST either:

1. Call a method annotated with a request method designator for HEAD or, if none present,
2. Call a method annotated with a request method designator for GET and discard any returned entity.

Note that option 2 may result in reduced performance where entity creation is significant.
On receipt of an OPTIONS request an implementation MUST either:

1. Call a method annotated with a request method designator for OPTIONS or, if none present,
2. Generate an automatic response using the metadata provided by the JAX-RS annotations on the matching class and its methods.
   

   Appendix B, HTTP Header Support

Content-Length

Processed automatically for requests, set automatically in responses if value is provided by the MessageBodyWriter used to serialize the message entity.

...so I assume MessageBodyWriter for String should do it, right?

I added some tests here: ApplicationPathHttpRootTest.java.

And they both fall on their faces right away, despite running with patched Vert.x core:

[ERROR] Tests run: 3, Failures: 0, Errors: 2, Skipped: 0, Time elapsed: 1.543 s <<< FAILURE! - in io.quarkus.resteasy.test.root.ApplicationPathHttpRootTest

[ERROR] testJohnResource  Time elapsed: 0.053 s  <<< ERROR!
java.lang.RuntimeException: 
java.lang.AssertionError: 1 expectation failed.
Expected header "content-length" was not is "10", was "null". Headers are:
Content-Type=text/plain;charset=UTF-8
connection=keep-alive

[ERROR] testHeadResources  Time elapsed: 0.01 s  <<< ERROR!
java.lang.RuntimeException: 
java.lang.AssertionError: 1 expectation failed.
Expected header "content-length" was not is "11", was "null". Headers are:
Content-Type=text/plain;charset=UTF-8
connection=keep-alive

WDYT?

[sberyozkin]

@Karm But why a content-length is tested ? The JAX-RS spec is a bit unclear about it but it implies (when it says @GET body should be stripped when processing @HEAD) that Head does not return a body.

[sberyozkin]

@Karm Hi, what would test well if the JAX-RS methods are invoked is to return Response with only few headers set:

public static class HelloResource {

        @GET
        public Response hello() {
            // when this method is used instead to check Head, the payload must not be returned
            return Response.ok("hello world").type("text/plain").build();
        }

        @HEAD
        public Response head() {
            return Response.type("application/json").build();
        }
}

[sberyozkin]

@Karm Checking around, here for example, suggests the payload related headers, such as Content-Length may be dropped. So we just need to confirm that the headers like Content-Type is set when the JAX-RS methods are invoked. Thanks

[Karm]

@sberyozkin

https://tools.ietf.org/html/rfc7231#section-3.3

Explicitly lists Content-Length. You are correct.

The thing is though that Thorntail / Undertow gives you the Content-Length.

Thorntail 2.6

$ curl http://localhost:8080/data/hello -I --head
HTTP/1.1 200 OK
Connection: keep-alive
Content-Type: application/octet-stream
Content-Length: 11
Date: Thu, 20 Feb 2020 15:36:55 GMT

$ curl http://localhost:8080/data/hello
Hello World

Quarkus 1.2.0

$ curl http://localhost:8080/data/hello -I --head
HTTP/1.1 200 OK
Content-Type: application/octet-stream
connection: keep-alive

Exactly the same app:

@Path("/hello")
@Singleton
public class HelloController {

    @GET
    public String sayHello() {
        return "Hello World";
    }
}

So...if we could, I would align it with what Thorntail/Undertow gives you...

[Karm]

To generate test apps:

curl -O -J 'https://start.microprofile.io/api/project?supportedServer=QUARKUS&artifactId=quarkus'

curl -O -J 'https://start.microprofile.io/api/project?supportedServer=THORNTAIL_V2&artifactId=thorntail'

[sberyozkin]

Hi @Karm, thanks for all the tests. But Quarkus one is more optimal :-), the one from Undertow/Thorntail might cause, in theory at least, some side-effects with some eager clients reacting to Content-Length; especially with having a JAX-RS @Head method returning a body, which in itself is not a correct Head implementation
Thanks

[Karm]

@sberyozkin @rsearls I suggest we close this then.

If anyone complains, e.g. if anyone uses Conent-Length from HEAD to pre-allocate resources (embedded systems) or to check response (enough to know the length); we can iterate back to it. With all the notes it will be easy to pick up the slack.

[sberyozkin]

Sounds good to me, lets see if @rsearls agrees

[rsearls]

I have no objection

[machi1990]

I have no objection

Closing then! Thanks all.