mtrudel/bandit

Reproducible: Server hangs on every other request

m0rt3nlund opened this issue · 6 comments

Hi!

I have a problem where the webserver hangs for every other request when using Plug.Conn.read_body()

First I thought it was something in my project so I created a reproducible scenario where this happens (https://github.com/m0rt3nlund/bandit_test)
It does not apply to Plug.Cowboy

To test I use Postman and a POST request (http://127.0.0.1:4001/test) with the following raw body (Content type text):
https://github.com/m0rt3nlund/bandit_test/blob/main/postman_body.text

Environment and deps
Erlang: OTP26
Elixir: 1.16

image

I have tried to investigate this myself but I cannot seem to understand why this happens.
Does not look wrong when I inspect it with Wireshark.

After some timeout this is printed:

[error] ** (Bandit.HTTPError) Header read timeout
    (bandit 1.5.2) lib/bandit/http1/socket.ex:398: Bandit.HTTPTransport.Bandit.HTTP1.Socket.request_error!/2
    (bandit 1.5.2) lib/bandit/http1/socket.ex:50: Bandit.HTTPTransport.Bandit.HTTP1.Socket.read_headers/1
    (bandit 1.5.2) lib/bandit/pipeline.ex:29: Bandit.Pipeline.run/4
    (bandit 1.5.2) lib/bandit/http1/handler.ex:12: Bandit.HTTP1.Handler.handle_data/3
    (bandit 1.5.2) lib/bandit/delegating_handler.ex:18: Bandit.DelegatingHandler.handle_data/3
    (bandit 1.5.2) c:/Development/Source/Maskon/Elixir/teste/deps/thousand_island/lib/thousand_island/handler.ex:379: Bandit.DelegatingHandler.handle_info/2
    (stdlib 5.2) gen_server.erl:1095: :gen_server.try_handle_info/3
    (stdlib 5.2) gen_server.erl:1183: :gen_server.handle_msg/6
    (stdlib 5.2) proc_lib.erl:241: :proc_lib.init_p_do_apply/3

Are you 100% certain you're not rebinding conn on every call into Plug.Conn functions? This looks exactly like that's the case

Here's your problem:

https://github.com/m0rt3nlund/bandit_test/blob/main/lib/teste_web/controllers/page_controller.ex#L38

You need to rebind conn on every call - Bandit keeps important things in there

Cowboy gets away with this in some cases because they smear connection state across a few processes so you won't always see this (but you will sometimes; it depends on where you're not rebinding). Bandit is very much intentionally a single-process-per-connection design, so we're more sensitive to rebinding.

Regardless, the Plug contract is clear on this; you need to track and rebind returned conn values for correctness.

Closing for hygiene. Thanks for the report!

Thank you very much for a prompt and good answer! This project is great!

I of course should have returned the new conn, now everything works as expected!

My thought was that since the docs says that the body is read out of the socket I did not think there was anything in the conn that was updated and then did not even think that the new conn was needed.. Silly me..

But then I know a bunch more about the inner workings of both Plug and Bandit

Thanks!