Skip to content

Quality: Global mutable call id can grow unbounded across long-lived processes#2691

Open
tuanaiseo wants to merge 1 commit intosinonjs:mainfrom
tuanaiseo:contribai/improve/quality/global-mutable-call-id-can-grow-unbounde
Open

Quality: Global mutable call id can grow unbounded across long-lived processes#2691
tuanaiseo wants to merge 1 commit intosinonjs:mainfrom
tuanaiseo:contribai/improve/quality/global-mutable-call-id-can-grow-unbounde

Conversation

@tuanaiseo
Copy link
Copy Markdown

Problem

callId is module-scoped and incremented on every invocation. In long-running test runners or embedded usage, this can grow indefinitely and eventually lose integer precision semantics for strict ordering comparisons.

Severity: low
File: lib/sinon/proxy-invoke.js

Solution

Reset or wrap callId safely (e.g., modulo Number.MAX_SAFE_INTEGER) and document ordering guarantees when rollover occurs.

Changes

  • lib/sinon/proxy-invoke.js (modified)

Testing

  • Existing tests pass
  • Manual review completed
  • No new warnings/errors introduced

`callId` is module-scoped and incremented on every invocation. In long-running test runners or embedded usage, this can grow indefinitely and eventually lose integer precision semantics for strict ordering comparisons.

Affected files: proxy-invoke.js

Signed-off-by: tuanaiseo <221258316+tuanaiseo@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@fatso83 fatso83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for noticing! Just a minor note.

Comment thread lib/sinon/proxy-invoke.js
const matchings = this.matchingFakes(args);
const currentCallId = callId++;
const currentCallId = callId;
callId = callId === maxSafeInteger ? 0 : callId + 1;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe replace that

Suggested change
callId = callId === maxSafeInteger ? 0 : callId + 1;
callId = callId >= maxSafeInteger ? 0 : callId + 1;

And we require a test 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants