Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

repl: repl clobbers RegExp.$1 #597

Closed
bnoordhuis opened this issue Jan 24, 2015 · 8 comments
Closed

repl: repl clobbers RegExp.$1 #597

bnoordhuis opened this issue Jan 24, 2015 · 8 comments
Labels
confirmed-bug Issues with confirmed bugs. good first issue Issues that are suitable for first-time contributors. repl Issues and PRs related to the REPL subsystem.

Comments

@bnoordhuis
Copy link
Member

$ out/Release/iojs
> RegExp.$1
'RegExp.$1'
> eval(RegExp.$1)
# snip
    at <error: RangeError: Maximum call stack size exceeded>
@bnoordhuis bnoordhuis added repl Issues and PRs related to the REPL subsystem. confirmed-bug Issues with confirmed bugs. labels Jan 24, 2015
@vkurchatkin
Copy link
Contributor

is this the same issue: nodejs/node-v0.x-archive#4088 ?

@bnoordhuis
Copy link
Member Author

Yes, it is. I'd forgotten about that other issue, actually.

@vkurchatkin
Copy link
Contributor

As an idea: we can execute regexps in a different context

@chrisdickinson chrisdickinson added the good first issue Issues that are suitable for first-time contributors. label Jul 8, 2015
@chrisdickinson
Copy link
Contributor

Given that we have a patch, this would make a good first bug for anyone with the time to PR it in.

@thefourtheye
Copy link
Contributor

I was looking at it the other day, I was able to reproduce the problem but couldn't understand the fix suggested. I ll spend sometime this week to understand it.

@bnoordhuis
Copy link
Member Author

Reproducing here for posterity:

diff --git a/lib/repl.js b/lib/repl.js
index 361745f..fe3077c 100644
--- a/lib/repl.js
+++ b/lib/repl.js
@@ -104,9 +104,14 @@ function REPLServer(prompt, stream, eval_, useGlobal, ignoreUndefined) {

   self.useGlobal = !!useGlobal;
   self.ignoreUndefined = !!ignoreUndefined;
+  self._saved = [];

   self.eval = eval_ || function(code, context, file, cb) {
     var err, result;
+
+    // restore RegExp.$1 to RegExp.$9
+    /^\u0000(.*)\u0000(.*)\u0000(.*)\u0000(.*)\u0000(.*)\u0000(.*)\u0000(.*)\u0000(.*)\u0000(.*)$/.test(self._saved.join('\u0000'));
+
     try {
       if (self.useGlobal) {
         result = vm.runInThisContext(code, file);
@@ -116,6 +121,11 @@ function REPLServer(prompt, stream, eval_, useGlobal, ignoreUndefined) {
     } catch (e) {
       err = e;
     }
+
+    for (var i = 1; i <= 9; ++i) {
+      self._saved[i] = RegExp['$' + i];
+    }
+
     cb(err, result);
   };

@thefourtheye It works by creating a string from the 9 saved values, then doing a capturing match against that, thereby restoring the values of RegExp.$1 to RegExp.$9.

Aside: using '\u0000' as the separator is probably not unique enough in practice.

@thefourtheye
Copy link
Contributor

@bnoordhuis Thanks for helping me understand :-)

thefourtheye added a commit to thefourtheye/io.js that referenced this issue Jul 30, 2015
Fixes: nodejs#597

In REPL, if we evaluate the `RegExp` object's predefined properties,
and if they happen to have the same expression, for example,

    > RegExp.$1
    'RegExp.$1'

then doing `eval(RegExp.$1)` would evluated `RegExp.$1` recursively and
eventually throw `RangeError: Maximum call stack size exceeded`.

This patch stores the old values of `RegExp`'s predefined proprties and
restores them just before the current expression entered by user is
evaluated.
thefourtheye added a commit that referenced this issue Aug 4, 2015
In REPL, if we evaluate the `RegExp` object's predefined properties,
and if they happen to have the same expression, for example,

    > RegExp.$1
    'RegExp.$1'

then doing `eval(RegExp.$1)` would evaluate `RegExp.$1` recursively and
eventually throw `RangeError: Maximum call stack size exceeded`.

This patch stores the old values of `RegExp`'s predefined proprties in
an array and restores them just before the current expression entered
by user is evaluated.

Fixes: #597
PR-URL: #2137
Reviewed-By: Ben Noordhuis <[email protected]>
@thefourtheye
Copy link
Contributor

In light of #2137's landing, closing this bug.

Fishrock123 pushed a commit to Fishrock123/node that referenced this issue Aug 4, 2015
In REPL, if we evaluate the `RegExp` object's predefined properties,
and if they happen to have the same expression, for example,

    > RegExp.$1
    'RegExp.$1'

then doing `eval(RegExp.$1)` would evaluate `RegExp.$1` recursively and
eventually throw `RangeError: Maximum call stack size exceeded`.

This patch stores the old values of `RegExp`'s predefined proprties in
an array and restores them just before the current expression entered
by user is evaluated.

Fixes: nodejs#597
PR-URL: nodejs#2137
Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. good first issue Issues that are suitable for first-time contributors. repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

No branches or pull requests

4 participants