repl: remove REPLServer.createContext side effects
The internal method `REPLServer.createContext()` had unexpected side effects. When called, the value for the `underscoreAssigned` and `lines` properties were reset. This change ensures that those properties are not modified when a context is created. Fixes: https://github.com/nodejs/node/issues/14226 Refs: https://github.com/nodejs/node/issues/7619 PR-URL: https://github.com/nodejs/node/pull/14331 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This commit is contained in:
parent
676522fc02
commit
ed1ba4580b
18
lib/repl.js
18
lib/repl.js
@ -644,14 +644,18 @@ REPLServer.prototype.createContext = function() {
|
|||||||
context.module = module;
|
context.module = module;
|
||||||
context.require = require;
|
context.require = require;
|
||||||
|
|
||||||
|
internalModule.addBuiltinLibsToObject(context);
|
||||||
|
|
||||||
|
return context;
|
||||||
|
};
|
||||||
|
|
||||||
|
REPLServer.prototype.resetContext = function() {
|
||||||
|
this.context = this.createContext();
|
||||||
this.underscoreAssigned = false;
|
this.underscoreAssigned = false;
|
||||||
this.lines = [];
|
this.lines = [];
|
||||||
this.lines.level = [];
|
this.lines.level = [];
|
||||||
|
|
||||||
internalModule.addBuiltinLibsToObject(context);
|
Object.defineProperty(this.context, '_', {
|
||||||
|
|
||||||
Object.defineProperty(context, '_', {
|
|
||||||
configurable: true,
|
configurable: true,
|
||||||
get: () => this.last,
|
get: () => this.last,
|
||||||
set: (value) => {
|
set: (value) => {
|
||||||
@ -663,12 +667,6 @@ REPLServer.prototype.createContext = function() {
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
return context;
|
|
||||||
};
|
|
||||||
|
|
||||||
REPLServer.prototype.resetContext = function() {
|
|
||||||
this.context = this.createContext();
|
|
||||||
|
|
||||||
// Allow REPL extensions to extend the new context
|
// Allow REPL extensions to extend the new context
|
||||||
this.emit('reset', this.context);
|
this.emit('reset', this.context);
|
||||||
};
|
};
|
||||||
@ -784,7 +782,7 @@ function complete(line, callback) {
|
|||||||
var flat = new ArrayStream(); // make a new "input" stream
|
var flat = new ArrayStream(); // make a new "input" stream
|
||||||
var magic = new REPLServer('', flat); // make a nested REPL
|
var magic = new REPLServer('', flat); // make a nested REPL
|
||||||
replMap.set(magic, replMap.get(this));
|
replMap.set(magic, replMap.get(this));
|
||||||
magic.context = magic.createContext();
|
magic.resetContext();
|
||||||
flat.run(tmp); // eval the flattened code
|
flat.run(tmp); // eval the flattened code
|
||||||
// all this is only profitable if the nested REPL
|
// all this is only profitable if the nested REPL
|
||||||
// does not have a bufferedCommand
|
// does not have a bufferedCommand
|
||||||
|
@ -2,6 +2,7 @@
|
|||||||
const common = require('../common');
|
const common = require('../common');
|
||||||
const assert = require('assert');
|
const assert = require('assert');
|
||||||
const repl = require('repl');
|
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();
|
const stream = new common.ArrayStream();
|
||||||
@ -23,4 +24,43 @@ function testContext(repl) {
|
|||||||
|
|
||||||
// ensure that the repl console instance does not have a setter
|
// ensure that the repl console instance does not have a setter
|
||||||
assert.throws(() => context.console = 'foo', TypeError);
|
assert.throws(() => context.console = 'foo', TypeError);
|
||||||
|
repl.close();
|
||||||
|
}
|
||||||
|
|
||||||
|
testContextSideEffects(repl.start({ input: stream, output: stream }));
|
||||||
|
|
||||||
|
function testContextSideEffects(server) {
|
||||||
|
assert.ok(!server.underscoreAssigned);
|
||||||
|
assert.strictEqual(server.lines.length, 0);
|
||||||
|
|
||||||
|
// an assignment to '_' in the repl server
|
||||||
|
server.write('_ = 500;\n');
|
||||||
|
assert.ok(server.underscoreAssigned);
|
||||||
|
assert.strictEqual(server.lines.length, 1);
|
||||||
|
assert.strictEqual(server.lines[0], '_ = 500;');
|
||||||
|
assert.strictEqual(server.last, 500);
|
||||||
|
|
||||||
|
// use the server to create a new context
|
||||||
|
const context = server.createContext();
|
||||||
|
|
||||||
|
// ensure that creating a new context does not
|
||||||
|
// have side effects on the server
|
||||||
|
assert.ok(server.underscoreAssigned);
|
||||||
|
assert.strictEqual(server.lines.length, 1);
|
||||||
|
assert.strictEqual(server.lines[0], '_ = 500;');
|
||||||
|
assert.strictEqual(server.last, 500);
|
||||||
|
|
||||||
|
// reset the server context
|
||||||
|
server.resetContext();
|
||||||
|
assert.ok(!server.underscoreAssigned);
|
||||||
|
assert.strictEqual(server.lines.length, 0);
|
||||||
|
|
||||||
|
// ensure that assigning to '_' in the new context
|
||||||
|
// does not change the value in our server.
|
||||||
|
assert.ok(!server.underscoreAssigned);
|
||||||
|
vm.runInContext('_ = 1000;\n', context);
|
||||||
|
|
||||||
|
assert.ok(!server.underscoreAssigned);
|
||||||
|
assert.strictEqual(server.lines.length, 0);
|
||||||
|
server.close();
|
||||||
}
|
}
|
||||||
|
Loading…
x
Reference in New Issue
Block a user