Skip to content

Commit 9e80f6f

Browse files
zdenkoGeoffreyBooth
authored andcommitted
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
1 parent fc5ab1a commit 9e80f6f

File tree

4 files changed

+111
-11
lines changed

4 files changed

+111
-11
lines changed

lib/coffeescript/nodes.js

Lines changed: 10 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/coffeescript/rewriter.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/nodes.coffee

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1359,14 +1359,27 @@ exports.Range = class Range extends Base
13591359
[lt, gt] = ["#{idx} <#{@equals}", "#{idx} >#{@equals}"]
13601360

13611361
# Generate the condition.
1362-
condPart = if @stepNum?
1363-
if @stepNum > 0 then "#{lt} #{@toVar}" else "#{gt} #{@toVar}"
1364-
else if known
1365-
[from, to] = [@fromNum, @toNum]
1366-
if from <= to then "#{lt} #{to}" else "#{gt} #{to}"
1367-
else
1368-
cond = if @stepVar then "#{@stepVar} > 0" else "#{@fromVar} <= #{@toVar}"
1369-
"#{cond} ? #{lt} #{@toVar} : #{gt} #{@toVar}"
1362+
[from, to] = [@fromNum, @toNum]
1363+
# Always check if the `step` isn't zero to avoid the infinite loop.
1364+
stepCond = if @stepNum then "#{@stepNum} !== 0" else "#{@stepVar} !== 0"
1365+
condPart =
1366+
if known
1367+
unless @step?
1368+
if from <= to then "#{lt} #{to}" else "#{gt} #{to}"
1369+
else
1370+
# from < to
1371+
lowerBound = "#{from} <= #{idx} && #{lt} #{to}"
1372+
# from > to
1373+
upperBound = "#{from} >= #{idx} && #{gt} #{to}"
1374+
if from <= to then "#{stepCond} && #{lowerBound}" else "#{stepCond} && #{upperBound}"
1375+
else
1376+
# from < to
1377+
lowerBound = "#{@fromVar} <= #{idx} && #{lt} #{@toVar}"
1378+
# from > to
1379+
upperBound = "#{@fromVar} >= #{idx} && #{gt} #{@toVar}"
1380+
"#{stepCond} && (#{@fromVar} <= #{@toVar} ? #{lowerBound} : #{upperBound})"
1381+
1382+
cond = if @stepVar then "#{@stepVar} > 0" else "#{@fromVar} <= #{@toVar}"
13701383

13711384
# Generate the step.
13721385
stepPart = if @stepVar

test/ranges.coffee

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,3 +116,82 @@ test "#1012 slices with arguments object", ->
116116

117117
test "#1409: creating large ranges outside of a function body", ->
118118
CoffeeScript.eval '[0..100]'
119+
120+
test "#2047: Infinite loop possible when `for` loop with `range` uses variables", ->
121+
up = 1
122+
down = -1
123+
a = 1
124+
b = 5
125+
126+
testRange = (arg) ->
127+
[from, to, step, expectedResult] = arg
128+
r = (x for x in [from..to] by step)
129+
arrayEq r, expectedResult
130+
131+
testData = [
132+
[1, 5, 1, [1..5]]
133+
[1, 5, -1, [1]]
134+
[1, 5, up, [1..5]]
135+
[1, 5, down, [1]]
136+
137+
[a, 5, 1, [1..5]]
138+
[a, 5, -1, [1]]
139+
[a, 5, up, [1..5]]
140+
[a, 5, down, [1]]
141+
142+
[1, b, 1, [1..5]]
143+
[1, b, -1, [1]]
144+
[1, b, up, [1..5]]
145+
[1, b, down, [1]]
146+
147+
[a, b, 1, [1..5]]
148+
[a, b, -1, [1]]
149+
[a, b, up, [1..5]]
150+
[a, b, down, [1]]
151+
152+
[5, 1, 1, [5]]
153+
[5, 1, -1, [5..1]]
154+
[5, 1, up, [5]]
155+
[5, 1, down, [5..1]]
156+
157+
[5, a, 1, [5]]
158+
[5, a, -1, [5..1]]
159+
[5, a, up, [5]]
160+
[5, a, down, [5..1]]
161+
162+
[b, 1, 1, [5]]
163+
[b, 1, -1, [5..1]]
164+
[b, 1, up, [5]]
165+
[b, 1, down, [5..1]]
166+
167+
[b, a, 1, [5]]
168+
[b, a, -1, [5..1]]
169+
[b, a, up, [5]]
170+
[b, a, down, [5..1]]
171+
]
172+
173+
testRange d for d in testData
174+
175+
test "#2047: from, to and step as variables", ->
176+
up = 1
177+
down = -1
178+
a = 1
179+
b = 5
180+
181+
r = (x for x in [a..b] by up)
182+
arrayEq r, [1..5]
183+
184+
r = (x for x in [a..b] by down)
185+
arrayEq r, [1]
186+
187+
r = (x for x in [b..a] by up)
188+
arrayEq r, [5]
189+
190+
r = (x for x in [b..a] by down)
191+
arrayEq r, [5..1]
192+
193+
a = 1
194+
b = -1
195+
step = 0
196+
r = (x for x in [b..a] by step)
197+
arrayEq r, []

0 commit comments

Comments
 (0)