vm: refactor to use more primordials

[aduh95]

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

[nodejs-github-bot]

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

[github-actions]

Landed in 8297860...1d02a35

[rubenstolk]

Great - an automatic upgrade from 14.15.1 to 14.15.2 started crashing our production environment. Lesson learnt, don't accept minor updates from Node just like that...

[aduh95]

@rubenstolk Do you want to open an issue describing the bug?

[rubenstolk]

@aduh95 I don't know if it's a bug. While running this inside a vm script:

(req) { // an express request object
  const dummyReq = { ...req, model: undefined }; // create a copy of `req` with some modifications
  // Here dummyReq.headers has become undefined
}

This piece of code has been working for years, but since 14.15.2 the dummyReq doesn't have headers anymore...

[rubenstolk]

Hmm looking deeper into this, I don't think it has to do with the vm, I made an assumption here. The behavior is also there when running outside of vm.

[aduh95]

@rubenstolk I'm not able to reproduce the bug:

$ node --version
v14.15.2
$ node -p 'new vm.Script("\
const req = { headers: { headerKey: `value` }, method : `value` };\
const dummy = { ...req, method: undefined };\
dummy.headers\
").runInNewContext()'
{ headerKey: 'value' }

I'm seeing the expected { headerKey: 'value' } on stdout…

Opening a new issue would make your problem likely to be fixed quicker as more people would notice the issue and help fix it – only people already subscribed to this thread are seeing our messages here.

[rubenstolk]

@aduh95 this is what fails, so it's NOT because of vm:

require('http').createServer((req, res) => {
  const dummyReq = { ...req };
  console.log(!!req.headers, !!dummyReq.headers);
  res.writeHead(200, { 'Content-Type': 'text/plain' });
  res.end('ok');
}).listen(8000);

[ExE-Boss]

@rubenstolk
Do note that object spread only copies own enumerable properties, e.g.:

const result = { 
	...(Object.create(
		{
			prototypeProperty: "foo",
		},
		{
			nonEnumerableProperty: {
				configurable: true,
				enumerable: false,
				writable: true,
				value: 123,
			}
		}
	)),
};

Results in an empty object with a prototype of Object.prototype.

[rubenstolk]

Let's continue the conversation here: #36550, would be interested to understand why the behavior has changed @ExE-Boss !