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

http: make --insecure-http-parser configurable per-stream or per-server by addaleax · Pull Request #31448 · nodejs/node · GitHub

/ node Public

http: make --insecure-http-parser configurable per-stream or per-server - #31448

Closed
addaleax wants to merge 4 commits into
nodejs:masterfrom
addaleax:insecure-http-parser
ConversationCommits (4)ChecksFiles changed
Closed

http: make --insecure-http-parser configurable per-stream or per-server#31448
addaleax wants to merge 4 commits into
nodejs:masterfrom
addaleax:insecure-http-parser

Conversation

addaleax commented Jan 21, 2020

Copy link
Copy Markdown
Member

From the issue:

Some servers deviate from HTTP spec enougth that Node.js can't
communicate with them, but "work" when --insecure-http-parser
is enabled globally. It would be useful to be able to use this
mode, as a client, only when connecting to known bad servers.

This is largely equivalent to #31446
in terms of code changes.

Fixes: #31440
Refs: #31446

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

http: make --insecure-http-parser configurable per-stream or per-server
324866d 
From the issue:

> Some servers deviate from HTTP spec enougth that Node.js can't
> communicate with them, but "work" when `--insecure-http-parser`
> is enabled globally. It would be useful to be able to use this
> mode, as a client, only when connecting to known bad servers.

This is largely equivalent to nodejs#31446
in terms of code changes.

Fixes: nodejs#31440
Refs: nodejs#31446
addaleax added http Issues or PRs related to the http subsystem. semver-minor PRs that contain new features and should be released in the next minor version. labels Jan 21, 2020
addaleax requested a review from sam-github January 21, 2020 21:37

nodejs-github-bot commented Jan 21, 2020

Copy link
Copy Markdown
Collaborator

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

sam-github approved these changes Jan 21, 2020
View reviewed changes

sam-github left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

+1 for the code, but I suggested a doc clarification (it would apply to both http and https obviously).

Comment thread doc/api/http.md Outdated
addaleax added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jan 22, 2020

nodejs-github-bot commented Jan 22, 2020

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Jan 22, 2020

Copy link
Copy Markdown
Collaborator

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

Trott reviewed Jan 23, 2020
View reviewed changes
Comment thread doc/api/http.md Outdated
Trott reviewed Jan 23, 2020
View reviewed changes
Comment thread doc/api/http.md Outdated
fixup! http: make --insecure-http-parser configurable per-stream or p…
819468e 
…er-server

Co-Authored-By: Rich Trott <rtrott@gmail.com>

nodejs-github-bot commented Jan 23, 2020

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Jan 23, 2020

Copy link
Copy Markdown
Collaborator

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

addaleax added a commit that referenced this pull request Jan 24, 2020
http: make --insecure-http-parser configurable per-stream or per-server
d6f1e39 
From the issue:

> Some servers deviate from HTTP spec enougth that Node.js can't
> communicate with them, but "work" when `--insecure-http-parser`
> is enabled globally. It would be useful to be able to use this
> mode, as a client, only when connecting to known bad servers.

This is largely equivalent to #31446
in terms of code changes.

Fixes: #31440
Refs: #31446

PR-URL: #31448
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

addaleax commented Jan 24, 2020

Copy link
Copy Markdown
Member Author

Landed in d6f1e39, thanks for the reviews!

addaleax closed this Jan 24, 2020
addaleax deleted the insecure-http-parser branch January 24, 2020 16:55
addaleax added a commit to sam-github/node that referenced this pull request Jan 24, 2020
http: make --insecure-http-parser configurable per-stream or per-server
22525e2 
From the issue:

> Some servers deviate from HTTP spec enougth that Node.js can't
> communicate with them, but "work" when `--insecure-http-parser`
> is enabled globally. It would be useful to be able to use this
> mode, as a client, only when connecting to known bad servers.

This is largely equivalent to nodejs#31446
in terms of code changes.

Fixes: nodejs#31440
Refs: nodejs#31446

PR-URL: nodejs#31448
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
addaleax mentioned this pull request Jan 24, 2020
Closed
4 tasks
addaleax added a commit to addaleax/node that referenced this pull request Jan 24, 2020
http: make --insecure-http-parser configurable per-stream or per-server
c204d14 
From the issue:

> Some servers deviate from HTTP spec enougth that Node.js can't
> communicate with them, but "work" when `--insecure-http-parser`
> is enabled globally. It would be useful to be able to use this
> mode, as a client, only when connecting to known bad servers.

This is largely equivalent to nodejs#31446
in terms of code changes.

Fixes: nodejs#31440
Refs: nodejs#31446

PR-URL: nodejs#31448
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

addaleax commented Jan 24, 2020

Copy link
Copy Markdown
Member Author

This should land wherever --insecure-http-parser is provided, imo – v12.x backport is in #31500, v10.x backport is added to #30471 (along with the flag itself)

zsw007 added a commit to ibmruntimes/node that referenced this pull request Feb 11, 2020
http: make --insecure-http-parser configurable per-stream or per-server
e989acc 
Backport 7fc5656

Original commit message:

    From the issue:

    > Some servers deviate from HTTP spec enougth that Node.js can't
    > communicate with them, but "work" when `--insecure-http-parser`
    > is enabled globally. It would be useful to be able to use this
    > mode, as a client, only when connecting to known bad servers.

    This is largely equivalent to
    nodejs/node#31446 in terms of code changes.

    Fixes: nodejs/node#31440
    Refs: nodejs/node#31446

    Backport-PR-URL: nodejs/node#31500
    PR-URL: nodejs/node#31448
    Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
    Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
    Reviewed-By: James M Snell <jasnell@gmail.com>
    Reviewed-By: Rich Trott <rtrott@gmail.com>
zsw007 added a commit to ibmruntimes/node that referenced this pull request Feb 12, 2020
http: make --insecure-http-parser configurable per-stream or per-server
5d54f9a 
Backport 7fc5656

Original commit message:

    From the issue:

    > Some servers deviate from HTTP spec enougth that Node.js can't
    > communicate with them, but "work" when `--insecure-http-parser`
    > is enabled globally. It would be useful to be able to use this
    > mode, as a client, only when connecting to known bad servers.

    This is largely equivalent to
    nodejs/node#31446 in terms of code changes.

    Fixes: nodejs/node#31440
    Refs: nodejs/node#31446

    Backport-PR-URL: nodejs/node#31500
    PR-URL: nodejs/node#31448
    Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
    Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
    Reviewed-By: James M Snell <jasnell@gmail.com>
    Reviewed-By: Rich Trott <rtrott@gmail.com>
zsw007 added a commit to ibmruntimes/node that referenced this pull request Feb 12, 2020
http: make --insecure-http-parser configurable per-stream or per-server
5836ae5 
Backport 7fc5656

Original commit message:

    From the issue:

    > Some servers deviate from HTTP spec enougth that Node.js can't
    > communicate with them, but "work" when `--insecure-http-parser`
    > is enabled globally. It would be useful to be able to use this
    > mode, as a client, only when connecting to known bad servers.

    This is largely equivalent to
    nodejs/node#31446 in terms of code changes.

    Fixes: nodejs/node#31440
    Refs: nodejs/node#31446

    Backport-PR-URL: nodejs/node#31500
    PR-URL: nodejs/node#31448
    Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
    Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
    Reviewed-By: James M Snell <jasnell@gmail.com>
    Reviewed-By: Rich Trott <rtrott@gmail.com>
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
Trott Trott approved these changes
cjihrig cjihrig approved these changes
+1 more reviewer
sam-github sam-github approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. http Issues or PRs related to the http subsystem. 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.

allow insecure-http-header parsing to be requested per server

6 participants

Back | FazBrowse Home | New Git URL