mirror of
https://github.com/morluto/rea.git
synced 2026-11-10 16:57:19 +00:00
fix(javascript): emit no timer for a literal non-function callback (#8044)
* fix(javascript): emit no timer for a literal non-function callback
Node validates the timer callback before creating a handle:
if (typeof callback !== 'function')
throw new ERR_INVALID_ARG_TYPE('callback', 'function', callback);
The throw is synchronous, before any timer exists, but the analysis reported a
`schedule` operation for such calls.
Measured on Node 24.22.1:
setTimeout("code", 0) -> THROWS ERR_INVALID_ARG_TYPE
setTimeout(`code`, 0) -> THROWS ERR_INVALID_ARG_TYPE
setTimeout(1, 0) -> THROWS ERR_INVALID_ARG_TYPE
setTimeout(1n, 0) -> THROWS ERR_INVALID_ARG_TYPE
setTimeout(true, 0) -> THROWS ERR_INVALID_ARG_TYPE
setTimeout(null, 0) -> THROWS ERR_INVALID_ARG_TYPE
setTimeout(void 0, 0) -> THROWS ERR_INVALID_ARG_TYPE
setTimeout([], 0) -> THROWS ERR_INVALID_ARG_TYPE
setTimeout({}, 0) -> THROWS ERR_INVALID_ARG_TYPE
setTimeout() -> THROWS ERR_INVALID_ARG_TYPE
setImmediate("code") -> THROWS ERR_INVALID_ARG_TYPE
setInterval("code", 0) -> THROWS ERR_INVALID_ARG_TYPE
--- accepted by Node ---
setTimeout(function(){}, 0) -> OK
setTimeout(() => {}, 0) -> OK
setTimeout(fn, 0) -> OK
setTimeout(class {}, 0) -> OK (fails only when the class is invoked)
So a schedule is dropped only for an exactly readable literal that is not a
function, or for a missing argument. An identifier, a call result or a spread may
still be a function at runtime and keeps its schedule.
* chore: retrigger title and readiness checks after the title was corrected
* fix(javascript): scope the timer callback rule to Node's timers module
greptile flagged two problems with the first version.
P1 - browser timers disappeared. The rule was applied to bare globals, but
browser and Electron renderer sources reach this analysis without a Node/browser
distinction, and there `setTimeout("refresh()", 0)` is a real timer: the host
evaluates the string. The rule now applies only when the timer is imported from
`node:timers`, where that form cannot exist. A bare global schedule keeps its
fact whatever argument 0 holds.
P2 - unused import. `readExactJavaScriptLiteral` was imported but never used; the
helper reads shapes through `unwrapJavaScriptExpression`. Removed.
Measured on Node 24.22.1 (the throw is what makes a node:timers schedule
impossible; the bare-global case is intentionally left alone):
import { setTimeout } from "node:timers";
setTimeout("code", 0) -> THROWS ERR_INVALID_ARG_TYPE
setTimeout(null, 0) -> THROWS ERR_INVALID_ARG_TYPE
setTimeout(fn, 0) -> OK
--- bare global, host may be a browser ---
setTimeout("code", 0) -> kept as a schedule
24 cases cover both sides plus the cancellation link.
* fix(javascript): reject every non-function global and unary timer callback
Reviewing the timer callback guard against Node 24.22.1 found two gaps. Both are
in the same "a call that always throws must not emit a resolved fact" family.
**An unmodified global that is not a function was accepted.** The guard read
literal nodes only, so a bare global identifier passed through even though Node
rejects it before creating a handle:
setTimeout(undefined) -> THREW ERR_INVALID_ARG_TYPE
setTimeout(NaN) -> THREW ERR_INVALID_ARG_TYPE
setTimeout(Infinity) -> THREW ERR_INVALID_ARG_TYPE
`isUnmodifiedGlobalReference` keeps a shadowed binding out of the rule, so a
local `const NaN = () => {}` keeps its schedule.
**Only `void` was treated as a non-function unary operator.** Every unary operator
yields a primitive, and Node rejects a primitive callback:
setTimeout(!0) -> THREW ERR_INVALID_ARG_TYPE
setTimeout(~0) -> THREW ERR_INVALID_ARG_TYPE
setTimeout(-1) -> THREW ERR_INVALID_ARG_TYPE
setTimeout(+"1") -> THREW ERR_INVALID_ARG_TYPE
setTimeout(typeof 1) -> THREW ERR_INVALID_ARG_TYPE
A class expression is still kept, because it passes Node's own `typeof` check and
only fails when invoked.
Fifteen regressions cover the rejected forms plus arrow, identifier, class,
spread and shadowed-global cases; ten fail on the previous commit.
* fix(javascript): let sloppy eval shadow the non-function global callback
Review finding. The new global check read `undefined`, `NaN` and `Infinity` as
unmodified globals, but sloppy `eval` can introduce a local function under one of
those names, which then shadows the global. Measured on Node 24.22.1 in script
scope:
function g() { eval("var undefined = () => {}"); setTimeout(undefined, 0); }
-> schedules a timer
function g() { eval("var NaN = () => {}"); setTimeout(NaN, 0); }
-> schedules a timer
`semanticOpaqueNameMayHaveChanged` now has to agree that no visible activation can
introduce such a name before the global reading is conclusive. In a module the
same `eval` is strict and introduces no outer binding, so the global reading still
holds and the timer is still dropped.
Three regressions cover the sloppy forms, and a fourth pins the no-eval cases as
still dropped. The three sloppy cases fail without the guard.
This commit is contained in:
1 parent
848f165417
commit
54d819f382
2 files changed
+232
No files matched your search
@@ -21,6 +21,7 @@ import {
|
||||
import { semanticBindingAtRead } from "./javascriptSemanticMutationInitializers.js";
|
||||
import { traverseJavaScriptValueAst } from "./javascriptSemanticTraversal.js";
|
||||
import { isUnmodifiedGlobalReference } from "./javascriptSemanticAmbientWrites.js";
|
||||
import { semanticOpaqueNameMayHaveChanged } from "./javascriptSemanticOpaqueEffects.js";
|
||||
import { range } from "./javascriptStaticAnalysisHelpers.js";
|
||||
import {
|
||||
builtinDataEffectMethod,
|
||||
@@ -286,10 +287,79 @@ const emitterIdentity = (
|
||||
};
|
||||
};
|
||||
|
||||
/**
|
||||
* A timer proven to come from the Node `timers` module rather than a bare
|
||||
* global. Browser and Electron renderer sources reach this analysis without a
|
||||
* Node/browser distinction, and there `setTimeout("refresh()", 0)` is a real
|
||||
* timer: the string is evaluated by the host. Node's callback rule therefore
|
||||
* applies only when the timer is imported from `node:timers`, where that form
|
||||
* cannot exist.
|
||||
*/
|
||||
const nodeBuiltinTimerMethod = (
|
||||
callee: t.Node,
|
||||
state: JavaScriptSemanticAnalysisState,
|
||||
): JavaScriptSemanticTimerOperation["method"] | null =>
|
||||
admittedTimerMethod(
|
||||
builtinDataEffectMethod(callee, state, ["timers", "node:timers"]) ?? "",
|
||||
);
|
||||
|
||||
/**
|
||||
* A schedule imported from Node's `timers` module requires a function in
|
||||
* argument 0. Node validates it before creating a handle, so a provable literal
|
||||
* that is not a function throws ERR_INVALID_ARG_TYPE synchronously and no timer
|
||||
* exists. Only literals the analyzer can read exactly are decided; an identifier
|
||||
* or call may still be a function at runtime.
|
||||
*/
|
||||
const schedulesWithNonFunctionCallback = (
|
||||
node: t.Node,
|
||||
state: JavaScriptSemanticAnalysisState,
|
||||
): boolean => {
|
||||
if (!t.isCallExpression(node) && !t.isOptionalCallExpression(node))
|
||||
return false;
|
||||
const argument = node.arguments[0];
|
||||
// No argument at all cannot be a function. A spread may still expand to one.
|
||||
if (argument === undefined) return true;
|
||||
if (t.isSpreadElement(argument)) return false;
|
||||
const value = unwrapJavaScriptExpression(argument).node;
|
||||
if (t.isFunctionExpression(value) || t.isArrowFunctionExpression(value))
|
||||
return false;
|
||||
if (
|
||||
t.isStringLiteral(value) ||
|
||||
t.isNumericLiteral(value) ||
|
||||
t.isBigIntLiteral(value) ||
|
||||
t.isBooleanLiteral(value) ||
|
||||
t.isNullLiteral(value) ||
|
||||
t.isTemplateLiteral(value) ||
|
||||
t.isArrayExpression(value) ||
|
||||
t.isObjectExpression(value)
|
||||
)
|
||||
return true;
|
||||
// `undefined`, `NaN` and `Infinity` are unmodified globals, and none is a
|
||||
// function. A shadowed binding may still hold one, so only the global counts.
|
||||
if (
|
||||
t.isIdentifier(value) &&
|
||||
["undefined", "NaN", "Infinity"].includes(value.name) &&
|
||||
isUnmodifiedGlobalReference(value, state, value.name) &&
|
||||
!semanticOpaqueNameMayHaveChanged(value, state)
|
||||
)
|
||||
return true;
|
||||
// A class passes Node's own typeof check; invoking it is a separate failure,
|
||||
// so the schedule itself still happens. Every other unary operator (including
|
||||
// `void`, `!`, `~`, `typeof` and `delete`) yields a primitive, never a
|
||||
// function.
|
||||
return t.isUnaryExpression(value);
|
||||
};
|
||||
|
||||
const collectTimer = (input: TimerCollectionInput): void => {
|
||||
const { node, ownerCallableId, state, output, ancestors } = input;
|
||||
const method = timerMethod(node.callee, state);
|
||||
if (method === null) return;
|
||||
if (
|
||||
TIMER_SCHEDULE_METHODS.some((candidate) => candidate === method) &&
|
||||
nodeBuiltinTimerMethod(node.callee, state) !== null &&
|
||||
schedulesWithNonFunctionCallback(node, input.state)
|
||||
)
|
||||
return;
|
||||
const kind = TIMER_SCHEDULE_METHODS.some((candidate) => candidate === method)
|
||||
? "schedule"
|
||||
: "cancel";
|
||||
|
||||
Reference in new issue
Block a user