fix(javascript): emit no event operation for a literal non-function listener (#8621)

* fix(javascript): emit no event operation for a literal non-function listener

`EventEmitter` validates a listener before registering or removing anything:

    if (typeof listener !== 'function')
      throw new ERR_INVALID_ARG_TYPE('listener', 'function', listener);

The throw is synchronous, before any listener exists, but the analysis reported a
`register` or `remove` operation for those calls.

Measured on Node 24.22.1:

    emitter.on(1)                      -> THREW ERR_INVALID_ARG_TYPE
    emitter.on("x", 1)                 -> THREW ERR_INVALID_ARG_TYPE
    emitter.on("x", null)              -> THREW ERR_INVALID_ARG_TYPE
    emitter.on("x", true)              -> THREW ERR_INVALID_ARG_TYPE
    emitter.on("x", {}) / ([])         -> THREW ERR_INVALID_ARG_TYPE
    emitter.once / addListener / prependListener / prependOnceListener /
    off / removeListener with a literal -> THREW ERR_INVALID_ARG_TYPE
    --- accepted ---
    emitter.on("x", () => {})          -> ok
    emitter.on("x", function () {})    -> ok

`rejectsEventListener` drops the operation only for an exactly readable literal
that cannot be a function. An identifier, a call result or a spread may still
supply a function and keeps its operation, and a class expression passes Node's
own `typeof` check.

`emit` and `removeAllListeners` are untouched: `emit` passes its remaining
arguments as data to listeners that already exist, and `removeAllListeners` takes
only an optional event name.

* fix(javascript): price a non-finite listener and keep a shifted spread

Two review findings on the event-listener guard.

**A non-finite numeric literal was missed.** Babel stores `1e999` as a
`NumericLiteral` holding `Infinity`, and the exact-value reader refuses to price a
non-finite number, so the guard never saw it:

    e.on("x", 1e999)  -> THREW ERR_INVALID_ARG_TYPE

A numeric literal is always a number and therefore never callable, so the guard now
decides on the literal node directly.

**A leading spread shifts the listener position.** `e.on(...args, 1)` does not pass
`1` as the listener: Node spreads the array and ignores the trailing argument once
the listener is satisfied. The guard read `arguments[1]` regardless, so it dropped a
real registration. It now leaves the position undecided when argument 0 is a spread.

Five regressions cover both forms; all five fail without the fixes.
This commit is contained in:
吴佳浩Alben authored and GitHub committed 2026-11-02 16:47:22 +08:00
1 parent 2ac6d3efe2
commit 59f5502a40
2 files changed
+159 -1

No files matched your search

@@ -0,0 +1,115 @@
import { expect, it } from "vitest";
import { analyzeJavaScriptSemantics } from "./javascriptSemanticAnalysis.js";
/**
* A listener must be a function, and Node throws `ERR_INVALID_ARG_TYPE` before
* anything is registered or removed when it is not.
*
* Measured on Node 24.22.1:
*
* emitter.on(1) -> THREW ERR_INVALID_ARG_TYPE
* emitter.on("x", 1) -> THREW ERR_INVALID_ARG_TYPE
* emitter.on("x", null) -> THREW ERR_INVALID_ARG_TYPE
* emitter.on("x", true) -> THREW ERR_INVALID_ARG_TYPE
* emitter.on("x", {} / []) -> THREW ERR_INVALID_ARG_TYPE
* emitter.once / addListener /
* prependListener / prependOnceListener / off / removeListener, same
* emitter.on("x", () => {}) -> ok
* emitter.on("x", function () {}) -> ok
*/
const eventsFor = (call: string) =>
analyzeJavaScriptSemantics(
`import { EventEmitter } from "node:events";\nconst e = new EventEmitter();\nfunction g(fn, args) { ${call}; }`,
).eventOperations.map(({ kind, method, eventName }) => ({
kind,
method,
eventName,
}));
it.each([
["a numeric listener", `e.on("x", 1)`],
["a string listener", `e.on("x", "fn")`],
["a template-literal listener", 'e.on("x", `fn`)'],
["a boolean listener", `e.on("x", true)`],
["a null listener", `e.on("x", null)`],
["a void listener", `e.on("x", void 0)`],
["a BigInt listener", `e.on("x", 0n)`],
["a RegExp listener", `e.on("x", /re/)`],
["an object listener", `e.on("x", {})`],
["an array listener", `e.on("x", [])`],
["a once numeric listener", `e.once("x", 1)`],
["an addListener numeric listener", `e.addListener("x", 1)`],
["a prependListener numeric listener", `e.prependListener("x", 1)`],
["a prependOnceListener numeric listener", `e.prependOnceListener("x", 1)`],
["a removeListener numeric listener", `e.removeListener("x", 1)`],
["an off numeric listener", `e.off("x", 1)`],
] as const)("emits no event operation for %s", (_name, call) => {
expect(eventsFor(call)).toEqual([]);
});
it.each([
["an arrow listener", `e.on("x", () => {})`, "on"],
["a function-expression listener", `e.on("x", function () {})`, "on"],
["a class-expression listener", `e.on("x", class {})`, "on"],
["an identifier listener", `e.on("x", fn)`, "on"],
["a call-result listener", `e.on("x", make())`, "on"],
["a spread listener", `e.on("x", ...listeners)`, "on"],
["an once arrow listener", `e.once("x", () => {})`, "once"],
[
"a removeListener identifier",
`e.removeListener("x", fn)`,
"removeListener",
],
] as const)("keeps the event operation for %s", (_name, call, method) => {
expect(eventsFor(call)).toEqual([
{ kind: expect.any(String) as unknown as string, method, eventName: "x" },
]);
});
/**
* `emit` passes its remaining arguments to listeners that already exist, so it
* does not validate them the way a registration does.
*/
it("keeps an emit with a literal payload", () => {
expect(eventsFor(`e.emit("x", 1)`)).toEqual([
{ kind: "dispatch", method: "emit", eventName: "x" },
]);
});
/** `removeAllListeners` takes only an optional event name, never a listener. */
it("keeps removeAllListeners with a literal event name", () => {
expect(eventsFor(`e.removeAllListeners("x")`)).toEqual([
{ kind: "remove", method: "removeAllListeners", eventName: "x" },
]);
});
/**
* Two review findings.
*
* Babel stores `1e999` as a `NumericLiteral` holding `Infinity`, which the
* exact-value reader refuses to price, so the non-finite value was missed:
*
* e.on("x", 1e999) -> THREW ERR_INVALID_ARG_TYPE
*
* A leading spread shifts the runtime argument positions, so argument 1 is not
* provably the listener:
*
* e.on(...["x", fn], 1) -> registers fn; the trailing 1 is ignored
*/
it.each([
["an overflowing numeric listener", `e.on("x", 1e999)`],
["another overflowing numeric listener", `e.on("x", 1e400)`],
] as const)("emits no event operation for %s", (_name, call) => {
expect(eventsFor(call)).toEqual([]);
});
it.each([
["a leading spread", `e.on(...args, 1)`, "on"],
["a leading spread with a literal array", `e.on(...["x", fn], 1)`, "on"],
["a leading spread on once", `e.once(...args, 0)`, "once"],
] as const)("keeps the event operation for %s", (_name, call, method) => {
expect(eventsFor(call)).toEqual([
{ kind: "register", method, eventName: null },
]);
});
@@ -7,7 +7,11 @@ import type {
JavaScriptSemanticTimerOperation,
} from "./javascriptSemanticIr.js";
import { createSemanticCallableIdStack } from "./javascriptSemanticCallableStack.js";
import { semanticStaticPropertyName } from "./javascriptAstValues.js";
import {
readExactJavaScriptLiteral,
semanticStaticPropertyName,
unwrapJavaScriptExpression,
} from "./javascriptAstValues.js";
import {
resolveSemanticBindingState,
semanticResolutionBlocked,
@@ -127,6 +131,36 @@ export const collectJavaScriptSemanticAsyncEffects = (
};
};
/**
* A listener must be a function, and Node throws `ERR_INVALID_ARG_TYPE` before
* anything is registered or removed when it is not. An exactly readable literal
* that cannot be a function is therefore not an event operation, while an
* identifier, a call result or a spread may still supply one and keeps its
* operation.
*
* A class expression passes Node's own `typeof` check, so it is kept.
*/
const rejectsEventListener = (listener: t.Node): boolean => {
if (t.isSpreadElement(listener)) return false;
const value = unwrapJavaScriptExpression(listener).node;
if (t.isFunctionExpression(value) || t.isArrowFunctionExpression(value))
return false;
if (t.isClassExpression(value)) return false;
// A numeric literal is always a number, including the non-finite value Babel
// stores for `1e999`, which the exact-value reader refuses to price.
if (t.isNumericLiteral(value)) return true;
// Every unary operator yields a primitive, and a primitive is never callable.
return (
t.isUnaryExpression(value) ||
readExactJavaScriptLiteral(value).found ||
t.isTemplateLiteral(value) ||
t.isBigIntLiteral(value) ||
t.isRegExpLiteral(value) ||
t.isArrayExpression(value) ||
t.isObjectExpression(value)
);
};
const collectEvent = (
node: t.CallExpression | t.OptionalCallExpression,
ownerCallableId: string | null,
@@ -146,6 +180,15 @@ const collectEvent = (
const listenerArgument = EVENT_LISTENER_METHODS.includes(method)
? node.arguments[1]
: undefined;
// A leading spread shifts the runtime argument positions, so argument 1 is not
// provably the listener: `e.on(...["x", fn], 1)` registers `fn` and ignores the
// trailing `1`.
if (
!t.isSpreadElement(eventArgument) &&
listenerArgument !== undefined &&
rejectsEventListener(listenerArgument)
)
return;
const emitter = emitterIdentity(member.object, state);
const eventName = dataEffectLiteralString(eventArgument);
output.push({