Skip to content
This repository has been archived by the owner on Nov 6, 2022. It is now read-only.

Allow duplicated Content-Length with the same value #460

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

obatysh
Copy link

@obatysh obatysh commented Dec 20, 2018

No description provided.

@indutny
Copy link
Member

indutny commented Dec 21, 2018

Hello!

Thank you for submitting this patch. Wouldn't allowing duplicate content-length header be a violation of protocol specification?

@obatysh
Copy link
Author

obatysh commented Dec 22, 2018

RFC https://tools.ietf.org/html/rfc7230#page-31 says that duplicate Content-Length fields with the same decimal value can be either rejected as invalid or accepted by replacing duplicate with a single valid Content-Length field. So, it is not a violation, it is one of the possible options.
Not sure about "replacement" implementation, maybe this pull-request should be improved not to report the Content-Length field second time.

@indutny
Copy link
Member

indutny commented Dec 22, 2018

Good point!

I'd love to hear opinion of @nodejs/http on that? Would we prefer to reject it or parse it?

@ploxiln
Copy link
Contributor

ploxiln commented Dec 23, 2018

it's worth noting that this costs an additional 8 bytes per struct http_parser

@bnoordhuis
Copy link
Member

Yes. That means it breaks ABI and can't land in a v2.x release. See discussion in #435.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants