Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error attributes not set on request in Jetty 12 #12720

Open
rorytorneymf opened this issue Jan 15, 2025 · 5 comments
Open

Error attributes not set on request in Jetty 12 #12720

rorytorneymf opened this issue Jan 15, 2025 · 5 comments
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@rorytorneymf
Copy link

Jetty version(s)
12.1.0.alpha1

Jetty Environment
ee10

Java version/vendor (use: java -version)
openjdk 17.0.9 2023-10-17
OpenJDK Runtime Environment Temurin-17.0.9+9 (build 17.0.9+9)
OpenJDK 64-Bit Server VM Temurin-17.0.9+9 (build 17.0.9+9, mixed mode, sharing)

Description
I have migrated an application from:

Dropwizard 4.0.10
Jetty 11.0.24

to:

Dropwizard 5.0.0-alpha.5
Jetty 12.1.0.alpha1

In the Jetty 12 version of the application, the error attributes are not getting set on the request, which results in the ErrorPageErrorHandler returning a generic error page rather than my custom error page.

How to reproduce?

I have a Jetty 11 and Jetty 12 version of the application here:

https://github.com/rorytorneymf/jetty11-error-attributes/blob/main/src/main/java/com/example/app/App.java
https://github.com/rorytorneymf/jetty12-error-attributes/blob/main/src/main/java/com/example/app/App.java

To debug, debug the main method with the arguments server app-config-dev.json.

If you debug the Jetty 11 version, setting a breakpoint on the AttributesMap.setAttribute and the ErrorPageErrorHandler.getErrorPage method, then navigate to an invalid URL such as http://localhost:3200/bla you will see that the error attributes are getting set via the HttpChannelState.sendError method:

jetty-11-attribute-getting-set

which then results in this line of code in the ErrorPageErrorHandler returning an appropriate value (403 in this case), which gets mapped to the custom error page:

errorStatusCode = (Integer)request.getAttribute(Dispatcher.ERROR_STATUS_CODE);

However, repeating the above steps in the Jetty 12 version of the app, you will see that the error attributes are not getting set, which then results in this line of code in the ErrorPageErrorHandler returning null, which results in a generic error page being shown rather than my custom error page:

errorStatusCode = (Integer)request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_STATUS);

From what I can tell, the error attributes should be getting set via ServletChannelState.sendError in Jetty 12, but that is not getting called.

Note:

I originally thought that this was the same issue as #12505, but I don't think that's the case, as there does not seem to be any error attributes getting set (either server or servlet attributes).

@rorytorneymf rorytorneymf added the Bug For general bugs on Jetty side label Jan 15, 2025
@janbartel
Copy link
Contributor

@rorytorneymf I am wondering if a recently fixed PR like #12586 may be causing your problem. That hasn't been released yet, but you could try the nightly build of 12.1.0-SNAPSHOT here: https://oss.sonatype.org/content/repositories/jetty-snapshots/org/eclipse/jetty/jetty-home/

@rorytorneymf
Copy link
Author

rorytorneymf commented Jan 16, 2025

Thanks @janbartel I tried the 12.1.0-SNAPSHOT in my sample app but it did not help.

For now I am using the following workaround to set the error attributes manually:

import org.eclipse.jetty.ee10.servlet.ErrorPageErrorHandler;
import org.eclipse.jetty.ee10.servlet.ServletContextRequest;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Response;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.handler.ErrorHandler;
import org.eclipse.jetty.util.Callback;

import jakarta.servlet.RequestDispatcher;
import jakarta.servlet.http.HttpServletRequest;

final class CustomErrorPageErrorHandler extends ErrorPageErrorHandler
{
    private static final boolean JETTY_VERSION_MATCHES_12_0_X = Server.getVersion().matches("^12\\.0\\..*");

    @Override
    public boolean handle(final Request request, final Response response, final Callback callback) throws Exception
    {
        final ServletContextRequest servletContextRequest = Request.as(request, ServletContextRequest.class);
        final HttpServletRequest httpServletRequest = servletContextRequest.getServletApiRequest();

        final int errorStatusCode = response.getStatus();
        final String errorMessage = HttpStatus.getMessage(response.getStatus());

        if (JETTY_VERSION_MATCHES_12_0_X) {
            // Attributes that need to be set in Jetty 12.0.x
            // Copied from version 12.0.16 of this class:
            // https://github.com/jetty/jetty.project/blob/jetty-12.0.16/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannelState.java#L1061
            httpServletRequest.setAttribute(org.eclipse.jetty.ee10.servlet.ErrorHandler.ERROR_CONTEXT,
                    servletContextRequest.getErrorContext());
            httpServletRequest.setAttribute(RequestDispatcher.ERROR_REQUEST_URI,
                    httpServletRequest.getRequestURI());
            httpServletRequest.setAttribute(RequestDispatcher.ERROR_SERVLET_NAME,
                    servletContextRequest.getServletName());
            httpServletRequest.setAttribute(RequestDispatcher.ERROR_STATUS_CODE,
                    errorStatusCode);
            httpServletRequest.setAttribute(RequestDispatcher.ERROR_MESSAGE,
                    errorMessage);

            httpServletRequest.setAttribute(ErrorHandler.ERROR_CONTEXT,
                    servletContextRequest.getServletContext());
            httpServletRequest.setAttribute(ErrorHandler.ERROR_MESSAGE,
                    errorMessage);
            httpServletRequest.setAttribute(ErrorHandler.ERROR_STATUS,
                    errorStatusCode);
        } else {
            // Attributes that need to be set in Jetty >= 12.1.x
            // Copied from version 12.1.0.alpha1 of this class:
            // https://github.com/jetty/jetty.project/blob/jetty-12.1.0.alpha1/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannelState.java#L1072
            httpServletRequest.setAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_STATUS,
                    errorStatusCode);
            httpServletRequest.setAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_CONTEXT,
                    servletContextRequest.getServletContext());
            httpServletRequest.setAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_MESSAGE,
                    errorMessage);
            httpServletRequest.setAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_STATUS,
                    response.getStatus());
            httpServletRequest.setAttribute("org.eclipse.jetty.server.error_origin",
                    servletContextRequest.getServletName());
        }

        return super.handle(request, response, callback);
    }
}

@janbartel
Copy link
Contributor

Hhhm, actually it is the same problem as #12505 (& PR #12586), however that fix was really only applied to ee11 fully, as it required a change to the api for the ErrorHandler/ErrorPageHandler classes.

@lachlan-roberts is working on a different PR that I think backports the ee11 changes to ee10, along with some other changes. I wonder if you could build and test against his branch? His PR is #12698.

@rorytorneymf
Copy link
Author

rorytorneymf commented Jan 17, 2025

Thanks @janbartel I tried that branch but ErrorPageErrorHandler.getErrorPage is not being called because enteredServletChannel==false in this if block:

Image

@janbartel
Copy link
Contributor

@lachlan-roberts can you look at this please? We probably need some changes in the work you're doing to account for this usecase - which is similar actually to the use-case of #12505 where we had not yet entered the servlet handler, but yet need to send an error (because the target was protected).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
None yet
Development

No branches or pull requests

2 participants