Skip to content
This repository was 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
obatysh wants to merge 2 commits into
nodejs:masterfrom
obatysh:content-length-folding
Closed

obatysh wants to merge 2 commits into
nodejs:masterfrom
obatysh:content-length-folding

Conversation

@obatysh

@obatysh obatysh commented Dec 19, 2018

Copy link
Copy Markdown

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
Comment thread http_parser.c Outdated
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
Copy Markdown
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
Copy Markdown
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 <info@bnoordhuis.nl>
@bnoordhuis

Copy link
Copy Markdown
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 <info@bnoordhuis.nl>
@sam-github

Copy link
Copy Markdown
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