jetty / jetty.project

Eclipse Jetty® - Web Container & Clients - supports HTTP/2, HTTP/1.1, HTTP/1.0, websocket, servlets, and more
https://eclipse.dev/jetty
Other
3.83k stars 1.91k forks source link

Use of Handler.Wrapper without a child handler should produce a clear error message. #12300

Open joakime opened 1 day ago

joakime commented 1 day ago

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.

janbartel commented 1 day ago

@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 commented 1 day ago

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.