Fix #2047: Infinite loop possible when for loop with range uses variables (#4853)

* fix #2047

* Additional check for 'step'; tests

* Fix #4105 (#4855)

* Update output

* Throw warning for unsupported runtimes, e.g. Node < 6 (#4839)

* fix #1403 (#4854)

* Update output

* [Change]: Destructuring with non-final spread should still use rest syntax (#4517) (#4825)

* destructuring optimization

* refactor

* minor improvement, fix errors

* minor refactoring

* improvements

* Update output

* Update output

* Fix #4843: bad output when assigning to @prop in destructuring assignment with defaults (#4848)

* fix #4843

* improvements

* typo

* small fix

* Fix #3441: parentheses wrapping expression throw invalid error  (#4849)

* fix #3441

* improvements

* refactor

* Fix #1726: expression in property access causes unexpected results (#4851)

* fix #1726

* Explain what's happening, rather than just linking to an issue

* Updated output

* Optimization

* Update output

* remove casting to number

* cleanup tests

lib/coffeescript/nodes.js

@@ -2031,7 +2031,7 @@
       // When compiled normally, the range returns the contents of the *for loop*
       // needed to iterate over the values in the range. Used by comprehensions.
       compileNode(o) {
-        var cond, condPart, from, gt, idx, idxName, known, lt, namedIndex, stepPart, to, varPart;
+        var cond, condPart, from, gt, idx, idxName, known, lowerBound, lt, namedIndex, stepCond, stepPart, to, upperBound, varPart;
         if (!this.fromVar) {
           this.compileVariables(o);
         }
@@ -2052,7 +2052,15 @@
         }
         [lt, gt] = [`${idx} <${this.equals}`, `${idx} >${this.equals}`];
         // Generate the condition.
-        condPart = this.stepNum != null ? this.stepNum > 0 ? `${lt} ${this.toVar}` : `${gt} ${this.toVar}` : known ? ([from, to] = [this.fromNum, this.toNum], from <= to ? `${lt} ${to}` : `${gt} ${to}`) : (cond = this.stepVar ? `${this.stepVar} > 0` : `${this.fromVar} <= ${this.toVar}`, `${cond} ? ${lt} ${this.toVar} : ${gt} ${this.toVar}`);
+        [from, to] = [this.fromNum, this.toNum];
+        // Always check if the `step` isn't zero to avoid the infinite loop.
+        stepCond = this.stepNum ? `${this.stepNum} !== 0` : `${this.stepVar} !== 0`;
+        condPart = known ? this.step == null ? from <= to ? `${lt} ${to}` : `${gt} ${        // from < to
+to}` : (lowerBound = `${from} <= ${idx} && ${lt} ${// from > to
+to}`, upperBound = `${from} >= ${idx} && ${gt} ${to}`, from <= to ? `${stepCond} && ${lowerBound}` : `${stepCond} && ${// from < to
+upperBound}`) : (lowerBound = `${this.fromVar} <= ${idx} && ${lt} ${// from > to
+this.toVar}`, upperBound = `${this.fromVar} >= ${idx} && ${gt} ${this.toVar}`, `${stepCond} && (${this.fromVar} <= ${this.toVar} ? ${lowerBound} : ${upperBound})`);
+        cond = this.stepVar ? `${this.stepVar} > 0` : `${this.fromVar} <= ${this.toVar}`;
         // Generate the step.
         stepPart = this.stepVar ? `${idx} += ${this.stepVar}` : known ? namedIndex ? from <= to ? `++${idx}` : `--${idx}` : from <= to ? `${idx}++` : `${idx}--` : namedIndex ? `${cond} ? ++${idx} : --${idx}` : `${cond} ? ${idx}++ : ${idx}--`;
         if (namedIndex) {

lib/coffeescript/rewriter.js

@@ -218,7 +218,7 @@
       indexOfTag(i, ...pattern) {
         var fuzz, j, k, ref, ref1;
         fuzz = 0;
-        for (j = k = 0, ref = pattern.length; 0 <= ref ? k < ref : k > ref; j = 0 <= ref ? ++k : --k) {
+        for (j = k = 0, ref = pattern.length; undefined !== 0 && (0 <= ref ? 0 <= k && k < ref : 0 >= k && k > ref); j = 0 <= ref ? ++k : --k) {
           if (pattern[j] == null) {
             continue;
           }

src/nodes.coffee

@@ -1359,14 +1359,27 @@ exports.Range = class Range extends Base
     [lt, gt] = ["#{idx} <#{@equals}", "#{idx} >#{@equals}"]
 
     # Generate the condition.
-    condPart = if @stepNum?
-      if @stepNum > 0 then "#{lt} #{@toVar}" else "#{gt} #{@toVar}"
-    else if known
-      [from, to] = [@fromNum, @toNum]
-      if from <= to then "#{lt} #{to}" else "#{gt} #{to}"
-    else
-      cond = if @stepVar then "#{@stepVar} > 0" else "#{@fromVar} <= #{@toVar}"
-      "#{cond} ? #{lt} #{@toVar} : #{gt} #{@toVar}"
+    [from, to] = [@fromNum, @toNum]
+    # Always check if the `step` isn't zero to avoid the infinite loop.
+    stepCond = if @stepNum then "#{@stepNum} !== 0" else "#{@stepVar} !== 0"
+    condPart =
+      if known
+        unless @step?
+          if from <= to then "#{lt} #{to}" else "#{gt} #{to}"
+        else
+          # from < to
+          lowerBound = "#{from} <= #{idx} && #{lt} #{to}"
+          # from > to
+          upperBound = "#{from} >= #{idx} && #{gt} #{to}"
+          if from <= to then "#{stepCond} && #{lowerBound}" else "#{stepCond} && #{upperBound}"
+      else
+        # from < to
+        lowerBound = "#{@fromVar} <= #{idx} && #{lt} #{@toVar}"
+        # from > to
+        upperBound = "#{@fromVar} >= #{idx} && #{gt} #{@toVar}"
+        "#{stepCond} && (#{@fromVar} <= #{@toVar} ? #{lowerBound} : #{upperBound})"
+
+    cond = if @stepVar then "#{@stepVar} > 0" else "#{@fromVar} <= #{@toVar}"
 
     # Generate the step.
     stepPart = if @stepVar

test/ranges.coffee

@@ -116,3 +116,82 @@ test "#1012 slices with arguments object", ->
 
 test "#1409: creating large ranges outside of a function body", ->
   CoffeeScript.eval '[0..100]'
+
+test "#2047: Infinite loop possible when `for` loop with `range` uses variables", ->
+  up = 1
+  down = -1
+  a = 1
+  b = 5
+
+  testRange = (arg) ->
+    [from, to, step, expectedResult] = arg
+    r = (x for x in [from..to] by step)
+    arrayEq r, expectedResult
+
+  testData = [
+    [1, 5, 1, [1..5]]
+    [1, 5, -1, [1]]
+    [1, 5, up, [1..5]]
+    [1, 5, down, [1]]
+
+    [a, 5, 1, [1..5]]
+    [a, 5, -1, [1]]
+    [a, 5, up, [1..5]]
+    [a, 5, down, [1]]
+
+    [1, b, 1, [1..5]]
+    [1, b, -1, [1]]
+    [1, b, up, [1..5]]
+    [1, b, down, [1]]
+
+    [a, b, 1, [1..5]]
+    [a, b, -1, [1]]
+    [a, b, up, [1..5]]
+    [a, b, down, [1]]
+
+    [5, 1, 1, [5]]
+    [5, 1, -1, [5..1]]
+    [5, 1, up, [5]]
+    [5, 1, down,  [5..1]]
+
+    [5, a, 1, [5]]
+    [5, a, -1, [5..1]]
+    [5, a, up, [5]]
+    [5, a, down, [5..1]]
+
+    [b, 1, 1, [5]]
+    [b, 1, -1, [5..1]]
+    [b, 1, up, [5]]
+    [b, 1, down, [5..1]]
+
+    [b, a, 1, [5]]
+    [b, a, -1, [5..1]]
+    [b, a, up, [5]]
+    [b, a, down, [5..1]]
+  ]
+
+  testRange d for d in testData
+
+test "#2047: from, to and step as variables", ->
+  up = 1
+  down = -1
+  a = 1
+  b = 5
+
+  r = (x for x in [a..b] by up)
+  arrayEq r, [1..5]
+
+  r = (x for x in [a..b] by down)
+  arrayEq r, [1]
+
+  r = (x for x in [b..a] by up)
+  arrayEq r, [5]
+
+  r = (x for x in [b..a] by down)
+  arrayEq r, [5..1]
+
+  a = 1
+  b = -1
+  step = 0
+  r = (x for x in [b..a] by step)
+  arrayEq r, []