nodejs / http-parser

http request/response parser for c
MIT License
6.33k stars 1.53k forks source link

Consolidate duplicate content-length response headers #435

Open derekargueta opened 6 years ago

derekargueta commented 6 years ago

While deploying Envoy, which uses this project for parsing HTTP/1.1 messages, we've seen an occasional bug in a web service where the health-check response contains duplicate content-length headers, looking like:

< HTTP/1.1 200 OK
--
< Content-Length: 0
< Content-Length: 0
<

This library considers the duplicate content-length headers erroneous (code), and as a result Envoy fails the health-check and won't route traffic to that host. Per RFC 7230, Section 3.3.2, if the values of duplicate content-length headers are equivalent, it is acceptable to either reject the message or consolidate the values into a single content-length header.

This issue is a proposal to implement the latter - if the duplicate content-length headers are found to contain the same value, consolidate them into a single content-length header to improve robustness.

Envoy issue

bnoordhuis commented 6 years ago

For background: e2e467b91262246b339fb3d80c8408d498b4a43b - rejecting duplicate Content-Length headers was added as a security measure.

There is no room in struct http_parser to store the previous Content-Length value without breaking ABI (relevant when the second header is spread across chunks) so I don't think we could accept identical duplicates even if we wanted to, at least not in v2.x. And there are no plans for a v3.0 release.