ErrorPageErrorHandler does not use the proper attributes for error handling

[jebeaudet]

Jetty version(s)
12.0.14

Jetty Environment
ee10

Java version/vendor
openjdk 21.0.3 2024-04-16 LTS

OS type/version
MacOS

Description
I'm upgrading an app from Jetty 10 to 12 and I'm running into some unexpected errors being mapped by jetty. I'm using the latest Spring Boot 3.3 version with no special custom configuration.

Basically I have a request that is clearly a poor hacking attempt on the path //WEB-INF/classes/META-INF/microprofile-config.properties. This is correctly mapped to a 404 by Jetty, however during error handling, I end up with a blank 500.

Tracing into the code, I see that I end up in the ErrorPageErrorHandler class here : https://github.com/jetty/jetty.project/blob/jetty-12.0.x/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorPageErrorHandler.java#L63. This fails because it can't find any Throwable(which is normal AFAICT) but it cannot find a errorStatusCode either so it falls back on the generic error page.

Walking up the stack trace, I see that it's called from the Response interface https://github.com/jetty/jetty.project/blob/jetty-12.0.x/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java#L605 with an ErrorHandler.ErrorRequest instance. The ErrorRequest instance has the proper information (the status code + message) but it's under the keys in the ErrorHandler class :

Now, I understand that the servlet + server part has been decoupled completely and that the ErrorPageErrorHandler is correctly looking at the servlet spec keys to find the exception or the status code. However, shouldn't this class also look at the server specific keys as a fallback?

Thanks in advance!

[jebeaudet]

I've made a small repo to reproduce but it's really simple : https://github.com/jebeaudet/jetty-gh-12505

Checkout the code, build and run and just go to http://localhost:8080/WEB-INF/classes/META-INF/microprofile-config.properties and you'll get a 500 instead of what I would expect a 404.

Digging into it some more, I see it's because the WEB-INF and META-INF are protected path! So it really seems like a corner case since a request to /potato will properly end up in 404 with the same ErrorPageErrorHandler, though the statusCode is properly set in those cases.

[janbartel]

@jebeaudet thanks for the repro, will look into it. I've recently fixed the ErrorHandler/ErrorPageErrorHandler for ee11 in jetty-12.1.x to ensure only the core attributes are used, see #11982. I'll look at porting that to ee10 and maybe backporting to jetty-12.0.x ee10.

[janbartel]

@jebeaudet do you have an ErrorPageErrorHandler mapped with a code or with an exception or both? I can't tell from the repro.

[jebeaudet]

@janbartel By default (as it's the case in my repro sample), Spring will inject a global error handler here. This mapper uses the special org.eclipse.jetty.server.error_page.global key to map a /error handler. No other mappings will be present.

[janbartel]

@jebeaudet not being familiar with spring implementation, I'm getting lost down in spring calling stack in the debugger. So I tried creating a repro case as a jetty unit test. The code is here:

    @Test
    public void testErrorPageErrorHandler() throws Exception
    {
        ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.NO_SECURITY | ServletContextHandler.NO_SESSIONS);
        contextHandler.setContextPath("/foo");
        contextHandler.setBaseResourceAsPath(Path.of("/tmp"));
        ServletHolder defaultHolder = new ServletHolder(new DefaultServlet());
        defaultHolder.setDisplayName("default");

        contextHandler.addServlet(defaultHolder, "/");
        contextHandler.addServlet(ErrorDumpServlet.class, "/error/*");
        ErrorPageErrorHandler errorPageErrorHandler = new ErrorPageErrorHandler();
        //errorPageErrorHandler.addErrorPage(404, "/error/TestException");
        errorPageErrorHandler.addErrorPage(ErrorPageErrorHandler.GLOBAL_ERROR_PAGE, "/error/TestException");
        contextHandler.setErrorHandler(errorPageErrorHandler);
        startServer(contextHandler);

        try (StacklessLogging stackless = new StacklessLogging(ServletChannel.class))
        {
            StringBuilder rawRequest = new StringBuilder();
            rawRequest.append("GET /foo/this/does/not/exist").append(" HTTP/1.1\r\n");
            rawRequest.append("Host: test\r\n");
            rawRequest.append("Connection: close\r\n");
            rawRequest.append("\r\n");

            String rawResponse = _connector.getResponse(rawRequest.toString());

            HttpTester.Response response = HttpTester.parseResponse(rawResponse);
            System.err.println(response);
            System.err.println(response.getContent());
        }
    }

    private void startServer(ServletContextHandler servletContextHandler) throws Exception
    {
        _server = new Server();
        _connector = new LocalConnector(_server);
        _server.addConnector(_connector);

        // Add regression test for double dispatch
        servletContextHandler.addFilter(EnsureSingleDispatchFilter.class, "/*", EnumSet.allOf(DispatcherType.class));
        _server.setHandler(servletContextHandler);
        //_server.setDumpAfterStart(true);
        _server.start();
    }

And I can't seem to reproduce the error 500, I always get error 404 handled correctly. So it seems this might only happen with the spring integration? Could you maybe mutate my test code to more closely match what spring is doing so I can reproduce the error so I can debug it easily?

[janbartel]

@jebeaudet never mind, I've found it! Converted it to a WebAppContext, which has the protections on WEB-INF and I was able to generate the problem. I think I've got enough to work with now:

@Test
    public void testErrorPage() throws Exception
    {
        WebAppContext contextHandler = new WebAppContext();
        contextHandler.setContextPath("/foo");
        contextHandler.setBaseResourceAsPath(Path.of("/tmp"));
        ServletHolder defaultHolder = new ServletHolder(new DefaultServlet());
        defaultHolder.setDisplayName("default");

        contextHandler.addServlet(defaultHolder, "/");
        contextHandler.addServlet(ErrorDumpServlet.class, "/error/*");
        contextHandler.addServlet(GlobalErrorDumpServlet.class, "/global/*");
        ErrorPageErrorHandler errorPageErrorHandler = new ErrorPageErrorHandler();
        errorPageErrorHandler.addErrorPage(404, "/error/TestException");
        errorPageErrorHandler.addErrorPage(ErrorPageErrorHandler.GLOBAL_ERROR_PAGE, "/global/TestException");
        contextHandler.setErrorHandler(errorPageErrorHandler);
        Server server = new Server();
        server.setHandler(contextHandler);

        LocalConnector connector = new LocalConnector(server);
        server.addConnector(connector);
        server.start();

        try (StacklessLogging stackless = new StacklessLogging(ServletChannel.class))
        {
            StringBuilder rawRequest = new StringBuilder();
            rawRequest.append("GET /foo/WEB-INF/classes/this/does/not/exist").append(" HTTP/1.1\r\n");
            rawRequest.append("Host: test\r\n");
            rawRequest.append("Connection: close\r\n");
            rawRequest.append("\r\n");

            String rawResponse = connector.getResponse(rawRequest.toString());

            HttpTester.Response response = HttpTester.parseResponse(rawResponse);
            System.err.println(response);
            System.err.println(response.getContent());
        }
    }