ACINQ / phoenix

Phoenix is a self-custodial Bitcoin wallet using Lightning to send/receive payments.
https://phoenix.acinq.co
Apache License 2.0
656 stars 97 forks source link

Content-Length header set to 0 in GET requests for LNURL-auth #393

Closed ekzyis closed 10 months ago

ekzyis commented 1 year ago

Hi, we've noticed that Phoenix sets the Content-Length header to 0 for the GET request to the LNURL-auth callback.

According to the HTTP spec, GET requests from user agents SHOULD NOT include the Content-Length header:

A user agent SHOULD NOT send a Content-Length header field when the request message does not contain content and the method semantics do not anticipate such data.

-- https://www.rfc-editor.org/rfc/rfc9110#field.content-length

See https://github.com/stackernews/stacker.news/issues/407 for more details.

robbiehanson commented 10 months ago

I investigated this, and confirmed that the Ktor library does indeed send "Content-Length: 0" for GET requests.

I also figured out how to remove it. But removing that header wasn't easy or straight-forward. One has to use a custom plugin, and understand the internals of Ktor. In other words, it's basically undocumented.

A user agent SHOULD NOT send a Content-Length header field when the request message does not contain content and the method semantics do not anticipate such data.

I think this confirms that Ktor shouldn't be sending that header (at least not when method == GET && body.length == 0).

However "SHOULD NOT" is not the same as "MUST NOT". So if we follow the robustness principle:

be conservative in what you send, be liberal in what you accept

then we'd probably conclude that:

I think you already submitted an issue to NextJS ?

And I submitted an issue to Ktor: KTOR-6508

ekzyis commented 10 months ago

However "SHOULD NOT" is not the same as "MUST NOT". So if we follow the robustness principle:

be conservative in what you send, be liberal in what you accept then we'd probably conclude that:

  • the default server setup you're using is overly strict
  • Ktor should not be sending that header value by default

yes, that makes sense, we fixed it on our side by removing the Content-length header on the login route in nginx :)

I think you already submitted an issue to NextJS ?

Looks like we did not! That was an oversight on our part. Will create one now

Update: Other people already reported similar problems and in a comment, someone mentioned this Content-length: 0 problem. So I think they are already aware.