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: nodejs#14226 Refs: nodejs#7619 PR-URL: nodejs#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>

Author: lance

lib/repl.js

@@ -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) => {
@@ -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);
 };
@@ -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

test/parallel/test-repl-context.js

@@ -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();
@@ -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();
 }