Skip to content
This repository was archived by the owner on Sep 9, 2024. It is now read-only.

fix: only merge non-enumerable global properties that are not in VM scope #28

Merged
merged 4 commits into from
Mar 24, 2022

Conversation

Josh-Cena
Copy link
Contributor

@Josh-Cena Josh-Cena commented Mar 24, 2022

Follow-up of #27. Fix #29

Case as where #27 failed:

new vm.Script("globalThis.Prism = 1; console.log(Prism)").runInNewContext({ globalThis });

Because the globalThis in this case is an exotic object, the latter Prism isn't actually made a "global".

new vm.Script("console.log({} instanceof Object)").runInNewContext({ console, Object });

Logs false because Object is exotic. Removing Object from the scope causes it to log true.

The resolution is to filter out anything that's already in the VM scope.

cc @pierrec This is a hotfix.
cc @slorber FYI

Copy link

@slorber slorber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 looks like a better solution to me, and can confirm it works for Docusaurus

would still be useful to allow excluding some globals?

for (var k, i = 0, n = keys.length; i < n; i++) {
k = keys[i]
a[k] = b[k]
}
return a
}

var vmGlobals = new vm.Script('vmGlobals = Object.getOwnPropertyNames(globalThis)')
Copy link

@slorber slorber Mar 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review hint:

  • 👍 eager call but only takes < 1ms for my mac

  • List is:

[
  "Object",
  "Function",
  "Array",
  "Number",
  "parseFloat",
  "parseInt",
  "Infinity",
  "NaN",
  "undefined",
  "Boolean",
  "String",
  "Symbol",
  "Date",
  "Promise",
  "RegExp",
  "Error",
  "AggregateError",
  "EvalError",
  "RangeError",
  "ReferenceError",
  "SyntaxError",
  "TypeError",
  "URIError",
  "globalThis",
  "JSON",
  "Math",
  "console",
  "Intl",
  "ArrayBuffer",
  "Uint8Array",
  "Int8Array",
  "Uint16Array",
  "Int16Array",
  "Uint32Array",
  "Int32Array",
  "Float32Array",
  "Float64Array",
  "Uint8ClampedArray",
  "BigUint64Array",
  "BigInt64Array",
  "DataView",
  "Map",
  "BigInt",
  "Set",
  "WeakMap",
  "WeakSet",
  "Proxy",
  "Reflect",
  "FinalizationRegistry",
  "WeakRef",
  "decodeURI",
  "decodeURIComponent",
  "encodeURI",
  "encodeURIComponent",
  "escape",
  "unescape",
  "eval",
  "isFinite",
  "isNaN",
  "SharedArrayBuffer",
  "Atomics",
  "WebAssembly",
];

if (!vmGlobals.includes(name)) {
sandbox[name] = global[name]
}
})
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review hint

New globals automatically forwarded:

[
  "global",
  "process",
  "Buffer",
  "atob",
  "btoa",
  "URL",
  "URLSearchParams",
  "TextEncoder",
  "TextDecoder",
  "AbortController",
  "AbortSignal",
  "EventTarget",
  "Event",
  "MessageChannel",
  "MessagePort",
  "MessageEvent",
  "clearInterval",
  "clearTimeout",
  "setInterval",
  "setTimeout",
  "queueMicrotask",
  "performance",
  "clearImmediate",
  "setImmediate",
  "__extends",
  "__assign",
  "__rest",
  "__decorate",
  "__param",
  "__metadata",
  "__awaiter",
  "__generator",
  "__exportStar",
  "__createBinding",
  "__values",
  "__read",
  "__spread",
  "__spreadArrays",
  "__spreadArray",
  "__await",
  "__asyncGenerator",
  "__asyncDelegator",
  "__asyncValues",
  "__makeTemplateObject",
  "__importStar",
  "__importDefault",
  "__classPrivateFieldGet",
  "__classPrivateFieldSet",
  "consola",
  "regeneratorRuntime",
];

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of these globals are injected by the transpiler. Only stuff like URL process TextDecoder etc. are of our interest.

@Josh-Cena
Copy link
Contributor Author

Josh-Cena commented Mar 24, 2022

would still be useful to allow excluding some globals?

I don't think that's necessarily the goal of this module and I frankly can't see how that's useful after this bug is fixed😅 Since we have a forked SSG plugin, if you really want to exclude some globals, just use includeGlobals=false and use the more verbose scope instead. WDYT?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ReferenceError: __webpack_require__ is not defined
3 participants