Hibernate Validator: Wrong MediaType resolution in Resteasy Classic

[Sgitario]

Describe the bug

The hibernate validator extension only takes into account the @Produces annotation set at Method/Class level. The accept header set by users is ignored. For example, having:

@GET
@Path("/rest-end-point-validation/{id}/")
@Produces({ MediaType.APPLICATION_JSON, MediaType.TEXT_PLAIN })
public String testRestEndPointValidation(@Digits(integer = 5, fraction = 0) @RestPath("id") String id) {
    return id;
}

When there is validation errors, the result will always be JSON, regardless if the user calls it with the HTTP header ACCEPT=text/plain.

Expected behavior

The hibernate validator should take into account the header ACCEPT to serialise the validator errors.
This affects to the RESTEASY classic and reactive extensions. Issue for fixing the Resteasy Reactive extension: #20888

Actual behavior

No response

How to Reproduce?

1.- git clone https://github.com/Sgitario/quarkus-test-suite
2.- cd quarkus-test-suite
3.- git checkout reproducer_20859
4.- cd http-minimum
5.- mvn clean verify to use 999-SNAPSHOT
or mvn clean verify -Dquarkus.platform.version=2.4.0.CR1 to use 2.4.0.CR1

See commit Sgitario/quarkus-test-suite@fbad513

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

No response

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

No response

Additional information

No response

[quarkus-bot]

/cc @FroMage, @geoand, @gsmet, @stuartwdouglas, @yrodiere

[geoand]

I don't see this being the case with RESTEasy Reactive. I am seeing the validation report being serialized properly according to the Accept header

[yrodiere]

There's been changes around this recently (#20230), which I think affected both Resteasy classic and Resteasy Reactive. @Sgitario which version of Quarkus are you using?

Quarkus definitely takes the Accept header into account. It's just that, for error messages, it doesn't support all media types, so sometimes you will get something different from what you requested.

Quarkus used to return HTML (text/html) error messages when you requested a media type that we don't support; now it returns text/json, because we decided it was more human-readable (#20198).

You can change the default with the configuration property quarkus.http.unhandled-error-content-type-default (must be either json or html).

While I believe JSON is better than HTML in this case, I'll grant you it's not much more human-readable, though; especially stacktraces. It would make sense to support plain text error messages, and maybe even make that the default.

[yrodiere]

Ah, it seems I mixed things up again.

My message above is mostly relevant for generic internal errors (HTTP status code 5xx, message generated by QuarkusErrorHandler.java), so validation errors occurring during the service execution.

Validation errors of REST endpoint input (HTTP status code 4xx instead of 5xx) are handled by different code (ResteasyViolationExceptionMapper.java), but they should still respect the Accept header, and they probably have the same problem that they only support JSON/XML.

One thing that changed recently (#20141) is that internal errors are no longer handled by ResteasyViolationExceptionMapper.java. That code used to always return text/plain error messages for internal errors, regardless of the Accept header, while QuarkusErrorHandler.java takes the Accept header into account for internal errors, but does not support text/plain.

So, yeah, it's complex. A reproducer would definitely help pinpoint what it is that is bothering you exactly.

[Sgitario]

Ah, it seems I mixed things up again.

My message above is mostly relevant for generic internal errors (HTTP status code 5xx, message generated by QuarkusErrorHandler.java), so validation errors occurring during the service execution.

Validation errors of REST endpoint input (HTTP status code 4xx instead of 5xx) are handled by different code (ResteasyViolationExceptionMapper.java), but they should still respect the Accept header, and they probably have the same problem that they only support JSON/XML.

One thing that changed recently (#20141) is that internal errors are no longer handled by ResteasyViolationExceptionMapper.java. That code used to always return text/plain error messages for internal errors, regardless of the Accept header, while QuarkusErrorHandler.java takes the Accept header into account for internal errors, but does not support text/plain.

So, yeah, it's complex. A reproducer would definitely help pinpoint what it is that is bothering you exactly.

Thanks for your inputs! I'm working in a reproducer, I will update this issue asap.

[Sgitario]

@yrodiere @geoand I've updated the issue with the reproducer.

Basically, what the scenarios are doing is to deploy custom classes with selected extensions:

@QuarkusScenario
public class HttpUsingJsonResteasyReactiveIT {

    @QuarkusApplication(classes = { Hello.class, HelloReactiveResource.class }, dependencies = {
            @Dependency(artifactId = "quarkus-resteasy-reactive-jackson")
    })
    static final RestService app = new RestService();

    @Test
    public void validateDefaultMediaType() {
        given().get("/reactive/validate-no-produces/boom")
                .then()
                .statusCode(HttpStatus.SC_BAD_REQUEST)
                // if JSON library is present, default is JSON
                .contentType(ContentType.JSON)
                .body("parameterViolations[0].message", containsString("numeric value out of bounds"));
    }
}

And I could find the following failures:

Resteasy Classic Failures:
- Using JSON and XML extensions, regardless the ACCEPT header is, it always returns the JSON validation object.
- Using XML extension, the default response is in TEXT (using JSON extension, the default is JSON as expected) -> I don't think this is a major issue though.
- Using XML extension, if an ACCEPT header is set to either TEXT or XML, the response content type is always empty.


Resteasy Reactive Failures:
- When no default produces, the response content type is TEXT but the object is JSON.
- When using accept = text, the response is JSON

Moreover, I managed to fix these failures in the hibernate-validator extension. However, I prefer to wait for your thoughts before raising a PR.

[Sgitario]

I linked the PR that I mentioned before.
The changes included in the PR will fix most of the failures from the reproducer but:

• In Resteasy Classic, using the XML extension, the default content type for validation errors is TEXT.
• In Resteasy Reactive, when no extensions, the default content type for validation errors is JSON.

But I guess these are not issues, but the expected behaviour.