FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Update http parser 2.9.1 v12.x by sam-github · Pull Request #30473 · nodejs/node · GitHub

/ node Public

Update http parser 2.9.1 v12.x - #30473

Closed
sam-github wants to merge 4 commits into
nodejs:v12.x-stagingfrom
sam-github:update-http_parser-2.9.1-v12.x
ConversationCommits (4)ChecksFiles changed
Closed

Update http parser 2.9.1 v12.x#30473
sam-github wants to merge 4 commits into
nodejs:v12.x-stagingfrom
sam-github:update-http_parser-2.9.1-v12.x

Conversation

sam-github commented Nov 13, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

We don't use nodejs/http-parser in 13.x and master, but it exists on 8, 10, and 12, and has security fixes.

I suggest we update it, I PRed all three branches:

I'm not sure if this is right way, maybe I should have just PRed 12.x, and the backports would flow down? Except 8.x doesn't get a lot of updates, its likely worth getting these known sec fixes in before it's EOL.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

nodejs-github-bot added http_parser Issues and PRs related to the HTTP Parser dependency or the http_parser binding. v12.x labels Nov 13, 2019

sam-github commented Jan 7, 2020

Copy link
Copy Markdown
Contributor Author

Should not be included until it can be released with a backport of #30567

sam-github force-pushed the update-http_parser-2.9.1-v12.x branch from 2fec4f5 to 9ac0a2c Compare January 7, 2020 20:37
BridgeAR added the blocked PRs that are blocked by other issues or PRs. label Jan 7, 2020

sam-github commented Jan 7, 2020

Copy link
Copy Markdown
Contributor Author

Since this introduces a breaking change in HTTP parsing I backported 02a0c74 on top of it.

See:

Original commit message below, since it now applies to both parsers (lhttp-parser aka "legacy" and llhttp), I changed the description to:

http: opt-in insecure HTTP header parsing

    http: llhttp opt-in insecure HTTP header parsing
    
    Allow insecure HTTP header parsing. Make clear it is insecure.
    
    See:
    - https://github.com/nodejs/node/pull/30553
    - https://github.com/nodejs/node/issues/27711#issuecomment-556265881
    - https://github.com/nodejs/node/issues/30515

sam-github force-pushed the update-http_parser-2.9.1-v12.x branch from ffc453a to 945711e Compare January 7, 2020 21:20

nodejs-github-bot commented Jan 7, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/28271/

sam-github force-pushed the update-http_parser-2.9.1-v12.x branch from 945711e to f094fab Compare January 7, 2020 21:47

sam-github commented Jan 7, 2020

Copy link
Copy Markdown
Contributor Author

#31253 should also be backported onto this, I'll do it once it has been approved.

MylesBorins commented Jan 8, 2020

Copy link
Copy Markdown
Contributor

@sam-github is this ready to land now?

sam-github commented Jan 8, 2020

Copy link
Copy Markdown
Contributor Author

It lacks code review, and also see #30473 (comment), it lacks a unit test (as does master).

12.x just went out, I think this can wait a couple days to get the above in order, it'll land in time for the next 12.x release.

targos force-pushed the v12.x-staging branch 2 times, most recently from 3900f4a to 32e5c39 Compare January 8, 2020 16:34
sam-github force-pushed the update-http_parser-2.9.1-v12.x branch 2 times, most recently from 11c2aac to d694bdb Compare January 9, 2020 23:58
sam-github and others added 4 commits January 9, 2020 16:08
deps: upgrade http-parser to v2.9.1
d48a1b5
deps,http: http_parser set max header size to 8KB
fbe3c63 
Reapplying HTTP_MAX_HEADER_SIZE=8192 to http_parser.gyp.

CVE-2018-12121

PR-URL: https://github.com/nodejs-private/node-private/pull/143
Ref: https://github.com/nodejs-private/security/issues/139
Ref: https://github.com/nodejs-private/http-parser-private/pull/2
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
http: opt-in insecure HTTP header parsing
d4bcc5f 
Allow insecure HTTP header parsing. Make clear it is insecure.

See:
- nodejs#30553
- nodejs#27711 (comment)
- nodejs#30515

PR-URL: nodejs#30567
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
test: check that --insecure-http-parser works
2e5037b 
Test that using --insecure-http-parser will disable validation of
invalid characters in HTTP headers.

See:
- nodejs#30567

PR-URL: nodejs#31253
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
sam-github force-pushed the update-http_parser-2.9.1-v12.x branch from d694bdb to 2e5037b Compare January 10, 2020 00:09

nodejs-github-bot commented Jan 10, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/28337/

sam-github removed the blocked PRs that are blocked by other issues or PRs. label Jan 10, 2020

sam-github commented Jan 10, 2020

Copy link
Copy Markdown
Contributor Author

@nodejs/lts PTAL. Note that in the last commit, the backport of #31253 , I duplicated the test so that it runs with the legacy as well as the llhttp parser.

sam-github mentioned this pull request Jan 10, 2020
Closed
4 tasks

nodejs-github-bot commented Jan 10, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/28342/

nodejs-github-bot commented Jan 10, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/28348/

sam-github commented Jan 10, 2020

Copy link
Copy Markdown
Contributor Author

This is ready to land on v12.x-staging @nodejs/lts

sam-github added the semver-minor PRs that contain new features and should be released in the next minor version. label Jan 13, 2020

sam-github commented Jan 13, 2020

Copy link
Copy Markdown
Contributor Author

Marked semver-minor, see discussion leading to #30471 (comment)

targos commented Jan 14, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Landed in c6600d1, 25f2df4, 3a7a3be, ccf9038. Thank you!

targos closed this Jan 14, 2020
targos pushed a commit that referenced this pull request Jan 14, 2020
deps: upgrade http-parser to v2.9.1
c6600d1 
PR-URL: #30473
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
targos pushed a commit that referenced this pull request Jan 14, 2020
deps,http: http_parser set max header size to 8KB
25f2df4 
Reapplying HTTP_MAX_HEADER_SIZE=8192 to http_parser.gyp.

CVE-2018-12121

PR-URL: https://github.com/nodejs-private/node-private/pull/143
Backport-PR-URL: #30473
Ref: https://github.com/nodejs-private/security/issues/139
Ref: https://github.com/nodejs-private/http-parser-private/pull/2
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jan 14, 2020
http: opt-in insecure HTTP header parsing
3a7a3be 
Allow insecure HTTP header parsing. Make clear it is insecure.

See:
- #30553
- #27711 (comment)
- #30515

PR-URL: #30567
Backport-PR-URL: #30473
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jan 14, 2020
test: check that --insecure-http-parser works
ccf9038 
Test that using --insecure-http-parser will disable validation of
invalid characters in HTTP headers.

See:
- #30567

PR-URL: #31253
Backport-PR-URL: #30473
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
sam-github deleted the update-http_parser-2.9.1-v12.x branch January 15, 2020 00:13
targos mentioned this pull request Jan 15, 2020
Closed
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
deps,http: http_parser set max header size to 8KB
614ce0c 
Reapplying HTTP_MAX_HEADER_SIZE=8192 to http_parser.gyp.

CVE-2018-12121

PR-URL: nodejs-private/node-private#143
Backport-PR-URL: #30473
Ref: nodejs-private/security#139
Ref: nodejs-private/http-parser-private#2
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
zsw007 added a commit to ibmruntimes/node that referenced this pull request Feb 11, 2020
deps: upgrade http-parser to v2.9.1
52c955e 
Backport d41314e

Original commit message:

    PR-URL: nodejs/node#30473
    Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
    Reviewed-By: James M Snell <jasnell@gmail.com>
    Reviewed-By: Richard Lau <riclau@uk.ibm.com>
zsw007 added a commit to ibmruntimes/node that referenced this pull request Feb 11, 2020
http: opt-in insecure HTTP header parsing
11fdae4 
Backport 496736f

Original commit message:

    Allow insecure HTTP header parsing. Make clear it is insecure.

    See:
    - nodejs/node#30553
    - nodejs/node#27711 (comment)
    - nodejs/node#30515

    PR-URL: nodejs/node#30567
    Backport-PR-URL: nodejs/node#30473
    Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
    Reviewed-By: Anna Henningsen <anna@addaleax.net>
    Reviewed-By: Denys Otrishko <shishugi@gmail.com>
    Reviewed-By: James M Snell <jasnell@gmail.com>
zsw007 added a commit to ibmruntimes/node that referenced this pull request Feb 11, 2020
test: check that --insecure-http-parser works
b696d8d 
Backport ab1fcb8

Original commit message:

    Test that using --insecure-http-parser will disable validation of
    invalid characters in HTTP headers.

    See:
    - nodejs/node#30567

    PR-URL: nodejs/node#31253
    Backport-PR-URL: nodejs/node#30473
    Reviewed-By: Richard Lau <riclau@uk.ibm.com>
    Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
zsw007 added a commit to ibmruntimes/node that referenced this pull request Feb 12, 2020
http: opt-in insecure HTTP header parsing
f0049c4 
Backport 496736f

Original commit message:

    Allow insecure HTTP header parsing. Make clear it is insecure.

    See:
    - nodejs/node#30553
    - nodejs/node#27711 (comment)
    - nodejs/node#30515

    PR-URL: nodejs/node#30567
    Backport-PR-URL: nodejs/node#30473
    Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
    Reviewed-By: Anna Henningsen <anna@addaleax.net>
    Reviewed-By: Denys Otrishko <shishugi@gmail.com>
    Reviewed-By: James M Snell <jasnell@gmail.com>
zsw007 added a commit to ibmruntimes/node that referenced this pull request Feb 12, 2020
test: check that --insecure-http-parser works
b9eeafe 
Backport ab1fcb8

Original commit message:

    Test that using --insecure-http-parser will disable validation of
    invalid characters in HTTP headers.

    See:
    - nodejs/node#30567

    PR-URL: nodejs/node#31253
    Backport-PR-URL: nodejs/node#30473
    Reviewed-By: Richard Lau <riclau@uk.ibm.com>
    Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
zsw007 added a commit to ibmruntimes/node that referenced this pull request Feb 12, 2020
deps: upgrade http-parser to v2.9.1
8672cc6 
Backport d41314e

Original commit message:

    PR-URL: nodejs/node#30473
    Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
    Reviewed-By: James M Snell <jasnell@gmail.com>
    Reviewed-By: Richard Lau <riclau@uk.ibm.com>
zsw007 added a commit to ibmruntimes/node that referenced this pull request Feb 12, 2020
http: opt-in insecure HTTP header parsing
942e61e 
Backport 496736f

Original commit message:

    Allow insecure HTTP header parsing. Make clear it is insecure.

    See:
    - nodejs/node#30553
    - nodejs/node#27711 (comment)
    - nodejs/node#30515

    PR-URL: nodejs/node#30567
    Backport-PR-URL: nodejs/node#30473
    Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
    Reviewed-By: Anna Henningsen <anna@addaleax.net>
    Reviewed-By: Denys Otrishko <shishugi@gmail.com>
    Reviewed-By: James M Snell <jasnell@gmail.com>
zsw007 added a commit to ibmruntimes/node that referenced this pull request Feb 12, 2020
test: check that --insecure-http-parser works
ec48296 
Backport ab1fcb8

Original commit message:

    Test that using --insecure-http-parser will disable validation of
    invalid characters in HTTP headers.

    See:
    - nodejs/node#30567

    PR-URL: nodejs/node#31253
    Backport-PR-URL: nodejs/node#30473
    Reviewed-By: Richard Lau <riclau@uk.ibm.com>
    Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
gengjiawen gengjiawen approved these changes
richardlau richardlau approved these changes

Assignees

No one assigned

Labels

http_parser Issues and PRs related to the HTTP Parser dependency or the http_parser binding. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

Back | FazBrowse Home | New Git URL