From ed1ba4580b4abdc38047f4f74ab79ba0b71f3da4 Mon Sep 17 00:00:00 2001
From: Lance Ball <lball@redhat.com>
Date: Fri, 14 Jul 2017 23:23:02 -0400
Subject: [PATCH] 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>
---
 lib/repl.js                        | 18 ++++++--------
 test/parallel/test-repl-context.js | 40 ++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/lib/repl.js b/lib/repl.js
index f745283097..6fc984fbcf 100644
--- a/lib/repl.js
+++ b/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
diff --git a/test/parallel/test-repl-context.js b/test/parallel/test-repl-context.js
index a2f0421431..9d18067bc2 100644
--- a/test/parallel/test-repl-context.js
+++ b/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();
 }