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

Fix: Content-Length with obsolete line folding and invalid input #458

Closed
wants to merge 2 commits into from

Conversation

obatysh
Copy link

@obatysh obatysh commented Dec 19, 2018

Fix for the issue: #456

  • treat obsolete line folding as space and continue parsing
  • update tests

…id input

- treat obsolete line folding as space and continue parsing
- update tests
http_parser.c Outdated
@@ -1434,6 +1434,10 @@ size_t http_parser_execute (http_parser *parser,
parser->header_state = h_content_length_num;
break;

// when obsolete line folding is encountered for content length continue to the s_header_value state
Copy link
Member

Choose a reason for hiding this comment

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

Can you use C89-style comments and wrap at 80 columns? Thanks.

Copy link
Author

Choose a reason for hiding this comment

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

Done

bnoordhuis pushed a commit that referenced this pull request Dec 24, 2018
Content-Length with line folding was accepted with invalid input. Treat
obsolete line folding as space and continue parsing

Fixes: #456
PR-URL: #458
Reviewed-By: Ben Noordhuis <[email protected]>
@bnoordhuis
Copy link
Member

Thanks for your contribution, Olga. Merged in cd88eef.

@bnoordhuis bnoordhuis closed this Dec 24, 2018
@obatysh obatysh deleted the content-length-folding branch December 27, 2018 10:27
ploxiln pushed a commit to ploxiln/http-parser that referenced this pull request Jan 4, 2019
Content-Length with line folding was accepted with invalid input. Treat
obsolete line folding as space and continue parsing

Fixes: nodejs#456
PR-URL: nodejs#458
Reviewed-By: Ben Noordhuis <[email protected]>
@sam-github
Copy link
Contributor

Is this a security issue? Does it need a CVE? Does it need releasing onto Node.js LTS branches?

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.

3 participants