Skip to content

http2: avoid Object.create(null) for Http2ServerRequest.headers - #33183

Closed
rexagod wants to merge 4 commits into
nodejs:masterfrom
rexagod:i-29829-12
Closed

http2: avoid Object.create(null) for Http2ServerRequest.headers#33183
rexagod wants to merge 4 commits into
nodejs:masterfrom
rexagod:i-29829-12

Conversation

@rexagod

@rexagod rexagod commented May 1, 2020

Copy link
Copy Markdown
Member

Currently http2.Http2ServerRequest.headers returns headers which is initialized by Object.create(null) which is a different behaviour than what is seen in http.

Refs: #29829

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 nodejs-github-bot added the http2 Issues and PRs related to the http2 subsystem. label May 1, 2020
@jasnell

jasnell commented May 1, 2020

Copy link
Copy Markdown
Member

The use of Object.create(null) in http/2 is intentional. For HTTP/1, the reason we do not use Object.create(null) for headers is because it would break too much code but it would have been preferable. I don't think this is one we want to change. A code comment to that effect would be good.

@BridgeAR BridgeAR left a comment

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.

LGTM % linter error.

@addaleax

Copy link
Copy Markdown
Member

Landed in ee87567 :)

@addaleax addaleax closed this May 19, 2020
addaleax pushed a commit that referenced this pull request May 19, 2020
Refs: #29829

PR-URL: #33183
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
codebytere pushed a commit that referenced this pull request May 21, 2020
Refs: #29829

PR-URL: #33183
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
codebytere pushed a commit that referenced this pull request Jun 7, 2020
Refs: #29829

PR-URL: #33183
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
codebytere pushed a commit to codebytere/node that referenced this pull request Jun 9, 2020
Refs: nodejs#29829

PR-URL: nodejs#33183
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@codebytere codebytere mentioned this pull request Jun 9, 2020
@codebytere codebytere mentioned this pull request Jun 28, 2020
QwireyInc pushed a commit to QwireyInc/node that referenced this pull request Apr 19, 2025
Refs: nodejs/node#29829

PR-URL: nodejs/node#33183
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

http2 Issues and PRs related to the http2 subsystem.

6 participants