NeilFraser / JS-Interpreter

A sandboxed JavaScript interpreter in JavaScript.
Apache License 2.0
1.96k stars 352 forks source link

Tasks created through createTask_ (such as when using setTimeout) are incorrectly interpreted as throwing their return values. #270

Closed brownstein closed 3 months ago

brownstein commented 3 months ago

When a task is added through createTask_, it effectively treats the root Program node as the parent state. This is ordinarily not a bad thing, but there's a bug on https://github.com/NeilFraser/JS-Interpreter/blob/master/interpreter.js#L3289 that breaks and falls back to the error handling code instead of returning, resulting in the value being thrown.

This issue tracks the need to change the break on https://github.com/NeilFraser/JS-Interpreter/blob/master/interpreter.js#L3289 to a return to resolve this issue.

To replicate, try interpreting:

setTimeout(function () {
  return 1;
}, 1);
brownstein commented 3 months ago

Slight correction - we need to check if type === Interpreter.Completion.RETURN for the early return here.

NeilFraser commented 3 months ago

Very nice bug. Thank you. It has been fixed exactly as you described.

That said, I have this gut feeling that there may be a larger issue going on here -- but I haven't been able to find any further bugs. For example, I've tried breaking to a label outside the task, but Acorn catches that. Maybe this bug is narrow after all.

Hey @cpcallen, you are good at breaking things...