Spring If controller method has produces="*/*" in 5.2.3 response is 500 instead of 406

With the following commit, if a RequestMapping specifies produces="*/*and the request headers don't specify any accept, it will override the response with a 500 as the code below suggests :

https://github.com/spring-projects/spring-framework/commit/34d32402d352fe7bca2f45d3bc5c7243459ec556#diff-24571dc5c7743b2a69f7a5df4f2109b2R316

Comment From: smaldini

Can reproduce with a simple controller:

@RestController
@RequestMapping("foo")
public class ResourceController {
    @GetMapping(produces = MediaType.ALL_VALUE)
    public void foo(final HttpServletResponse response) throws IOException {
        response.setHeader("Content-Range", "bytes */foo");
        response.sendError(HttpServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE); //416
    }
}

 RestAssured
            .given(//**.....**//)
            .header(HttpHeaders.RANGE, "bytes=10000-")
            .when()
            .port(this.port)
            .get("/foo")
            .then()
           .statusCode(Matchers.is(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE.value())); //fails

Comment From: smaldini

In spring 5.2.2 the behavior didn't seem all too right neither in fact, because instead of returning 416 status it would return 406 because content type doesn't match. Is it on purpose ?

Comment From: rstoyanchev

Yes, overriding 406 (Not Acceptable) with a 500 has some recent history.

Starting in 5.2 and #23205, if a controller method returns a ResponseEntity with a "Content-Type" set, then the controller has effectively fixed the content-type (rather than allow the content negotiation algorithm determine it). In that case if there is no suitable converter, it's a server-side error.

In 5.2.3 and #23287, this was further extended for the case where a controller method declares what it produces, and yet fails to produce it. Arguably through we should be double checking that the declared mime type is concrete, so I'm scheduling a follow-up fix.

That said, a produces="*/*" is rather unexpected here since a controller method supports all media types by default. Is there a reason for declaring it like that?

Comment From: rstoyanchev

On closer look, the problem is not what seems.

This happens after Boot's BasicErrorController tries to render the error. It fails because the request attribute HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE, which reflects the media types from the target controller method's producible condition, is still lingering.

Typically this attribute would be removed in case of an exception from the controller, so as to not interfere with error response rendering, but in this case the controller method operates directly on the response and calls sendError. We can clear this attribute in all cases after controller method handling.

Keep in mind however, this fix wouldn't help because on some Servlet container, sendError works as a forward and we wouldn't even have a chance to do anything until it's too late. So generally I would advise against handling the response directly like this unless it's really necessary. If you must however, as a workaround you can do this:

@GetMapping(produces = MediaType.ALL_VALUE)
public void foo(final HttpServletRequest request, final HttpServletResponse response) throws IOException {
    response.setHeader("Content-Range", "bytes */foo");
    request.removeAttribute(HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE); // <---
    response.sendError(HttpServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE); 
}

Note also that Spring MVC does support HTTP ranges. If you return a Resource it will automatically handle requests for specific ranges:

@GetMapping(produces = MediaType.ALL_VALUE)
public Resource foo() {
    Resource resource = ... ;
    return resource;
}

Comment From: rstoyanchev

I should also note with the above workaround in place, and Boot 2.2.4, I get a 416 response.

Comment From: rstoyanchev

You should also see the following https://github.com/spring-projects/spring-boot/issues/19522 which is coming in Boot 2.3 as an improvement so that error detail rendering does not end up overriding the original error response if it fails to find a compatible media type (say client wants text, but we want to render a JSON error response).