hapijs / h2o2

Proxy handler for hapi.js
Other
165 stars 66 forks source link

DELETE with content-length 0 produces a technically incorrect payload #140

Open deej-split opened 7 months ago

deej-split commented 7 months ago

Runtime

node.js

Runtime version

v20.11.0

Module version

10.0.4

Last module version without issue

No response

Used with

hapi

Any other relevant information

No response

What are you trying to achieve or the steps to reproduce?

We are using h2o2 to proxy requests. We have found that sending a DELETE request with a content-length of 0 has an unexpected result:

What was the result you got?

The result is that the request that is sent on has both a transfer-encoding and a content-length header. Evidently, this is technically incorrect. We have two other servers in our systems which do not like this header combination and reject the request. (other servers don't seem to care)

What result did you expect?

Probably that if there is no content, that we do not set a transfer-encoding header?

deej-split commented 7 months ago

fwiw, we worked around this by defining a custom httpClient that checks if the payload is 0 length, and if so passes a payload of undefined. I'm not sure it is 100% correct, but it works for our need.

kanongil commented 7 months ago

This issue seems to be a fallout of a decision to strip content-length headers in passthrough mode, which broke DELETE proxying. A fix was applied in https://github.com/hapijs/h2o2/commit/3e695df1904c15f0521775a6150a1d832da654e0, but this has issues due to Wreck incorrectly adding an extra content-length header.

As far as I can tell, this issue applies to all DELETE proxying with content-length, not only when it is 0.

As I see this, there are two bugs involved:

  1. H2o2 removes the content-length header. This removal does not really make sense, and seems to be an incorrect fix to an issue in another module.
  2. Wreck is too liberal in adding a content-length header. It should probably not add it when the headers include a transfer-encoding.

FYI, only one of these needs to be fixed, to resolve this specific issue.

kanongil commented 7 months ago

Another related h2o2 issue, seems to be that it does not strip hop-by-hop headers (including the already know ones), which can mess up the connection to the proxied server.

Again content-length is not a hop-by-hop header, and it makes no sense that h2o2 strips this when it doesn't do anything to the payload.

FYI, hapi already strips them from stream responses.