Object.assign / spread of http request object difference between v14.15.1 and v14.15.2

[rubenstolk]

• Version: 14.15.2
• Platform: any
• Subsystem: any

What steps will reproduce the bug?

While trying to create a cloned version of an http request object, prototype properties/methods such as headers and get get lost.

const assert = require('assert');
const http = require('http');

const server = http
 .createServer((req, res) => {
 const dummyReq = { ...req };
 res.writeHead(200, { 'Content-Type': 'text/plain' });
 res.end('ok');
 assert.deepStrictEqual(req.headers, dummyReq.headers);
 })
 .listen();

server.on('listening', () => {
 http.get(`http://localhost:${server.address().port}`);
});

What is the expected behavior?

I honestly don't know if the behavior from 14.15.1 or from 14.15.2 is expected.

Behavior until 14.15.1: dummyReq.headers is not undefined.

What do you see instead?

Output in 14.15.2: dummyReq.headers is undefined.

Additional info

The same happens while using Object.assign

[rubenstolk]

As @ExE-Boss indicated here (#36023 (comment)), the http request object must now be of a different inheritance level...

[ExE-Boss]

Object.assign also copies only own enumerable properties, but uses [[Set]] instead of [[Define]], so:

const target = {};
Object.assign(
	target,
	JSON.parse(`{
		"__proto__": null
	}`,
);

results in target having its prototype set to null instead of adding an own __proto__ property, because of the Object.prototype.__proto__ accessor.

---

Whereas:

const target = {
	...(JSON.parse(`{
		"__proto__": null
	}`)),
};

Results in an object with an own __proto__ property set to null and a prototype of Object.prototype.

[aduh95]

This likely related to #35281.

[ExE-Boss]

This is definitely caused by #35281.

[aduh95]

I think a quick fix would be to do something like:

const dummyReq = { ...req, get headers() { return req.headers }, get trailers() { return req.trailers; } };

[rubenstolk]

Yep, thanks for that! I have already implemented something similar.

[BethGriggs]

Opinions on whether this warrants an urgent revert of #35281 and follow-up patch release this week? cc: @ronag @mcollina

(I'd need to get started on that today if so, as i'll be out-of-office/mostly offline from tomorrow until the New Year.)

[mcollina]

I'd just revert it in v14, not v15.

[mhdawson]

I agree with @mcollina that a revert in v14 would amke sense, and it would be good to get a release out.