search

daurnimator / lua-http

HTTP Library for Lua. Supports HTTP(S) 1.0, 1.1 and 2.0; client and server.
https://daurnimator.github.io/lua-http/
MIT License
807 stars 82 forks source link

unhandled errors leave connection open #146

Closed pspacek closed 5 years ago

pspacek commented 5 years ago

Unhandled errors in HTTP server code (like #145) leave connection between client and Lua-http server open but effectively dead.

That is very annoying for clients which believe something is happening on server side and have to wait for timeout. Closing TCP connection immediately would be much nicer for clients.

daurnimator commented 5 years ago

I haven't looked at #145 hard yet; but isn't that a stream-level error, not a connection one?

Furthermore, shouldn't stream-level errors be thrown from your 'on_stream' handler? IIRC that should end up shutting the stream down correctly and informing the client?

Note: I am currently traveling and won't be able to look deep into this issue until easter

pspacek commented 5 years ago

As far as I can tell in #145 neither onstream or onerror get called because the error is thrown from inside of lua-http "guts" for HTTP/2 frame handling. This particular case seems to me like an internal error in lua-http and in that case I kind of expect it closes the connection or does something else to prevent connection stall.

daurnimator commented 5 years ago

The bug you found (#145) was tripping an internal assert that indicates a bug in the library or incorrect library usage. It should bubble up to the top level, as internal state is inconsistent and could lead to security issues.

After some deliberation, I'm going to close this as WONTFIX.

I'm curious if the way you call into lua-http may actually be ignoring errors if you continued on after the error was thrown!

pspacek commented 5 years ago

Sorry for late reply. lua-http server is used as part of Knot Resolver. At the same time Knot Resolver allows users to provide their own Lua code which which is being executed as coroutines.

This is part of code which executes coroutines: https://gitlab.labs.nic.cz/knot/knot-resolver/blob/master/daemon/lua/sandbox.lua.in#L510

As you can see errors are ignored which prevents a typo in user supplied snippet from crashing the whole DNS resolver.

Question is how to handle errors from lua-http server in a reasonable way. Blocking a whole connection or killing resolver is not an option... Do you have a recommendation?

daurnimator commented 5 years ago

Question is how to handle errors from lua-http server in a reasonable way. Blocking a whole connection or killing resolver is not an option... Do you have a recommendation?

What do you want to happen? If there is an error due to corrupted lua-http state, then what can you do aside from restart the process?