Skip to content

http: add maxHeaderSize property#24860

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
cjihrig merged 1 commit into from
Dec 20, 2018
Merged

http: add maxHeaderSize property #24860

cjihrig merged 1 commit into from
Dec 20, 2018
+36 −0

Conversation

@cjihrig
Copy link
Contributor

@cjihrigcjihrig commented Dec 6, 2018•
edited
Loading

Requested by @mcollina in #24811, but I didn't want to stall that out. Ignore the commits to deps and cli, which are just #24811.

This PR exposes the value of --max-http-header-size as a property of the http module.

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

nodejs-github-bot commented Dec 6, 2018

@cjihrig build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1847/pipeline

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Dec 6, 2018
@cjihrigcjihrig mentioned this pull request Dec 6, 2018
Merged
4 tasks
mcollina
mcollina approved these changes Dec 7, 2018
View reviewed changes
Copy link
Member

@mcollinamcollina left a comment

Choose a reason for hiding this comment

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

LGTM

mhdawson
mhdawson approved these changes Dec 12, 2018
View reviewed changes
Copy link
Member

@mhdawsonmhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@mcollina
Copy link
Member

mcollina commented Dec 12, 2018

@nodejs/lts would you be ok in backporting this to 6/8/10?

sam-github
sam-github reviewed Dec 12, 2018
View reviewed changes
lib/http.js Outdated
Copy link
Contributor

@sam-githubsam-githubDec 12, 2018

Choose a reason for hiding this comment

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

should this be configurable: false, writeable: false?

Copy link
ContributorAuthor

@cjihrigcjihrigDec 20, 2018

Choose a reason for hiding this comment

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

writable is not supported on accessor descriptors.

@cjihrigcjihrig added the semver-minor PRs that contain new features and should be released in the next minor version. label Dec 20, 2018
@cjihrig
http: add maxHeaderSize property
9ac108d
This commit exposes the value of --max-http-header-size as a property of the http module. PR-URL: nodejs#24860 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: James M Snell <[email protected]>
@cjihrig
Copy link
ContributorAuthor

cjihrig commented Dec 20, 2018

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

@cjihrigcjihrig merged commit 9ac108d into nodejs:masterDec 20, 2018
@cjihrigcjihrig deleted the http-max-headers branch December 20, 2018 18:17
MylesBorins pushed a commit that referenced this pull request Dec 25, 2018
@cjihrig@MylesBorins
http: add maxHeaderSize property
5e1ec70
This commit exposes the value of --max-http-header-size as a property of the http module. PR-URL: #24860 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: James M Snell <[email protected]>
@MylesBorins
Copy link
Contributor

MylesBorins commented Dec 25, 2018

Should this be included in the various releases being prepared for 6.x -> 11.x right now?

@mcollina
Copy link
Member

mcollina commented Dec 25, 2018

I think so, yes.

MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Dec 25, 2018
@cjihrig@MylesBorins
http: add maxHeaderSize property
e59cc49
This commit exposes the value of --max-http-header-size as a property of the http module. PR-URL: nodejs#24860 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 25, 2018
@cjihrig@MylesBorins
http: add maxHeaderSize property
17f4208
This commit exposes the value of --max-http-header-size as a property of the http module. PR-URL: #24860 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 25, 2018
@cjihrig@MylesBorins
http: add maxHeaderSize property
c5142b3
This commit exposes the value of --max-http-header-size as a property of the http module. Backport-PR-URL: #25218 PR-URL: #24860 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 25, 2018
@cjihrig@MylesBorins
http: add maxHeaderSize property
b6d3afb
This commit exposes the value of --max-http-header-size as a property of the http module. PR-URL: #24860 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 25, 2018
@cjihrig@MylesBorins
http: add maxHeaderSize property
446f8b5
This commit exposes the value of --max-http-header-size as a property of the http module. Backport-PR-URL: #25218 PR-URL: #24860 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 25, 2018
@cjihrig@MylesBorins
http: add maxHeaderSize property
b69e096
This commit exposes the value of --max-http-header-size as a property of the http module. Backport-PR-URL: #25218 PR-URL: #24860 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 25, 2018
@cjihrig@MylesBorins
http: add maxHeaderSize property
c0c4de7
This commit exposes the value of --max-http-header-size as a property of the http module. Backport-PR-URL: #25218 PR-URL: #24860 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: James M Snell <[email protected]>
This was referenced Dec 25, 2018
Merged
Merged
Merged
MylesBorins added a commit that referenced this pull request Dec 25, 2018
@MylesBorins
2018-12-26, Version 6.16.0 'Boron' (LTS)
a3aa4b4
The 6.15.0 security release introduced some unexpected breakages on the 6.x release line. This is a special release to fix a regression in the HTTP binary upgrade response body and add a missing CLI flag to adjust the max header size of the http parser. Notable changes: * cli: - add --max-http-header-size flag (cjihrig) #24811 * http: - add maxHeaderSize property (cjihrig) #24860 PR-URL: #25178
MylesBorins added a commit that referenced this pull request Dec 25, 2018
@MylesBorins
2018-12-26, Version 8.15.0 'Carbon' (LTS)
6d1c367
The 8.14.0 security release introduced some unexpected breakages on the 8.x release line. This is a special release to fix a regression in the HTTP binary upgrade response body and add a missing CLI flag to adjust the max header size of the http parser. Notable changes: * cli: - add --max-http-header-size flag (cjihrig) #24811 * http: - add maxHeaderSize property (cjihrig) #24860 PR-URL: #25177
MylesBorins added a commit that referenced this pull request Dec 25, 2018
@MylesBorins
2018-12-26, Version 11.6.0 (Current)
f59e73b
Notable Changes: * cli: - add --max-http-header-size flag (cjihrig) #24811 * crypto: - always accept certificates as public keys (Tobias Nießen) #24234 - add key object API (Tobias Nießen) [#24234](#24234) - update root certificates (Sam Roberts) #25113 * deps: - upgrade to libuv 1.24.1 (cjihrig) #25078 - upgrade npm to 6.5.0 (Audrey Eschright) #24734 * http: - add maxHeaderSize property (cjihrig) #24860 PR-URL: #25175
@MylesBorinsMylesBorins mentioned this pull request Dec 25, 2018
Merged
MylesBorins pushed a commit that referenced this pull request Dec 26, 2018
@cjihrig@MylesBorins
http: add maxHeaderSize property
ae50f48
This commit exposes the value of --max-http-header-size as a property of the http module. PR-URL: #24860 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: James M Snell <[email protected]>
MylesBorins added a commit that referenced this pull request Dec 26, 2018
@MylesBorins
2018-12-26, Version 11.6.0 (Current)
fb9130f
Notable Changes: * cli: - add --max-http-header-size flag (cjihrig) #24811 * crypto: - always accept certificates as public keys (Tobias Nießen) #24234 - add key object API (Tobias Nießen) [#24234](#24234) - update root certificates (Sam Roberts) #25113 * deps: - upgrade to libuv 1.24.1 (cjihrig) #25078 - upgrade npm to 6.5.0 (Audrey Eschright) #24734 * http: - add maxHeaderSize property (cjihrig) #24860 PR-URL: #25175
MylesBorins added a commit that referenced this pull request Dec 26, 2018
@MylesBorins
2018-12-26, Version 11.6.0 (Current)
968e901
Notable Changes: * cli: - add --max-http-header-size flag (cjihrig) #24811 * crypto: - always accept certificates as public keys (Tobias Nießen) #24234 - add key object API (Tobias Nießen) [#24234](#24234) - update root certificates (Sam Roberts) #25113 * deps: - upgrade to libuv 1.24.1 (cjihrig) #25078 - upgrade npm to 6.5.0 (Audrey Eschright) #24734 * http: - add maxHeaderSize property (cjihrig) #24860 PR-URL: #25175
MylesBorins added a commit that referenced this pull request Dec 26, 2018
@MylesBorins
2018-12-26, Version 6.16.0 'Boron' (LTS)
7b9c9b3
The 6.15.0 security release introduced some unexpected breakages on the 6.x release line. This is a special release to fix a regression in the HTTP binary upgrade response body and add a missing CLI flag to adjust the max header size of the http parser. Notable changes: * cli: - add --max-http-header-size flag (cjihrig) #24811 * http: - add maxHeaderSize property (cjihrig) #24860 PR-URL: #25178
MylesBorins added a commit that referenced this pull request Dec 26, 2018
@MylesBorins
2018-12-26, Version 8.15.0 'Carbon' (LTS)
9542324
The 8.14.0 security release introduced some unexpected breakages on the 8.x release line. This is a special release to fix a regression in the HTTP binary upgrade response body and add a missing CLI flag to adjust the max header size of the http parser. Notable changes: * cli: - add --max-http-header-size flag (cjihrig) #24811 * http: - add maxHeaderSize property (cjihrig) #24860 PR-URL: #25177
MylesBorins added a commit that referenced this pull request Dec 26, 2018
@MylesBorins
2018-12-26, Version 10.15.0 'Dubnium' (LTS)
78f2e14
The 10.14.0 security release introduced some unexpected breakages on the 10.x release line. This is a special release to fix a regression in the HTTP binary upgrade response body and add a missing CLI flag to adjust the max header size of the http parser. Notable Changes: * cli: - add --max-http-header-size flag (cjihrig) #24811 * http: - add maxHeaderSize property (cjihrig) #24860 PR-URL: #25176
MylesBorins added a commit that referenced this pull request Dec 26, 2018
@MylesBorins
2018-12-26, Version 11.6.0 (Current)
e01999d
Notable Changes: * cli: - add --max-http-header-size flag (cjihrig) #24811 * crypto: - always accept certificates as public keys (Tobias Nießen) #24234 - add key object API (Tobias Nießen) [#24234](#24234) - update root certificates (Sam Roberts) #25113 * deps: - upgrade to libuv 1.24.1 (cjihrig) #25078 - upgrade npm to 6.5.0 (Audrey Eschright) #24734 * http: - add maxHeaderSize property (cjihrig) #24860 PR-URL: #25175
This was referenced Dec 26, 2018
Closed
Closed
Closed
Closed
Closed
Closed
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
@cjihrig@refack
http: add maxHeaderSize property
d484553
This commit exposes the value of --max-http-header-size as a property of the http module. PR-URL: nodejs#24860 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: James M Snell <[email protected]>
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
@MylesBorins@refack
2018-12-26, Version 6.16.0 'Boron' (LTS)
3de976b
The 6.15.0 security release introduced some unexpected breakages on the 6.x release line. This is a special release to fix a regression in the HTTP binary upgrade response body and add a missing CLI flag to adjust the max header size of the http parser. Notable changes: * cli: - add --max-http-header-size flag (cjihrig) nodejs#24811 * http: - add maxHeaderSize property (cjihrig) nodejs#24860 PR-URL: nodejs#25178
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
@MylesBorins@refack
2018-12-26, Version 8.15.0 'Carbon' (LTS)
2b68938
The 8.14.0 security release introduced some unexpected breakages on the 8.x release line. This is a special release to fix a regression in the HTTP binary upgrade response body and add a missing CLI flag to adjust the max header size of the http parser. Notable changes: * cli: - add --max-http-header-size flag (cjihrig) nodejs#24811 * http: - add maxHeaderSize property (cjihrig) nodejs#24860 PR-URL: nodejs#25177
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
@MylesBorins@refack
2018-12-26, Version 10.15.0 'Dubnium' (LTS)
19c72e3
The 10.14.0 security release introduced some unexpected breakages on the 10.x release line. This is a special release to fix a regression in the HTTP binary upgrade response body and add a missing CLI flag to adjust the max header size of the http parser. Notable Changes: * cli: - add --max-http-header-size flag (cjihrig) nodejs#24811 * http: - add maxHeaderSize property (cjihrig) nodejs#24860 PR-URL: nodejs#25176
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
@MylesBorins@refack
2018-12-26, Version 11.6.0 (Current)
c842f7b
Notable Changes: * cli: - add --max-http-header-size flag (cjihrig) nodejs#24811 * crypto: - always accept certificates as public keys (Tobias Nießen) nodejs#24234 - add key object API (Tobias Nießen) [nodejs#24234](nodejs#24234) - update root certificates (Sam Roberts) nodejs#25113 * deps: - upgrade to libuv 1.24.1 (cjihrig) nodejs#25078 - upgrade npm to 6.5.0 (Audrey Eschright) nodejs#24734 * http: - add maxHeaderSize property (cjihrig) nodejs#24860 PR-URL: nodejs#25175
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollinamcollinamcollina approved these changes

@jasnelljasnelljasnell approved these changes

@codebyterecodebyterecodebytere approved these changes

@richardlaurichardlaurichardlau approved these changes

@mhdawsonmhdawsonmhdawson approved these changes

+1 more reviewer

@sam-githubsam-githubsam-github left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c++Issues and PRs that require attention from people who are familiar with C++.lib / srcIssues and PRs related to general changes in the lib or src directory.semver-minorPRs 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

@cjihrig@nodejs-github-bot@mcollina@MylesBorins@sam-github@jasnell@codebytere@richardlau@mhdawson