Skip to content

fix(decompress): treat inherited Object.prototype names as unsupported encodings - #5807

Open
NgoQuocViet2001 wants to merge 1 commit into
nodejs:mainfrom
NgoQuocViet2001:fix-decompress-inherited-encoding
Open

fix(decompress): treat inherited Object.prototype names as unsupported encodings#5807
NgoQuocViet2001 wants to merge 1 commit into
nodejs:mainfrom
NgoQuocViet2001:fix-decompress-inherited-encoding

Conversation

@NgoQuocViet2001

Copy link
Copy Markdown

Problem

supportedEncodings is a plain object literal, so the unsupported-encoding guard resolves inherited names through the prototype chain:

const supportedEncodings = {
  gzip: createGunzip,
  ...
}
...
if (!supportedEncodings[encoding]) {
  decompressors.length = 0
  return decompressors           // unsupported -> pass through
}
decompressors.push(supportedEncodings[encoding]())

onResponseStart lowercases the header before the lookup, and exactly two Object.prototype own-property names survive lowercasing — constructor and __proto__. Both are truthy, so both walk past the guard and get called.

Effect

Measured against main with a plain node:http origin and a client composed with interceptors.decompress():

content-encoding: "identity"          -> ok: status=200 body=hello
content-encoding: "bogus-encoding"    -> ok: status=200 body=hello
content-encoding: "constructor"       -> HUNG - promise never settled
content-encoding: "__proto__"         -> threw: TypeError: supportedEncodings[encoding] is not a function
content-encoding: "gzip, constructor" -> HUNG - promise never settled

constructor is the bad one: Object() returns a plain object, that object is pushed into the decompressor chain as if it were a stream, and client.request() then neither resolves nor rejects. No uncaughtException, no teardown — the caller's await and the socket both leak. A remote origin (or any intermediary that rewrites Content-Encoding) can wedge a client with a single response header.

An unrecognised encoding is supposed to pass through untouched, which is what bogus-encoding shows.

Fix

Give the lookup table a null prototype, so only its own entries can match. This matches the existing style in lib/core/util.js:973, lib/util/runtime-features.js:7 and lib/web/eventsource/eventsource.js:458.

const supportedEncodings = {
  __proto__: null,
  gzip: createGunzip,
  ...
}

All five cases above then pass through cleanly.

Test plan

  • Added three cases to test/interceptors/decompress.js beside the existing should pass through unsupported encoding, covering constructor, __proto__ and gzip, constructor.
  • Ran: node --test test/interceptors/decompress.js38 passing.
  • Checked: reverting only the __proto__: null line makes the constructor case hang rather than fail — the test run never terminates, which is the defect itself.
  • Ran: npx standard on both touched files → clean.

…d encodings

supportedEncodings is a plain object literal, so the guard

  if (!supportedEncodings[encoding])

resolves inherited names through the prototype chain instead of rejecting
them. onResponseStart lowercases the header first, which leaves exactly two
reachable from the wire: constructor and __proto__.

A response carrying Content-Encoding: constructor calls Object(), pushes the
plain object it returns into the decompressor chain, and the request then
never settles; __proto__ throws TypeError out of the interceptor. Give the
table a null prototype so only its own entries can match.
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.

1 participant