Skip to content

fix(jwt): detect the redis client correctly and branch markUsed on it - #28

Merged
ExorTek merged 1 commit into
masterfrom
fix/jwt-redis-dialect
Aug 6, 2026
Merged

fix(jwt): detect the redis client correctly and branch markUsed on it#28
ExorTek merged 1 commit into
masterfrom
fix/jwt-redis-dialect

Conversation

@ExorTek

@ExorTek ExorTek commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Stacked on #27 — merge that first.

@exortek/jwt@1.2.2's Redis store does not work with any supported client. Two independent defects:

  1. Detection never matches ioredis. The store picked its dialect from client.constructor.name, accepting only 'Redis' or 'Cluster'. A real ioredis instance reports 'EventEmitter' on both v5 and v6, so every ioredis client was treated as node-redis and add() sent SET key value [object Object].
  2. markUsed ignored the detected dialect and always used the ioredis positional eval form. node-redis does not reject that — it sends EVAL <script> 0, so the script ran against an empty KEYS and failed inside Redis.

Between them, the blacklist and the RFC 6749 §10.4 refresh-reuse registry were unusable on Redis.

Detection now probes the API surface (scanStream / status), as paseto's store already did — that fix was made there and never brought across. markUsed branches like its neighbours. deleteAll also seeds its SCAN cursor as a string, which node-redis requires from v6 and the peer range admits.

Measured against the published package via its public entrypoint

before after
ioredis 5 / 6 add()ERR syntax error pass
node-redis 4 markUsed() → Lua arg error pass

The integration suite's todo markers are lifted, and it now covers deleteAll, which had no live-Redis coverage at all.

Copilot AI lite review requested due to automatic review settings August 6, 2026 09:10

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Base automatically changed from test/repo-redis-driver-matrix to master August 6, 2026 09:14
The store picked its dialect from `client.constructor.name`, matching only
'Redis' or 'Cluster'. A real ioredis instance reports 'EventEmitter' — it
extends EventEmitter and no class name survives to the instance — so the
probe never matched and every ioredis client was treated as node-redis,
sending `SET key value [object Object]`.

`markUsed` had a second, independent defect: it never consulted the detected
dialect and always used the ioredis positional `eval` form. node-redis does
not reject that call, it sends `EVAL <script> 0`, so the script ran against an
empty KEYS and failed inside Redis.

Between the two, the blacklist and the RFC 6749 §10.4 refresh-reuse registry
were unusable on every supported client. Detection now probes the API surface
(`scanStream` / `status`) as paseto's store already did, and `markUsed`
branches like its neighbours.

Also seeds the `deleteAll` SCAN cursor as a string. node-redis typed cursors
as numbers through v5 but requires a string from v6, and the peer range
(`>=4.0.0`) admits v6.

The integration suite's todo markers are lifted and it now covers deleteAll,
which had no live-Redis coverage at all.
@ExorTek
ExorTek force-pushed the fix/jwt-redis-dialect branch from 7650adc to 713d9e7 Compare August 6, 2026 09:14
@ExorTek
ExorTek merged commit 925efa8 into master Aug 6, 2026
3 checks passed
@ExorTek
ExorTek deleted the fix/jwt-redis-dialect branch August 6, 2026 09:17
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