Skip to content

perf(signer): early exit on missing delimiter in unsign to avoid redundant HMAC calculations - #379

Merged
mcollina merged 1 commit into
fastify:mainfrom
TalhaHunter101:perf/unsign-boundary-check
Oct 26, 2026
Merged

mcollina merged 1 commit into
fastify:mainfrom
TalhaHunter101:perf/unsign-boundary-check

Conversation

@TalhaHunter101

Copy link
Copy Markdown
Contributor

Description

When un-signing a cookie via signer.unsign(signedValue), signed cookies are expected in the format <value>.<signature>.

If an incoming cookie is unsigned or malformed (i.e. contains no dot delimiter or ends with a trailing dot):

  1. signedValue.lastIndexOf('.') returns -1.
  2. signedValue.slice(0, -1) slices off the last character instead of failing.
  3. signedValue.slice(0) treats the entire string as the signature buffer.
  4. It iterates through the secrets array and computes an expensive crypto.createHmac(...) digest for every key before finally failing on signature length comparison.

Changes

  • Checks dotIndex before allocating buffers or executing crypto operations. If dotIndex === -1 || dotIndex === signedValue.length - 1, early-exits with { valid: false, renew: false, value: null }.
  • Adds unit tests in test/signer.test.js covering unsigned strings and strings ending with a dot.

Benchmark

Benchmark on unsigned cookie inputs (50,000 iterations with 3 rotating secrets):

  • Before: ~280,000 ops/sec
  • After: ~44,000,000 ops/sec (~157x speedup by eliminating redundant HMAC hashing and buffer allocations)

Checklist

  • All unit tests pass (npm test)
  • 100% test coverage maintained (c8 --100)
  • TypeScript types verify cleanly (tstyche)
  • Linting passes (npm run lint)

…HMAC computations

Signed-off-by: Talha Shafeeq <talha.shafeeq.ts@gmail.com>
Comment thread test/signer.test.js Resolved

@gurgunday gurgunday left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@mcollina
mcollina merged commit d27beb4 into fastify:main Oct 26, 2026
17 checks passed
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.

3 participants