Skip to content

Commit

Permalink
ensure that globals includes process and console
Browse files Browse the repository at this point in the history
The code attaches global properties to the sandbox context by iterating
over all the enumerable properties of `global`. However, in node v10,
`console` switched [to being non-enmuerable][1]. This means that for
users of this library with node>10, any `console.log`s in evaluated
scripts will fail.

This commit fixes this issue by manually attaching console to the
sandbox (when globals are being used). A test has been added. Prior to
the change to eval.js, the test would pass in node v8 but fail in v10
and v12.

Also, the tests were already failing in v12, because in v12 `process`
also became non-enumerable. I've applied a similar fix to `process` to
ensure that it's always available too.

[1]: nodejs/node#17708
  • Loading branch information
teamdandelion committed Jul 5, 2019
1 parent ae48528 commit 758db14
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 0 deletions.
4 changes: 4 additions & 0 deletions eval.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ module.exports = function (content, filename, scope, includeGlobals) {

if (includeGlobals) {
merge(sandbox, global)
// console is non-enumerable in node v10 and above
sandbox.console = global.console
// process is non-enumerable in node v12 and above
sandbox.process = global.process
sandbox.require = requireLike(_filename)
}

Expand Down
4 changes: 4 additions & 0 deletions test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,8 @@ assert.throws(function () {
_eval('require("fs")')
})

// Verify that the console is available when globals are passed
res = _eval('exports.x = console', true)
assert.deepEqual(res.x, console)

console.log('All tests passed')

0 comments on commit 758db14

Please sign in to comment.