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

allow content-length and transfer-encoding: c…#518

Closed
veshij wants to merge 8 commits into
nodejs:masterfrom
veshij:master
Closed

allow content-length and transfer-encoding: c…#518
veshij wants to merge 8 commits into
nodejs:masterfrom
veshij:master

Conversation

@veshij

@veshij veshij commented Jun 3, 2020

Copy link
Copy Markdown
Contributor

fixes #517

@veshij

veshij commented Jun 3, 2020

Copy link
Copy Markdown
Contributor Author

@indutny what would be the best way to pass new option to the parser? Use settings?

@veshij veshij changed the title WIP: allow content-length and transfer-encoding: c… allow content-length and transfer-encoding: c… Jun 5, 2020
@veshij

veshij commented Jun 5, 2020

Copy link
Copy Markdown
Contributor Author

Could anyone take a look at the change, please?

Comment thread http_parser.h Outdated

@bnoordhuis bnoordhuis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM, thanks for doing this.

I just opened #519 to add more Content-Length testing because that was rather lacking from the test suite... It would be good to land that one first and rebase this PR on top of it.

(You can rebase it now, of course, no need to wait for the actual merge.) #519 has been merged.

Comment thread http_parser.h Outdated
Comment thread test.c Outdated
Comment thread http_parser.h Outdated
@bnoordhuis

bnoordhuis commented Jul 10, 2020

Copy link
Copy Markdown
Member

Thanks Oleg, landed with minor indent/line length touch-ups in d0f3b05 e13b274 (now with a PR-URL tag.)

@bnoordhuis bnoordhuis closed this Jul 10, 2020
bnoordhuis pushed a commit to bnoordhuis/http-parser that referenced this pull request Jul 10, 2020
Fixes: nodejs#517
PR-URL: nodejs#518
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Pierce Lopez <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Request with Transfer-Encoding: chunked and Content-Length is valid per RFC, but rejected with HPE_UNEXPECTED_CONTENT_LENGTH

3 participants