nicolasff / webdis

A Redis HTTP interface with JSON output
https://webd.is
BSD 2-Clause "Simplified" License
2.82k stars 307 forks source link

Connection: close triggers a freeze for payloads bigger than certain limit #194

Closed przemek-pokrywka closed 3 years ago

przemek-pokrywka commented 3 years ago

Thanks for webdis, I like it a lot. I spotted the following issue when I put webdis behind a nginx, which I use for reverse-proxying:

If I send a PUT with some small payload, then webdis is fine, even when the Connection: close header is present:

(echo "[1"; yes ",1" | head -n 1957; echo "]" )  | curl -XPUT http://172.17.0.2:7379/SET/toobig -d @- -H "Connection: close"

but if the payload is bigger, then the connection hangs and eventually times out:

(echo "[1"; yes ",1" | head -n 1958; echo "]" )  | curl -XPUT http://172.17.0.2:7379/SET/toobig -d @- -H "Connection: close"

finally the problem goes away if I remove the Connection: close header.

(echo "[1"; yes ",1" | head -n 1958; echo "]" )  | curl -XPUT http://172.17.0.2:7379/SET/toobig -d @-

This is a problem, because the default configuration of nginx does append the Connection: close. I worked that around by instructing nginx to send Connection: keep-alive instead:

proxy_set_header Connection keep-alive;

but I think that this is better addressed at webdis directly. For start it might be good to add that to webdis known issues in the docs.

nicolasff commented 3 years ago

Hello Przemek,

Thanks a lot for the detailed bug report! I agree that it should be addressed by webdis. I can easily reproduce the issue on macOS, although I would expect it to be reproducible on Linux too.

The repro seems pretty straightforward; I changed it a little bit to have more control over the exact length but it does something similar.

On macOS 11.0, with length 1,000:

$ printf 'A%.0s' $(seq 1 1000) | curl -s -XPUT http://127.0.0.1:7379/SET/toobig -d @- -H "Connection: close" ; echo ; curl -s 'http://127.0.0.1:7379/STRLEN/toobig'
{"SET":[true,"OK"]}
{"STRLEN":1000}%

With 10,000: the PUT request hangs. The server does send a 100 Continue and the client reports having uploaded the full payload, but webdis gets stuck:

$ printf 'A%.0s' $(seq 1 10000) | curl -v -XPUT http://127.0.0.1:7379/SET/toobig -d @- -H "Connection: close" ; echo ; curl -s 'http://127.0.0.1:7379/STRLEN/toobig'
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 7379 (#0)
> PUT /SET/toobig HTTP/1.1
> Host: 127.0.0.1:7379
> User-Agent: curl/7.64.1
> Accept: */*
> Connection: close
> Content-Length: 10000
> Content-Type: application/x-www-form-urlencoded
> Expect: 100-continue
>
< HTTP/1.1 100 Continue
* We are completely uploaded and fine

It's not clear to me what is blocking, but it shouldn't be too hard to figure out. @jessie-murray is this something you could look into? If so, let me know if you need some help going through the code.

jessie-murray commented 3 years ago

@nicolasff sure, I can try to find it. I wonder if the additional step with the 100 Continue might have something to do with the issue. At least it will be easy to add a new test case for it once we have a patch to test :)