Using raw RestResponse return type has unexpected consequences

[knutwannheden]

Describe the bug

Specifying the raw RestResponse type as an endpoint's return type causes Quarkus to configure the ServerStringMessageBodyHandler for this endpoint. This will most often not be what the user wants.

In the following example, where the endpoint returns multipart/form-data responses, but has the raw RestResponse type as its return type, the POJO entity gets serialized using its toString() method rather than using MultipartMessageBodyWriter:

    @GET
    @Produces(MediaType.MULTIPART_FORM_DATA)
    @Path("file2")
    public RestResponse getFile2() {
        return RestResponse.ok(new DownloadFormData("test.txt", new File("test.txt")));
    }

The use case for returning RestResponse is to allow dynamically setting response headers, as documented. Of course the return type should be specified with a generic type parameter.

Expected behavior

Since RestResponse is the counterpart of the JAX-RS Response type, which isn't generic, I think it would make sense if Quarkus would log a build-time usage warning, whenever an endpoint returns the raw RestResponse type.

Actual behavior

Currently there is no warning and Quarkus will "silently" configure the ServerStringMessageBodyHandler.

How to Reproduce?

Create an endpoint with RestResponse as the return type.

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.13.2.Final

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

[quarkus-bot]

/cc @FroMage, @geoand, @stuartwdouglas

[geoand]

We'll should wait for #27526 to be merged before addressing this one

[knutwannheden]

I just noticed that if I supply a generic type parameter (I hadn't noticed that RestResponse was generic 🤦 ) then the response looks a lot better:

❯ curl http://localhost:8080/hello/file2 -i
HTTP/1.1 200 OK
content-length: 322
Content-Type: multipart/form-data

--2b496fef-3c0a-469e-89f3-2ff04b5355d1
Content-Disposition: form-data; name="file"; filename="test.txt"
Content-Type: application/octet-stream

test test
--2b496fef-3c0a-469e-89f3-2ff04b5355d1
Content-Disposition: form-data; name="name"
Content-Type: text/plain

test.txt
--2b496fef-3c0a-469e-89f3-2ff04b5355d1--

rather than:

❯ curl http://localhost:8080/hello/file2 -i
HTTP/1.1 200 OK
content-length: 34
Content-Type: multipart/form-data

org.acme.DownloadFormData@20accfa0

I suppose that would be a good reason to log a warning (or even an error) at build-time if there is no generic type parameter.

Yet, as you may notice, the boundary parameter is missing in the content type. For reference, here is what a proper response looks like:

❯ curl http://localhost:8080/hello/file -i
HTTP/1.1 200 OK
content-length: 322
Content-Type: multipart/form-data;boundary=736e7384-ca9b-4c3b-8cec-5231fe5e1d00

--736e7384-ca9b-4c3b-8cec-5231fe5e1d00
Content-Disposition: form-data; name="file"; filename="test.txt"
Content-Type: application/octet-stream

test test
--736e7384-ca9b-4c3b-8cec-5231fe5e1d00
Content-Disposition: form-data; name="name"
Content-Type: text/plain

test.txt
--736e7384-ca9b-4c3b-8cec-5231fe5e1d00--

The only difference being the Content-Type which is missing the boundary parameter when the return type is RestResponse<DownloadFormData> instead of DownloadFormData.

@geoand I propose to change this issue to log a warning when an endpoint returns RestResponse without supplying a generic type parameter and create a new issue for the problem with the missing boundary parameter on Content-Type. What do you think?

[knutwannheden]

I went ahead and filed #28634 for the problem regarding the Content-Type header.

[geoand]

@geoand I propose to change this issue to log a warning when an endpoint returns RestResponse without supplying a generic type parameter and create a new issue for the problem with the missing boundary parameter on Content-Type. What do you think?

👍🏼

[FroMage]

What's weird is that the @Produces should be used for determining how to serialise the contents of the RestResponse, no? Does the same bug happen when using Response? Intuitively I say this should work OOTB and pick the right MessageBodyWriter, even with a raw RestResponse, no?

[knutwannheden]

When I first looked at it, I was under the impression that this check should maybe also check for RestResponse:

quarkus/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/startup/RuntimeResourceDeployment.java

Line 396 in 3b2e75e

} else if (!Response.class.isAssignableFrom(rawEffectiveReturnType)) {

I could put that theory to the test.

[geoand]

We should definitely at least log a warning.

[geoand]

#28657 adds a warning