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

Use of RewriteHandler.LastRuleHandler without a child handler should produce a clear error message. #12300

Open
joakime opened this issue Sep 23, 2024 · 2 comments
Labels
Bug For general bugs on Jetty side

Comments

@joakime
Copy link
Contributor

joakime commented Sep 23, 2024

Jetty version(s)
12.0.13

Jetty Environment
Any

Java version/vendor (use: java -version)
Any

OS type/version
Any

Description
If an instance of a Handler.Wrapper without a child is allowed to start and begin handling requests, the error message is unclear.

java.lang.NullPointerException: Cannot invoke "org.eclipse.jetty.server.Request$Handler.handle(org.eclipse.jetty.server.Request, org.eclipse.jetty.server.Response, org.eclipse.jetty.util.Callback)" because "this._handler" is null

We should make the start of this kind of handler fail the start and throw a meaningful message that the Handler.Wrapper is missing a child.

@joakime joakime added the Bug For general bugs on Jetty side label Sep 23, 2024
@janbartel
Copy link
Contributor

@joakime actually the issue is not coming from Handler.Wrapper, this is a problem purely with the RewriteHandler.LastRuleHandler, which while it might be a Handler.Wrapper isn't using the inherited handler, but rather actually defines it's own _handler here: https://github.com/jetty/jetty.project/blob/jetty-12.0.x/jetty-core/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RewriteHandler.java#L148.

So we need a more meaningful message in RewriteHandler.LastRuleHandler if it starts with a null _handler.

It's possible to also put an IllegalStateException into Handler.Wrapper.doStart if the _handler is null but first we would have to check that the Handler.Wrapper instance is !dynamic (as if dynamic is true, starting without a handler set is allowed).

@gregw
Copy link
Contributor

gregw commented Sep 24, 2024

There is a use-case for optional handlers in Handler.Wrapper. The classic example is ServletHandler, which is a Wrapper, but typically does not have a nested Handler. This is only important for deployments without a default servlet.

@janbartel janbartel changed the title Use of Handler.Wrapper without a child handler should produce a clear error message. Use of RewriteHandler.LastRuleHandler without a child handler should produce a clear error message. Sep 30, 2024
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
Status: No status
Development

No branches or pull requests

3 participants