HTTP server data handler and end handler not being called.

[mark....@viewds.com]

We have encountered an issue in the Vert.x HTTP server API.

To receive an HTTP request, we register a handler that is delivered an HttpServerRequest object. We then need to register handlers on this HttpServerRequest object: a data handler to receive the body of the request; and an end handler to receive notification that the entire request has been received. There is a problem if these handlers are not registered before additional asynchronous processing occurs as the underlying infrastructure will receive the request while our code is waiting for an response from this additional processing and by the time we register the data handler and end handler, the request has already been received and our handlers are never called.

This problem has been reported previously in this forum and there is a bugzilla issue that was reported about 12 months ago describing it:

https://bugs.eclipse.org/bugs/show_bug.cgi?id=433459

If we are using only the core Vert.x API, then the above problem does not tend to cause a problem as we can register the data handler and end handler before initiating any additional processing, although this could lead to some coordination issues (for example, we do not want to process the request until authentication is complete so need to coordinate between the handler for the asynchronous directory lookup confirming authentication credentials and the end handler indicating the request has been completely received).

However, we have been using the vertx-apex package to handle our HTTP server requests because it provides some useful functionality such as cookie handlers and session management. When we use this package, however, we do not have a chance to register a data handler and end handler on the HttpServerRequest object until after other processing, such as authentication, has occurred. This is resulting in our handlers never being notified because the authentication processing in the authentication handler (and the clustered session management in a clustered environment) require asynchronous requests that delay our ability to register our handlers until after it is too late to receive any notifications.

We can currently work around this problem by informing the HttpServerRequest object to suspend receiving the body of the request until after all the additional processing has been completed (using the HttpServerRequest pause() and resume() methods), however, this requires considerable care in coding to ensure that receiving is resumed in all code paths. It would be preferable if the HttpServerRequest methods for registering the data handler and end handler could be modified to check for already received data and call the handlers appropriately, as proposed in the comment https://bugs.eclipse.org/bugs/show_bug.cgi?id=433459#c1 of the above bugzilla issue. This would probably require that the data handler is registered before the end handler, but documenting and checking for that should not be difficult.

[mark....@viewds.com]

Apologies for the formatting of this post, it was wrapping in the editor.

On further consideration, the suggestion of having the data handler and/or end handler called immediately if data has been received it a problem because it requires caching of the request body. With many concurrent connections, this could require excessive memory use which is clearly not desirable.

At present, we are setting up a request handler on vertx-apex root route that calls the pause() method on the HttpServerRequest. We then need to make sure that any code path that generates a response to the request calls the resume() method. This can be a problem, particularly if we use handlers implemented by other parties (e.g. the vertx-apex BasicAuthHandler). A possible resolution to this would be to have the HttpServerResponse end() methods always resume a previously paused request to ensure that the HTTP request processing completes once a response has been provided.

[Jazz]

Hi, 

I am just curious how can you do any kind of processing without receiving the body's data first?

Do you want to process the http headers first? then based on the result, determine if you want to receive the http body?

[mark....@viewds.com]

Yes, we are using HTTP basic authentication, which is based solely on HTTP headers, and if the current session is not authenticated, the basic authentication handler would generate an HTTP unauthorised response before any body handlers are even registered (using vertx-apex, that is). Where the basic authentication headers are provided, we need to carry out directory operations to authenticate the credentials before processing the body.

- Mark.

[Tim Fox]

On 14/04/15 00:12, mark....@viewds.com wrote:

We have encountered an issue in the Vert.x HTTP server API.

To receive an HTTP request, we register a handler that is delivered an HttpServerRequest object. We then need to register handlers on this HttpServerRequest object: a data handler to receive the body of the request; and an end handler to receive notification that the entire request has been received.

You could use a simple bodyHandler to do the above.

 There is a problem if these handlers are not registered before additional asynchronous processing occurs as the underlying infrastructure will receive the request while our code is waiting for an response from this additional processing and by the time we register the data handler and end handler, the request has already been received and our handlers are never called.

If you make sure a bodyHandler is registered first, this shouldn't happen.

This problem has been reported previously in this forum and there is a bugzilla issue that was reported about 12 months ago describing it:

https://bugs.eclipse.org/bugs/show_bug.cgi?id=433459

If we are using only the core Vert.x API, then the above problem does not tend to cause a problem as we can register the data handler and end handler before initiating any additional processing, although this could lead to some coordination issues (for example, we do not want to process the request until authentication is complete so need to coordinate between the handler for the asynchronous directory lookup confirming authentication credentials and the end handler indicating the request has been completely received).

However, we have been using the vertx-apex package to handle our HTTP server requests because it provides some useful functionality such as cookie handlers and session management. When we use this package, however, we do not have a chance to register a data handler and end handler on the HttpServerRequest object until after other processing, such as authentication, has occurred.

That shouldn't be the case. If you add a body handler before the auth handler then you'll always have the body before auth occurs.

 This is resulting in our handlers never being notified because the authentication processing in the authentication handler (and the clustered session management in a clustered environment) require asynchronous requests that delay our ability to register our handlers until after it is too late to receive any notifications.

We can currently work around this problem by informing the HttpServerRequest object to suspend receiving the body of the request until after all the additional processing has been completed (using the HttpServerRequest pause() and resume() methods), however, this requires considerable care in coding to ensure that receiving is resumed in all code paths. It would be preferable if the HttpServerRequest methods for registering the data handler and end handler could be modified to check for already received data and call the handlers appropriately, as proposed in the comment https://bugs.eclipse.org/bugs/show_bug.cgi?id=433459#c1 of the above bugzilla issue. This would probably require that the data handler is registered before the end handler, but documenting and checking for that should not be difficult.

--
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.
For more options, visit https://groups.google.com/d/optout.

[Mark Ennis]

I am going to assume you missed my follow up to my original post.

If we assume a high volume HTTP server, then we do not want to cache the
body of requests while authentication is being carried out due to the
potentially high memory usage this may entail. We want to be able to
pause receiving the request body until it is determined that we require
it. However, if we pause a request immediately and then use vertx-apex
route handlers, such as the basic authentication handler class, that
generate their own response in some circumstances, we may not have an
opportunity to resume receiving the request and the service hangs. Even
if we use only our own route handlers, making sure that all possible
code paths in a complex service resume receiving the request could
quickly become onerous.

A possible solution to this might be to have the HttpServerResponse
end() methods always make sure that a connection is resumed.

It also might be worth considering having the HttpServer pause a
connection after headers have been received and resume it when an end
handler is registered or an HttpServerResponse end() method is called.

- Mark.

[Tim Fox]

I have to admit I am still not seeing the issue here.

Perhaps you could summarise in a couple of sentences?

[Mark Ennis]

Tim,

Sorry, but a couple of sentences won't cut it here. My last post was an
attempt at brevity and it clearly did not work. :)

The current Vert.x HTTP server behaviour appears to be readily
misunderstood, as evidenced by your own comments in this mailing list
indicating that the problem of people not realising they need to
register body handlers immediately for an HTTPServerRequest has been
raised in this mailing list frequently (e.g.
https://groups.google.com/d/msg/vertx/dH8QP-GQgrY/ZTJKazXSE9MJ).

This misunderstanding appears to be encouraged by the way the vertx-apex
router classes encourage people to use the Vert.x HTTP server interface
because it gives the impression that you can set up handlers for
different routes without having to initially prepare global body
handlers. While these impressions are wrong, it appears to be a common
failing that it not remedied by Vert.x documentation or the behaviour of

the Vert.x HTTP server API.

I would like the Vert.x team to consider a possible solution to this
problem of ready misunderstanding.

Bruno Salmon, who submitted the bugzilla issue
https://bugs.eclipse.org/bugs/show_bug.cgi?id=433459, has proposed a
change to the HTTP server API that would help alert people that they are
using the interface incorrectly by throwing an exception if a body
handler is registered too late. However, this has not been implemented.

Tamas Foldesi, who commented on the same bugzilla issue, also proposed a
change to the API to have it cache the body of the request and provide
it when a body handler is registered. I expect this is not practical
because for a server in a high traffic environment, the amount of
information that could end up being cached could impose excessive memory
requirements on the application.

So I am proposing another alternative solution that, as far as I can
determine, would allow the HTTP server API to behave the way people
(currently mistakenly) expect it to and without breaking any current use
of that API or introducing significant performance cost.

My proposal is to have the Vert.x HTTP server pause receiving a request
after headers have been received and automatically resume it when an end

handler is registered or an HttpServerResponse end() method is called.

Existing applications that currently register body handlers immediately
will effectively never have their connection paused. New applications
that carry out header processing, for example for authentication, before
registering body handlers would find their code would work instead of
failing without any indication of cause. People using vertx-apex could
have route handlers that register different body handlers for different
routes after common authentication processing without this failing
without any indication of cause.

- Mark.

P.S. To get behaviour in the current Vert.x 3 implementation that
approaches what my proposal would hopefully provide, we are doing the
following (using vertx-apex):

router = Router.router(vertx);
/*
* This must be the first route registered.
*/
router.route().handler((RoutingContext ctx) -> {
ctx.addBodyEndHandler((Void v) -> {
ctx.request().resume();
});
ctx.request().pause();
ctx.next();
});

Then we need to make sure to resume receiving the request whenever a
body or end handler is registered, e.g.:

ctx.request().resume().endHandler(...);

As long as we write all the code that registers body or end handlers,
this pattern is not overly difficult to maintain, however, if the
broader community start providing "re-usable" route handlers that we
might like to be able to take advantage of, expecting this pattern to be
followed universally would not be reasonable. So removing the
requirement for this pattern by having the Vert.x HTTP server API behave
in such a way that it is not necessary would be preferable.

[Tim Fox]

Mark,

I am still missing something here.

Instead of focussing in on a solution (i.e. setting body handlers,
pausing the request, or whatever), I first would like to understand the
problem and a concrete use case where the issue manifests. Probably the
best way to do this would be to provide some code.

Could you provide a simple Apex example which demonstrates the issue?

[Tim Fox]

I've added some checks so Vert.x will throw exceptions if end/body etc
handlers are set after the request has been read.. not sure if this
really addresses your issue.

If there is a remaining issue I think the best way to communicate would
be to create a reproducer with some notes saying "Hey! I expect this to
actually do XXX but as you can see it does YYY" :)

[Mark Ennis]

Tim,

I have put some example code in
https://github.com/mark-ennis/vertx-bodyhandler to demonstrate my
concern. This code will display a page at
http://localhost:8080/restricted but if you submit this page the browser
will not receive a response.

This code was written following the pattern for adding an authentication
handler in the vert-apex documentation. This documentation tends to
overlook the requirement that body handlers need to be registered before
other things like session handlers and authentication handlers. While it
is easy enough to correct once you understand the issue, it is an easy
trap to fall into and can take a fair bit of work to figure out what is
wrong.

On 17/04/2015 20:15, Tim Fox wrote:
> I've added some checks so Vert.x will throw exceptions if end/body etc
> handlers are set after the request has been read.. not sure if this
> really addresses your issue.

This change is an improvement in helping diagnose why the HTTP server
fails to respond in some situations, but it is not what I have been
suggesting.

In my opinion, there is possibly a better solution that would allow my
example code to work as written without introducing other problems. It
also would handle the situation where an application does not want to
receive the body of a message until after other checks, such as
authentication, have been completed. This could be quite important in a
high traffic environment.

My proposal is to have the vertx-core HTTP server pause receiving a
request after headers have been received (e.g. just before calling the
request handler) and automatically resume it when a body/end handler is
registered or an HttpServerResponse.end() method is called.

Existing applications that currently register body handlers immediately
will effectively never have their connection paused. New applications
that carry out header processing, for example for authentication, before
registering body handlers would find their code would work instead of

failing without any indication of cause. Code for vert-apex that
followed patterns in the documentation, like I did in my example code,
should find it works instead of failing unexpectedly.

- Mark.

[Tim Fox]

On 19/04/15 23:01, Mark Ennis wrote:
> Tim,
>
> I have put some example code in
> https://github.com/mark-ennis/vertx-bodyhandler to demonstrate my
> concern. This code will display a page at
> http://localhost:8080/restricted but if you submit this page the
> browser will not receive a response.
>
> This code was written following the pattern for adding an
> authentication handler in the vert-apex documentation. This
> documentation tends to overlook the requirement that body handlers
> need to be registered before other things like session handlers and
> authentication handlers. While it is easy enough to correct once you
> understand the issue, it is an easy trap to fall into and can take a
> fair bit of work to figure out what is wrong.

With the latest sanity checks I added, the user should find out
immediately they are setting a handler after the event has already
occurred, so the above shouldn't be an issue any more.

[Mark Ennis]

On 20/04/2015 19:17, Tim Fox wrote:
> On 19/04/15 23:01, Mark Ennis wrote:
>> Tim,
>>
>> I have put some example code in
>> https://github.com/mark-ennis/vertx-bodyhandler to demonstrate my
>> concern. This code will display a page at
>> http://localhost:8080/restricted but if you submit this page the
>> browser will not receive a response.
>>
>> This code was written following the pattern for adding an
>> authentication handler in the vert-apex documentation. This
>> documentation tends to overlook the requirement that body handlers
>> need to be registered before other things like session handlers and
>> authentication handlers. While it is easy enough to correct once you
>> understand the issue, it is an easy trap to fall into and can take a
>> fair bit of work to figure out what is wrong.
>
> With the latest sanity checks I added, the user should find out
> immediately they are setting a handler after the event has already
> occurred, so the above shouldn't be an issue any more.

I am aware of that Tim. Did you actually read the rest of my message?
You asked me to provide code to demonstrate the problem I am trying to
suggest a solution for. I did that and yet you are still ignoring the
solution I am suggesting.

- Mark.

[Tim Fox]

On 20/04/15 22:20, Mark Ennis wrote:
> On 20/04/2015 19:17, Tim Fox wrote:
>> On 19/04/15 23:01, Mark Ennis wrote:
>>> Tim,
>>>
>>> I have put some example code in
>>> https://github.com/mark-ennis/vertx-bodyhandler to demonstrate my
>>> concern. This code will display a page at
>>> http://localhost:8080/restricted but if you submit this page the
>>> browser will not receive a response.
>>>
>>> This code was written following the pattern for adding an
>>> authentication handler in the vert-apex documentation. This
>>> documentation tends to overlook the requirement that body handlers
>>> need to be registered before other things like session handlers and
>>> authentication handlers. While it is easy enough to correct once you
>>> understand the issue, it is an easy trap to fall into and can take a
>>> fair bit of work to figure out what is wrong.
>>
>> With the latest sanity checks I added, the user should find out
>> immediately they are setting a handler after the event has already
>> occurred, so the above shouldn't be an issue any more.
>
> I am aware of that Tim. Did you actually read the rest of my message?
> You asked me to provide code to demonstrate the problem I am trying to
> suggest a solution for. I did that and yet you are still ignoring the
> solution I am suggesting.

Hi Mark,

I haven't ignored your solution, and yes, I did read it :)

Perhaps I wasn't clear in my previous comment - Given that there are now
sanity checks that would prevent people being caught out by the this, I
don't think any solution is necessary.

It's a general principle in Vert.x (or any event driven system, btw
Node.js is just the same) that if you don't have a handler listening
when an event arrives you won't get the event. It's natural that data
for an HTTP request starts to be read right after the headers, so it
seems natural to me that if a data handler isn't set when it is read...
the data won't be received. When the data has been read the end event
for the request is fired - at this point if there is no end handler,
again it seems natural that the event won't be received.

Automatically pausing and resuming requests will give behaviour that's
not consistent with how other events are handled in Vert.x, and will
appear strange to many users (especially those with previous experience
in Vert.x, Node.js or other similar systems).

It may also reduce performance, as pausing/resuming can cause I/O and
can cause backpressure on the connection. Given the above, and the fact
that the sanity checks mean that a user who is not used to the ordering
of events won't now be stuck for a long time wondering what is happened
as they will immediately receive an exception if they set a
data/body/end handler too late, it's my opinion that there is nothing
more to do here.

[Mark Ennis]

Tim,

Thank you for the explanation of why you don't consider my solution
applicable and thanks for implementing the sanity checks. While the
sanity checks are not my preferred solution, it is much better than no
indication of a problem.

- Mark.

[Lais Andrade]

Hi Tim,

Still about the auth and body handlers, what can we do if we don't want the server to read the body until the request is authenticated?

The body handler processes the request data and allocates it inside buffers, and event though it will be ignored once the request fails, 

the user would be able to inject data inside our server with a unauthorized request.