From f954aba39ee02193beb2dbddc6c0ea0edcca7c74 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 14 May 2018 14:51:50 +0200 Subject: [PATCH] repl: make console, module and require non-enumerable This aligns these globals with the regular context. PR-URL: https://github.com/nodejs/node/pull/20717 Reviewed-By: James M Snell Reviewed-By: Matteo Collina --- lib/repl.js | 15 +++++-- test/parallel/test-repl-console.js | 44 ------------------- test/parallel/test-repl-context.js | 39 +++++++++------- ...test-repl-function-definition-edge-case.js | 4 +- test/parallel/test-repl-let-process.js | 2 - test/parallel/test-repl-mode.js | 4 +- test/parallel/test-repl-options.js | 2 - test/parallel/test-repl-reset-event.js | 1 - 8 files changed, 36 insertions(+), 75 deletions(-) delete mode 100644 test/parallel/test-repl-console.js diff --git a/lib/repl.js b/lib/repl.js index 928e7c6d69cfae..9935d60fa73e03 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -793,7 +793,7 @@ REPLServer.prototype.createContext = function() { const _console = new Console(this.outputStream); Object.defineProperty(context, 'console', { configurable: true, - enumerable: true, + writable: true, value: _console }); @@ -813,9 +813,16 @@ REPLServer.prototype.createContext = function() { module.paths = CJSModule._resolveLookupPaths('', parentModule, true) || []; - var require = makeRequireFunction(module); - context.module = module; - context.require = require; + Object.defineProperty(context, 'module', { + configurable: true, + writable: true, + value: module + }); + Object.defineProperty(context, 'require', { + configurable: true, + writable: true, + value: makeRequireFunction(module) + }); addBuiltinLibsToObject(context); diff --git a/test/parallel/test-repl-console.js b/test/parallel/test-repl-console.js deleted file mode 100644 index 94547e4768bb76..00000000000000 --- a/test/parallel/test-repl-console.js +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const repl = require('repl'); - -// Create a dummy stream that does nothing -const stream = new common.ArrayStream(); - -const r = repl.start({ - input: stream, - output: stream, - useGlobal: false -}); - - -// ensure that the repl context gets its own "console" instance -assert(r.context.console); - -// ensure that the repl console instance is not the global one -assert.notStrictEqual(r.context.console, console); - -// ensure that the repl console instance does not have a setter -assert.throws(() => r.context.console = 'foo', TypeError); diff --git a/test/parallel/test-repl-context.js b/test/parallel/test-repl-context.js index 9d18067bc2aca4..914aa563bd50fb 100644 --- a/test/parallel/test-repl-context.js +++ b/test/parallel/test-repl-context.js @@ -4,32 +4,39 @@ const assert = require('assert'); const repl = require('repl'); const vm = require('vm'); -// Create a dummy stream that does nothing +// Create a dummy stream that does nothing. const stream = new common.ArrayStream(); -// Test when useGlobal is false -testContext(repl.start({ - input: stream, - output: stream, - useGlobal: false -})); +// Test context when useGlobal is false. +{ + const r = repl.start({ + input: stream, + output: stream, + useGlobal: false + }); -function testContext(repl) { - const context = repl.createContext(); - // ensure that the repl context gets its own "console" instance + // Ensure that the repl context gets its own "console" instance. + assert(r.context.console); + + // Ensure that the repl console instance is not the global one. + assert.notStrictEqual(r.context.console, console); + + const context = r.createContext(); + // Ensure that the repl context gets its own "console" instance. assert(context.console instanceof require('console').Console); - // ensure that the repl's global property is the context + // Ensure that the repl's global property is the context. assert.strictEqual(context.global, context); - // ensure that the repl console instance does not have a setter - assert.throws(() => context.console = 'foo', TypeError); - repl.close(); + // Ensure that the repl console instance is writable. + context.console = 'foo'; + r.close(); } -testContextSideEffects(repl.start({ input: stream, output: stream })); +// Test for context side effects. +{ + const server = repl.start({ input: stream, output: stream }); -function testContextSideEffects(server) { assert.ok(!server.underscoreAssigned); assert.strictEqual(server.lines.length, 0); diff --git a/test/parallel/test-repl-function-definition-edge-case.js b/test/parallel/test-repl-function-definition-edge-case.js index 1e3063e3db53ff..952fba4103cc26 100644 --- a/test/parallel/test-repl-function-definition-edge-case.js +++ b/test/parallel/test-repl-function-definition-edge-case.js @@ -1,12 +1,10 @@ // Reference: https://github.com/nodejs/node/pull/7624 'use strict'; -const common = require('../common'); +require('../common'); const assert = require('assert'); const repl = require('repl'); const stream = require('stream'); -common.globalCheck = false; - const r = initRepl(); r.input.emit('data', 'function a() { return 42; } (1)\n'); diff --git a/test/parallel/test-repl-let-process.js b/test/parallel/test-repl-let-process.js index 3e6c3e85665be1..dd8fa60f463d8b 100644 --- a/test/parallel/test-repl-let-process.js +++ b/test/parallel/test-repl-let-process.js @@ -2,8 +2,6 @@ const common = require('../common'); const repl = require('repl'); -common.globalCheck = false; - // Regression test for https://github.com/nodejs/node/issues/6802 const input = new common.ArrayStream(); repl.start({ input, output: process.stdout, useGlobal: true }); diff --git a/test/parallel/test-repl-mode.js b/test/parallel/test-repl-mode.js index 60b430d8c7ee31..df84d86a3c8dc3 100644 --- a/test/parallel/test-repl-mode.js +++ b/test/parallel/test-repl-mode.js @@ -1,11 +1,9 @@ 'use strict'; -const common = require('../common'); +require('../common'); const assert = require('assert'); const Stream = require('stream'); const repl = require('repl'); -common.globalCheck = false; - const tests = [ testSloppyMode, testStrictMode, diff --git a/test/parallel/test-repl-options.js b/test/parallel/test-repl-options.js index 6bd908aadf935d..1de49c8e861391 100644 --- a/test/parallel/test-repl-options.js +++ b/test/parallel/test-repl-options.js @@ -24,8 +24,6 @@ const common = require('../common'); const assert = require('assert'); const repl = require('repl'); -common.globalCheck = false; - // Create a dummy stream that does nothing const stream = new common.ArrayStream(); diff --git a/test/parallel/test-repl-reset-event.js b/test/parallel/test-repl-reset-event.js index 83ab7fd6a3ec52..4b3a04aeeb88c5 100644 --- a/test/parallel/test-repl-reset-event.js +++ b/test/parallel/test-repl-reset-event.js @@ -21,7 +21,6 @@ 'use strict'; const common = require('../common'); -common.globalCheck = false; const assert = require('assert'); const repl = require('repl');