Skip to content

Commit

Permalink
repl: remove REPLServer.createContext side effects
Browse files Browse the repository at this point in the history
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: #14226
Refs: #7619

PR-URL: #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>
  • Loading branch information
lance committed Aug 30, 2017
1 parent 676522f commit ed1ba45
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 10 deletions.
18 changes: 8 additions & 10 deletions lib/repl.js
Expand Up @@ -644,14 +644,18 @@ REPLServer.prototype.createContext = function() {
context.module = module;
context.require = require;

internalModule.addBuiltinLibsToObject(context);

return context;
};

REPLServer.prototype.resetContext = function() {
this.context = this.createContext();
this.underscoreAssigned = false;
this.lines = [];
this.lines.level = [];

internalModule.addBuiltinLibsToObject(context);

Object.defineProperty(context, '_', {
Object.defineProperty(this.context, '_', {
configurable: true,
get: () => this.last,
set: (value) => {
Expand All @@ -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
this.emit('reset', this.context);
};
Expand Down Expand Up @@ -784,7 +782,7 @@ function complete(line, callback) {
var flat = new ArrayStream(); // make a new "input" stream
var magic = new REPLServer('', flat); // make a nested REPL
replMap.set(magic, replMap.get(this));
magic.context = magic.createContext();
magic.resetContext();
flat.run(tmp); // eval the flattened code
// all this is only profitable if the nested REPL
// does not have a bufferedCommand
Expand Down
40 changes: 40 additions & 0 deletions test/parallel/test-repl-context.js
Expand Up @@ -2,6 +2,7 @@
const common = require('../common');
const assert = require('assert');
const repl = require('repl');
const vm = require('vm');

// Create a dummy stream that does nothing
const stream = new common.ArrayStream();
Expand All @@ -23,4 +24,43 @@ function testContext(repl) {

// ensure that the repl console instance does not have a setter
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();
}

0 comments on commit ed1ba45

Please sign in to comment.