nicolasff / webdis

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

OPTIONS response has invalid blank header with no value #217

Open ewired opened 2 years ago

ewired commented 2 years ago

I discussed the issue on the Fly help forum regarding the OPTIONS request responding with no CORS headers in #216. Fly's HTTP proxy is returning 502 because the OPTIONS response from Webdis has a blank : header with no name or value.

See two lines above last line:

< HTTP/1.1 200 OK
< Server: Webdis
< Allow: GET,POST,PUT,OPTIONS
< Access-Control-Allow-Methods: GET,POST,PUT,OPTIONS
< Access-Control-Allow-Origin: *
< Access-Control-Allow-Headers: X-Requested-With, Content-Type, Authorization
< Connection: Keep-Alive
< Content-Type: text/html
< Content-Length: 0
< :
<
* Connection #0 to host XYZZY.fly.dev left intact

https://community.fly.io/t/http-handler-502-errors-on-options-request/4160/5?u=ewired

nicolasff commented 2 years ago

Thanks for reporting this, I didn't see it yesterday when I ran it with an older version of Webdis but I can reproduce it on the latest. This is likely a regression from #205. /cc @jessie-murray

nicolasff commented 2 years ago

@jessie-murray I've tracked it down with git-bisect to your change in dc9d1b646e2d6d9daa295e23d54b3994f422bf02.

This is due to the header replacement code in http_response_set_header, which overwrites the value here:

https://github.com/nicolasff/webdis/blob/dc9d1b646e2d6d9daa295e23d54b3994f422bf02/src/http.c#L100-L108

but later still increments the header count:

https://github.com/nicolasff/webdis/blob/dc9d1b646e2d6d9daa295e23d54b3994f422bf02/src/http.c#L117-L117

It needs to avoid this increment if no new value was added. For the case of OPTIONS, somehow the Content-Length header is set twice, both times to 0. The second one causes this increment for which there's no key-value pair to write out.