Question about SockJS BodyHandler

[Jan Fengler]

Hi all,

we have triggered the following error:
"ERROR No BodyHandler was executed on the route. Please add a BodyHandler before the SockJS handler"
so we have added the following line:
sockjsRouter.route().handler(BodyHandler.create());
Before the Subrouter for SockJS

MainVerticle.getRouter().route("*******").subRouter(sockjsRouter);

The solution seems obvious at first, but in detail there are some questions:

• before the latest changes on Vert.X the BodyHandler was automatically added to only the parts of the WebSocket that are expected to have a body, not to everything (The Handler cannot even be applied only to the subrouter, we have found only the far more generic way described above)
  
• Is this the intended way to do this, or is there a better way to regain the functionality from earlier Versions of Vertx?
• Did we miss something?

Thx
  Jan Fengler

[Paulo Lopes]

Hi,

Like you noticed, previously the sockjs handler would perform most of the required actions to parse the body if needed, however this had several issues:

1. If you would upload a large payload, you could DDoS the server

2. A complex JSON upload could stackoverflow the server due to parsing of JSON

3. Event if the content type was incorrect the body would always be inflated into memory

All these security checks are implemented in body handler so, instead of duplicating the logic, we explicitly re-use the existing handlers that have been tested for secure code.

Cheers,

Paulo

[Jan Fengler]

Hello Paulo,

we do not query the reason of the change, but the code uses a subrouter for JSON and not a separate full router and on subrouter we were not able to add a body handler separately, we need to add it to the main router. Unfortunately that does not allow us to reproduce the behaviour that we had before the change.
This was the reason for us to suspect, that we probably have missed something, because the error message suggested, that we only had to add a body handler to fix the problem. Either the error message is not precise enough (at least for us to understand) or we have really missed the right way to restore exactly the previous behaviour.

If it would be possible to add a body handler to a subrouter, everything would have been clear enough (and the behaviour would have been restored close enough), but because that is not possible, we are currently unsure, if our "solution" is the way it is meant to be used, because of the big differences to the previous behaviour.

So are we doing this the way it is now meant to be used or is there a better way to deal with it?

Thx
  Jan

---

Von: ve...@googlegroups.com <ve...@googlegroups.com> im Auftrag von Paulo Lopes <pml...@gmail.com>
Gesendet: Freitag, 7. Oktober 2022 19:51
An: vert.x <ve...@googlegroups.com>
Betreff: [vertx:54890] Re: Question about SockJS BodyHandler

 

--
You received this message because you are subscribed to the Google Groups "vert.x" group.
To unsubscribe from this group and stop receiving emails from it, send an email to vertx+un...@googlegroups.com.
To view this discussion on the web, visit https://groups.google.com/d/msgid/vertx/4e93ff2c-4371-4661-baae-fb75c95c63edn%40googlegroups.com.

[Paulo Lopes]

Hi,

I do believe you can achive what you're asking with:

router.route("/app/eventbus/*")
  .handler(BodyHandler.create())
  .subRouter(SockJSHandler.create(vertx).bridge(....));

While you don't add the BodyHandler to the sockJS router yourself, you can add it to the route where you're mounting the sockjs bridge. If you really must have it in the sockjs router (which I won't recommend as this might interfere with the router setup) you can do something like:

subRouter = SockJSHandler.create(vertx).bridge(...);
subRouter.route().order(0).handler(BodyHandler.create(...));
router.route("/app/eventbus/*").subRotuer(subRouter);

As this is modifying the order of the mounted routes, it might work as you wish but may also force a reorder of routes that will produce undefined behavior, so use it with caution.

Cheers.

Paulo

---- On Mon, 10 Oct 2022 11:34:14 +0200 Jan Fengler <Jan.F...@adviqo.com> wrote ---

To view this discussion on the web, visit https://groups.google.com/d/msgid/vertx/AM0PR08MB3537FDCDCF10D9368E994D618B209%40AM0PR08MB3537.eurprd08.prod.outlook.com.

[Jan Fengler]

Hi Paolo,

we will try.

Thanks
  Jan

---

Von: Paulo Lopes <pa...@mlopes.net>
Gesendet: Montag, 10. Oktober 2022 12:31
An: vertx <ve...@googlegroups.com>
Cc: Jan Fengler <Jan.F...@adviqo.com>; qta...@adviqo.com <qta...@adviqo.com>
Betreff: Re: AW: [vertx:54890] Re: Question about SockJS BodyHandler

 

[Jan Fengler]

Hi Paulo,

this is the javadoc for the subrouter-Method (from https://vertx.io/docs/apidocs/io/vertx/reactivex/ext/web/Route.html#subRouter-io.vertx.reactivex.ext.web.Router-):

public Route subRouter(Router subRouter)

Use a (sub) Router as a handler. There are several requirements to be fulfilled for this to be accepted.

• The route path must end with a wild card
• Parameters are allowed but full regex patterns not
• No other handler can be registered before or after this call (but they can on a new route object for the same path)
• Only 1 router per path object

Parameters:

subRouter - the router to add

Returns:

a reference to this, so the API can be used fluently

AFAIC currently say, your first proposal seems to work (test ongoing, but no big difference to our own "solution"), but the third bullet point from the javadoc above seems misleading then, because "No other handler can be registered BEFORE or after this call...". Is'nt  that what's happening (and the documentation must be modified) or is the proposed solution using the mentioned exception via a new route object?

Thx
  Jan

---

Von: Jan Fengler <Jan.F...@adviqo.com>
Gesendet: Montag, 10. Oktober 2022 13:04
An: Paulo Lopes <pa...@mlopes.net>; vertx <ve...@googlegroups.com>
Cc: qta...@adviqo.com <qta...@adviqo.com>
Betreff: AW: AW: [vertx:54890] Re: Question about SockJS BodyHandler

 

[Paulo Lopes]

Indeed, that is a javadoc bug. It used to be like that before the sub routing got a major refactor long ago. Can you open an issue to review it.

Basically, the "before" is incorrect. Any handlers can be added before, yet only 1 routes is allowed per route.

---- On Wed, 12 Oct 2022 08:58:54 +0200 Jan Fengler <Jan.F...@adviqo.com> wrote ---

To view this discussion on the web, visit https://groups.google.com/d/msgid/vertx/AM0PR08MB35374F52959C5EE1C53313338B229%40AM0PR08MB3537.eurprd08.prod.outlook.com.

[Jan Fengler]

Hi Paulo,

I have just opened the issue "Fix Javadoc for subRouter" #4516 as proposed.
Sorry for the delay.

Thx
  Jan

---

Von: ve...@googlegroups.com <ve...@googlegroups.com> im Auftrag von Paulo Lopes <pa...@mlopes.net>
Gesendet: Mittwoch, 12. Oktober 2022 09:02
An: vertx <ve...@googlegroups.com>
Betreff: Re: AW: AW: [vertx:54898] Re: Question about SockJS BodyHandler

 

To view this discussion on the web, visit https://groups.google.com/d/msgid/vertx/183cb00edcc.dc2f34b64439982.3535831708958986724%40mlopes.net.