h2o2 icon indicating copy to clipboard operation
h2o2 copied to clipboard

DELETE with content-length 0 produces a technically incorrect payload

Open deej-split opened this issue 4 months ago • 3 comments

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:

  • We get a 0 length Buffer as the payload
  • h2o2 then concludes that since there's no transfer-encoding header in the originating request, (no encoding), there IS a payload, and this is a DELETe method that it should add a transfer-encoding: chunked header
  • Later on, the overall hapi system concludes (it seems) that since there IS a Buffer instance (even if 0) it will add a content-length header

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 avatar Feb 26 '24 22:02 deej-split

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.

deej-split avatar Feb 27 '24 18:02 deej-split

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 avatar Feb 28 '24 09:02 kanongil

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.

kanongil avatar Feb 28 '24 13:02 kanongil