vert-x3 / vertx-web

HTTP web applications for Vert.x
Apache License 2.0
1.11k stars 535 forks source link

Improving ProtocolUpgradeHandler priority checking #2676

Open tsegismont opened 2 weeks ago

tsegismont commented 2 weeks ago

In Vert.x Web there is a handler priority verification mechanism.

There are different priorities (e.g. authentication, protocol upgrade, user) and, when several handlers are installed on the same route, Vert.x Web throws an exception if the lower priority handler is set after a higher priority handler.

For example:

    router.route("/product*")
      .handler(basicAuthHandler) // BasicAuthHandler
      .handler(productProxyHandler); // ProxyHandler

Leads to:

java.lang.IllegalStateException: Cannot add [PROTOCOL_UPGRADE] handler to route with [AUTHENTICATION] handler at index 0
    at io.vertx.ext.web.impl.RouteState.addContextHandler(RouteState.java:572)
    at io.vertx.ext.web.impl.RouteImpl.handler(RouteImpl.java:143)

As a workaround, the handlers can be set on two different routes with the same configuration:

    router.route("/product*")
      .handler(basicAuthHandler);

    router.route("/product*")
      .handler(productProxyHandler);

In this example, the reason of the failure is that ProxyHandler has priority PROTOCOL_UPGRADE (because it can proxy websockets) but it's added after BasicAuthHandler, which has AUTHENTICATION priority.

A blunt move might be to revert some of the changes introduced in #2616 (i.e. no longer make ProxyHandlerImpl implement the ProtocolUpgradeHandler interface).

But, in this case, it makes sense to do the same with the other ProtocolUpgradeHandler, which is the GraphQLWSHandler. Indeed, in both cases there are good reasons for adding an auth handler when needed.

Any thoughts on this @pmlopes ?