vm: introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY #54394

joyeecheung · 2024-08-15T14:47:19Z

vm: introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY

Text for releasers:

new option for vm.createContext() to create a context with a freezable globalThis

This implements a flavor of vm.createContext() and friends that creates a context without contextifying its global object when vm.constants.DONT_CONTEXTIFY is used.
This is suitable when users want to freeze the context (impossible when the global is contextified i.e. has interceptors installed) or speed up the global access if they don't need the interceptor behavior.

const vm = require('node:vm');

{
  const oldContext = vm.createContext();
  console.log(vm.runInContext('globalThis', context) === oldContext);  // false
  console.log(oldContext.Array);  // undefined

  // In contextified contexts, freezing doesn't work
  try {
    vm.runInContext('Object.freeze(globalThis);', oldContext);
  } catch {
    // TypeError: Cannot freeze
  }
  console.log(vm.runInContext('globalThis.foo = 1; foo;', oldContext));  // 1
}

{
  const newContext = vm.createContext(vm.constants.DONT_CONTEXTIFY);
  console.log(vm.runInContext('globalThis', newContext) === newContext);  // true
  console.log(newContext.Array);  // [Function: Array]

  // In contexts without its global contextifyed, freezing works and prevents scripts from
  // accidentally leaking globals.
  vm.runInContext('Object.freeze(globalThis);', newContext);
  try {
    vm.runInContext('globalThis.foo = 1; foo;', newContext);
  } catch(e) {
    console.log(e); // Uncaught ReferenceError: foo is not defined
  }
}

github-actions · 2024-08-15T14:51:38Z

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @joyeecheung.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

codecov · 2024-08-15T16:42:52Z

Codecov Report

Attention: Patch coverage is 82.14286% with 10 lines in your changes missing coverage. Please review.

Project coverage is 87.33%. Comparing base (9edf4a0) to head (d43c426).
Report is 62 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_contextify.cc	80.43%	1 Missing and 8 partials ⚠️
src/node_contextify.h	75.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #54394   +/-   ##
=======================================
  Coverage   87.32%   87.33%           
=======================================
  Files         649      649           
  Lines      182603   182590   -13     
  Branches    35042    35039    -3     
=======================================
+ Hits       159464   159470    +6     
+ Misses      16400    16391    -9     
+ Partials     6739     6729   -10

Files with missing lines	Coverage Δ
lib/vm.js	`99.28% <100.00%> (+<0.01%)`	⬆️
src/node_contextify.h	`82.35% <75.00%> (-3.37%)`	⬇️
src/node_contextify.cc	`80.84% <80.43%> (+0.21%)`	⬆️

... and 46 files with indirect coverage changes

doc/api/vm.md

nodejs-github-bot · 2024-08-19T16:39:50Z

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

nodejs-github-bot · 2024-08-22T13:31:50Z

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

legendecas

Overall LGTM. Thanks!

legendecas · 2024-08-23T11:37:40Z

src/node_contextify.cc

+      if (!ctor_name->Equals(v8_context, env->object_string())
+               .FromMaybe(false) &&
+          new_context_global
+              ->DefineOwnProperty(


Not really relevant to this PR but I think we should not automatically create an own @@ToStringTag property on the global object: #54522.

src/node_contextify.cc

test/parallel/test-vm-context-dont-contextify.js

This implements a flavor of vm.createContext() and friends that creates a context without contextifying its global object. This is suitable when users want to freeze the context (impossible when the global is contextified i.e. has interceptors installed) or speed up the global access if they don't need the interceptor behavior. ```js const vm = require('node:vm'); const context = vm.createContext(vm.constants.DONT_CONTEXTIFY); // In contexts with contextified global objects, this is false. // In vanilla contexts this is true. console.log(vm.runInContext('globalThis', context) === context); // In contexts with contextified global objects, this would throw, // but in vanilla contexts freezing the global object works. vm.runInContext('Object.freeze(globalThis);', context); // In contexts with contextified global objects, freezing throws // and won't be effective. In vanilla contexts, freezing works // and prevents scripts from accidentally leaking globals. try { vm.runInContext('globalThis.foo = 1; foo;', context); } catch(e) { console.log(e); // Uncaught ReferenceError: foo is not defined } console.log(context.Array); // [Function: Array] ```

nodejs-github-bot · 2024-08-27T09:28:51Z

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

nodejs-github-bot · 2024-08-27T16:05:18Z

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

src/node_contextify.h

Co-authored-by: Chengzhong Wu <legendecas@gmail.com>

nodejs-github-bot · 2024-08-28T13:40:18Z

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

nodejs-github-bot · 2024-08-28T17:31:52Z

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

joyeecheung · 2024-08-28T22:44:00Z

CI is finally green. Can I get some re-review please? @addaleax @jasnell @legendecas ?

nodejs-github-bot · 2024-08-29T09:05:05Z

Landed in d98cfcc

This implements a flavor of vm.createContext() and friends that creates a context without contextifying its global object. This is suitable when users want to freeze the context (impossible when the global is contextified i.e. has interceptors installed) or speed up the global access if they don't need the interceptor behavior. ```js const vm = require('node:vm'); const context = vm.createContext(vm.constants.DONT_CONTEXTIFY); // In contexts with contextified global objects, this is false. // In vanilla contexts this is true. console.log(vm.runInContext('globalThis', context) === context); // In contexts with contextified global objects, this would throw, // but in vanilla contexts freezing the global object works. vm.runInContext('Object.freeze(globalThis);', context); // In contexts with contextified global objects, freezing throws // and won't be effective. In vanilla contexts, freezing works // and prevents scripts from accidentally leaking globals. try { vm.runInContext('globalThis.foo = 1; foo;', context); } catch(e) { console.log(e); // Uncaught ReferenceError: foo is not defined } console.log(context.Array); // [Function: Array] ``` PR-URL: nodejs#54394 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

This implements a flavor of vm.createContext() and friends that creates a context without contextifying its global object. This is suitable when users want to freeze the context (impossible when the global is contextified i.e. has interceptors installed) or speed up the global access if they don't need the interceptor behavior. ```js const vm = require('node:vm'); const context = vm.createContext(vm.constants.DONT_CONTEXTIFY); // In contexts with contextified global objects, this is false. // In vanilla contexts this is true. console.log(vm.runInContext('globalThis', context) === context); // In contexts with contextified global objects, this would throw, // but in vanilla contexts freezing the global object works. vm.runInContext('Object.freeze(globalThis);', context); // In contexts with contextified global objects, freezing throws // and won't be effective. In vanilla contexts, freezing works // and prevents scripts from accidentally leaking globals. try { vm.runInContext('globalThis.foo = 1; foo;', context); } catch(e) { console.log(e); // Uncaught ReferenceError: foo is not defined } console.log(context.Array); // [Function: Array] ``` PR-URL: #54394 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

Notable changes: net: * (SEMVER-MINOR) exclude ipv6 loopback addresses from server.listen (Giovanni Bucci) #54264 src: * (SEMVER-MINOR) add JS APIs for compile cache and NODE_DISABLE_COMPILE_CACHE (Joyee Cheung) #54501 src,lib: * (SEMVER-MINOR) add performance.uvMetricsInfo (Rafael Gonzaga) #54413 test_runner: * (SEMVER-MINOR) add support for coverage thresholds (Aviv Keller) #54429 * (SEMVER-MINOR) support running tests in process (Colin Ihrig) #53927 * (SEMVER-MINOR) defer inheriting hooks until run() (Colin Ihrig) #53927 vm: * (SEMVER-MINOR) introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY (Joyee Cheung) #54394 PR-URL: #54560

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Aug 15, 2024

joyeecheung force-pushed the vanilla-context branch from 2f4dbe9 to 3e87824 Compare August 15, 2024 14:48

joyeecheung added semver-minor PRs that contain new features and should be released in the next minor version. notable-change PRs with changes that should be highlighted in changelogs. labels Aug 15, 2024

joyeecheung added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Aug 15, 2024

This comment was marked as outdated.

Sign in to view

joyeecheung force-pushed the vanilla-context branch from 3e87824 to 055c9c9 Compare August 15, 2024 15:13

legendecas reviewed Aug 15, 2024

View reviewed changes

doc/api/vm.md Show resolved Hide resolved

addaleax reviewed Aug 15, 2024

View reviewed changes

doc/api/vm.md Outdated Show resolved Hide resolved

doc/api/vm.md Outdated Show resolved Hide resolved

joyeecheung force-pushed the vanilla-context branch 4 times, most recently from 94e06f3 to 8241574 Compare August 19, 2024 15:53

joyeecheung changed the title ~~vm: introduce vanilla contexts via vm.constants.VANILLA_CONTEXT~~ vm: introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY Aug 19, 2024

joyeecheung commented Aug 19, 2024

View reviewed changes

doc/api/vm.md Show resolved Hide resolved

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 19, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 19, 2024

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 22, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 22, 2024

legendecas reviewed Aug 23, 2024

View reviewed changes

jasnell approved these changes Aug 23, 2024

View reviewed changes

fixup! vm: introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY

c6cb027

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 27, 2024

legendecas approved these changes Aug 27, 2024

View reviewed changes

src/node_contextify.h Outdated Show resolved Hide resolved

fixup! vm: introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY

d43c426

Co-authored-by: Chengzhong Wu <legendecas@gmail.com>

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 28, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 28, 2024

legendecas approved these changes Aug 29, 2024

View reviewed changes

legendecas added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Aug 29, 2024

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 29, 2024

nodejs-github-bot merged commit d98cfcc into nodejs:main Aug 29, 2024
58 checks passed

legendecas added the vm Issues and PRs related to the vm subsystem. label Aug 30, 2024

RafaelGSS mentioned this pull request Aug 30, 2024

v22.8.0 proposal #54560

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

vm: introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY #54394

vm: introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY #54394

joyeecheung commented Aug 15, 2024 •

edited

Loading

github-actions bot commented Aug 15, 2024

This comment was marked as outdated.

codecov bot commented Aug 15, 2024 •

edited

Loading

nodejs-github-bot commented Aug 19, 2024

nodejs-github-bot commented Aug 22, 2024

legendecas left a comment

legendecas Aug 23, 2024

nodejs-github-bot commented Aug 27, 2024

nodejs-github-bot commented Aug 27, 2024

nodejs-github-bot commented Aug 28, 2024

nodejs-github-bot commented Aug 28, 2024

joyeecheung commented Aug 28, 2024

nodejs-github-bot commented Aug 29, 2024

vm: introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY #54394

vm: introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY #54394

Conversation

joyeecheung commented Aug 15, 2024 • edited Loading

new option for vm.createContext() to create a context with a freezable globalThis

github-actions bot commented Aug 15, 2024

This comment was marked as outdated.

codecov bot commented Aug 15, 2024 • edited Loading

Codecov Report

nodejs-github-bot commented Aug 19, 2024

nodejs-github-bot commented Aug 22, 2024

legendecas left a comment

Choose a reason for hiding this comment

legendecas Aug 23, 2024

Choose a reason for hiding this comment

nodejs-github-bot commented Aug 27, 2024

nodejs-github-bot commented Aug 27, 2024

nodejs-github-bot commented Aug 28, 2024

nodejs-github-bot commented Aug 28, 2024

joyeecheung commented Aug 28, 2024

nodejs-github-bot commented Aug 29, 2024

joyeecheung commented Aug 15, 2024 •

edited

Loading

codecov bot commented Aug 15, 2024 •

edited

Loading