Upgrde to Tomcat 8.5.39 breaks customized error controller

[crmky]

We upgrade our applications from Spring Boot 1.5.19.RELEASE to 1.5.20.RELEASE, and found the customized error controller is broken in this release because of Tomcat upgrade.

Tomcat 8.5.39 introduces a regression, apache/tomcat@d30d3e3#diff-95185e588de3604778aaae00c0afdf10. Now org.apache.coyote.Response.setStatus method will check previous status code and don't overwrite if the previous status is larger than 399.

    public void setStatus(int status) {
        if (this.status > 399) {
            // Don't overwrite first recorded error status
            return;
        }
        this.status = status;
    }

This causes problem for Spring Boot. For example, DefaultHandlerExceptionResolver may set the status to 400 for MissingServletRequestParameterException. But in our customized error controller, if we set the status to something else, the new status will be silently ignored.

[philwebb]

@crmky Can you provide a sample that show what you are doing in your customized error controller? Specifically I'm interested to know if you're using setStatus instead of sendError.

The javadoc on HttpServletResponse seems to suggest that setStatus should only be used when there is no error:

This method is used to set the return status code when there is no error (for example, for the SC_OK or SC_MOVED_TEMPORARILY status codes).

If this method is used to set an error code, then the container's error page mechanism will not be triggered. If there is an error and the caller wishes to invoke an error page defined in the web application, then sendError must be used instead.

[crmky]

@philwebb We use setStatus in the error controller. But even if I change it to setError, the result is still the same. setError will also call org.apache.coyote.Response.setStatus which will silently ignore the value.

I will provide an example later.

[crmky]

This is an example I created for that issue. Run with Spring Boot 1.5.19.RELEASE, the status code for "/" is 422. Run with Spring Boot 1.5.20.RELEASE, the status code is 400.

@SpringBootApplication
public class Application {

    @RestController
    public static class TestController {

        @RequestMapping("/")
        public String test(@RequestParam String a) {
            // request for / without parameter
            return a;
        }

    }

    @Configuration
    public static class TestConfiguration {

        @Bean
        public BasicErrorController errorController(ErrorAttributes errorAttributes,
                ServerProperties serverProperties) {
            return new BasicErrorController(errorAttributes, serverProperties.getError()) {

                @Override
                protected HttpStatus getStatus(HttpServletRequest request) {
                    // always return 422 in this example
                    return HttpStatus.UNPROCESSABLE_ENTITY;
                }
            };
        }
    }

    public static void main(String[] args) {
        SpringApplication.run(Application.class, args);
    }

}

[philwebb]

@crmky Thanks for the sample. @markt-asf Is there any way to overwrite the status in such circumstances. Is it not legal to call setStatus more than once with different codes?

[markt-asf]

Known bug. I missed the fix for the regression when I back-ported a bunch of error handling improvements from 9.0.x to 8.5.x. The fix is already in place in 8.5.x for 8.5.40 onwards.

[philwebb]

Thanks @markt-asf!

[philwebb]

@crmky I'll close this one and we'll get the fix when we upgrade (see #16504). You can override the version to downgrade until then.