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.

4 participants