Skip to content

fix(h2): keep a request header named __proto__ - #5669

Open
luantaraschi wants to merge 1 commit into
nodejs:mainfrom
luantaraschi:fix/h2-request-headers-proto
Open

fix(h2): keep a request header named __proto__#5669
luantaraschi wants to merge 1 commit into
nodejs:mainfrom
luantaraschi:fix/h2-request-headers-proto

Conversation

@luantaraschi

Copy link
Copy Markdown

This relates to...

The h2 half of what I flagged in a comment on #5667. test/prototype-headers.js already covers this concern for client.request over HTTP/1.

Rationale

buildRequestHeaders in lib/dispatcher/client-h2.js probes the accumulator by index:

const current = headers[key]
...
headers[key] = current ? `${current}, ${val}` : val

For __proto__ that probe returns Object.prototype, which is truthy, so the header takes the "already present" branch and the value becomes "[object Object], pwned". That string is then assigned through the Object.prototype setter, which refuses a string, so nothing is stored and the header never reaches the wire.

Reproduced over a real HTTP/2 connection, before the change:

x-control arrived: sent
__proto__ arrived: no

__proto__ is a valid field name, and the flat array form is exactly what this function receives from the dispatcher.

There is no Object.prototype pollution: the setter refuses the string, so the global prototype is untouched.

Changes

The probe uses Object.hasOwn so an inherited name is not mistaken for an existing header, and all four writes go through a setHeader helper using Object.defineProperty, the guard parseHeaders already uses in lib/core/util.js.

Test added to test/prototype-headers.js, alongside the HTTP/1 cases: an h2 request carrying __proto__ reaches the server with that header, with a control header that already worked.

test/+(http2|h2)*.js 104 passing with 1 skipped as before, lint clean.

Features

N/A

Bug Fixes

An HTTP/2 request no longer drops a header named __proto__, and no longer corrupts its value with [object Object], first.

Breaking Changes and Deprecations

None.

Status

buildRequestHeaders probed the accumulator with headers[key], which returns
Object.prototype for __proto__ instead of undefined. The header took the
already-present branch, its value became '[object Object], pwned', and the
assignment then hit the Object.prototype setter, which refuses a string, so
nothing reached the wire.

The probe uses Object.hasOwn and the writes go through a setHeader helper
using Object.defineProperty, the guard parseHeaders already uses.
Copilot AI lite review requested due to automatic review settings August 8, 2026 02:41

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codecov-commenter

codecov-commenter commented Aug 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.45%. Comparing base (dd85997) to head (fdd13bd).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5669      +/-   ##
==========================================
+ Coverage   93.43%   93.45%   +0.02%     
==========================================
  Files         110      110              
  Lines       38733    38795      +62     
==========================================
+ Hits        36190    36256      +66     
+ Misses       2543     2539       -4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants