HTTP/1.1: allow duplicate Content-Length

[brian-pane]

Description:
When Envoy receives an HTTP/1.1 request or response containing two or more identical Content-Length headers, it rejects the message as invalid. We discovered this when replacing Another HTTP Proxy with Envoy in front of an old web application that sends two Content-Length: 0 in its responses.

According to RFC 7230, Section 3.3.2, implementors have the option of accepting such malformed messages:

If a message is received that has multiple Content-Length header
fields with field-values consisting of the same decimal value, or a
single Content-Length header field with a field value containing a
list of identical decimal values (e.g., "Content-Length: 42, 42"),
indicating that duplicate Content-Length header fields have been
generated or combined by an upstream message processor, then the
recipient MUST either reject the message as invalid or replace the
duplicated field-values with a single valid Content-Length field
containing that decimal value prior to determining the message body
length or forwarding the message.

Currently, Envoy implements the "reject the message" option. Based on the robustness principle, I propose that it should implement the "replace the duplicated field-values" option. (This would require a change in http-parser.)

[mattklein123]

I don't have any immediate objection to this, though I know there are various security concerns around content-length processing that I don't know all the details of. @alyssawilk thoughts here?

[alyssawilk]

No objection as long as we validate they're the same. If this requires changes to http_parser I think you should just file and fix them there, though we can leave this open for tracking if we have tests which will break. If http_parser consolidates duplicate equivalent content lengths I suspect it would be invisible to Envoy (we'd just get content-length: 42).

[stale]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

[brian-pane]

The http_parser project won't incorporate the change in the foreseeable future, because it would break binary compatibility. nodejs/http-parser#435

A while ago, I started developing an alternate parser implementation (which would also help with performance), but I only got as far as writing a Ragel grammar. It may be a long time before I get to resume work on that, due to other project obligations.

[stale]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted". Thank you for your contributions.