From 56db73be27ae4f2098aaa378934bbd51c6be6d46 Mon Sep 17 00:00:00 2001 From: Alex Yang Date: Sun, 1 Mar 2026 21:42:41 +0900 Subject: [PATCH] fix(core): revive date strings in safeJSONParse for pre-parsed objects (#8248) --- .../src/db/internal-adapter.test.ts | 144 ++++++++++++++++++ packages/core/src/utils/json.ts | 55 +++++-- 2 files changed, 187 insertions(+), 12 deletions(-) diff --git a/packages/better-auth/src/db/internal-adapter.test.ts b/packages/better-auth/src/db/internal-adapter.test.ts index 7c35d2a7b9..be9b148ba8 100644 --- a/packages/better-auth/src/db/internal-adapter.test.ts +++ b/packages/better-auth/src/db/internal-adapter.test.ts @@ -1344,4 +1344,148 @@ describe("internal adapter test", async () => { expect(ttl).toBeLessThanOrEqual(300); }); }); + + describe("safeJSONParse date revival in secondary storage", () => { + /** + * Simulates a Redis client that auto-parses JSON (e.g. ioredis with + * certain configurations). The `get` method returns a pre-parsed object + * where date fields are still ISO 8601 strings, not Date instances. + */ + function createPreParsedStorage() { + const dataMap = new Map(); + const ttlMap = new Map(); + return { + dataMap, + ttlMap, + storage: { + set(key: string, value: string, ttl?: number) { + // Store as pre-parsed object (simulating Redis auto-parse) + dataMap.set(key, JSON.parse(value)); + if (ttl) ttlMap.set(key, ttl); + }, + get(key: string) { + return dataMap.get(key) ?? null; + }, + delete(key: string) { + dataMap.delete(key); + ttlMap.delete(key); + }, + }, + }; + } + + it("should return Date objects from findVerificationValue when storage returns pre-parsed objects", async () => { + const { storage } = createPreParsedStorage(); + + const opts = { + database: new DatabaseSync(":memory:"), + secondaryStorage: storage, + } satisfies BetterAuthOptions; + + (await getMigrations(opts)).runMigrations(); + const ctx = await init(opts); + + await ctx.internalAdapter.createVerificationValue({ + identifier: "date-test", + value: "test-value", + expiresAt: new Date(Date.now() + 60000), + }); + + const found = + await ctx.internalAdapter.findVerificationValue("date-test"); + expect(found).not.toBeNull(); + expect(found!.expiresAt).toBeInstanceOf(Date); + expect(found!.createdAt).toBeInstanceOf(Date); + expect(found!.updatedAt).toBeInstanceOf(Date); + }); + + it("should correctly detect expired verification when storage returns pre-parsed objects", async () => { + const { storage } = createPreParsedStorage(); + + const opts = { + database: new DatabaseSync(":memory:"), + secondaryStorage: storage, + } satisfies BetterAuthOptions; + + await (await getMigrations(opts)).runMigrations(); + const ctx = await init(opts); + + await ctx.internalAdapter.createVerificationValue({ + identifier: "expiry-check", + value: "test-value", + expiresAt: new Date(Date.now() + 60000), + }); + + const found = + await ctx.internalAdapter.findVerificationValue("expiry-check"); + expect(found).not.toBeNull(); + // This comparison would silently fail if expiresAt were a string + // because string < Date coerces to NaN, making it always false + expect(found!.expiresAt > new Date()).toBe(true); + expect(found!.expiresAt < new Date(Date.now() + 120000)).toBe(true); + }); + + it("should return Date objects for all date fields across multiple reads", async () => { + const { storage } = createPreParsedStorage(); + + const opts = { + database: new DatabaseSync(":memory:"), + secondaryStorage: storage, + } satisfies BetterAuthOptions; + + await (await getMigrations(opts)).runMigrations(); + const ctx = await init(opts); + + const expiresAt = new Date(Date.now() + 60000); + await ctx.internalAdapter.createVerificationValue({ + identifier: "multi-read-test", + value: "test-value", + expiresAt, + }); + + // First read: safeJSONParse receives pre-parsed object from storage + const first = + await ctx.internalAdapter.findVerificationValue("multi-read-test"); + expect(first).not.toBeNull(); + expect(first!.expiresAt).toBeInstanceOf(Date); + expect(first!.createdAt).toBeInstanceOf(Date); + expect(first!.updatedAt).toBeInstanceOf(Date); + + // Second read: verify consistency (the stored object wasn't mutated) + const second = + await ctx.internalAdapter.findVerificationValue("multi-read-test"); + expect(second).not.toBeNull(); + expect(second!.expiresAt).toBeInstanceOf(Date); + expect(second!.expiresAt.getTime()).toBe(first!.expiresAt.getTime()); + }); + + it("should preserve non-date string fields when reviving dates", async () => { + const { storage } = createPreParsedStorage(); + + const opts = { + database: new DatabaseSync(":memory:"), + secondaryStorage: storage, + } satisfies BetterAuthOptions; + + await (await getMigrations(opts)).runMigrations(); + const ctx = await init(opts); + + await ctx.internalAdapter.createVerificationValue({ + identifier: "string-field-test", + value: "my-token-value-123", + expiresAt: new Date(Date.now() + 60000), + }); + + const found = + await ctx.internalAdapter.findVerificationValue("string-field-test"); + expect(found).not.toBeNull(); + // Non-date strings must NOT be converted + expect(found!.identifier).toBe("string-field-test"); + expect(typeof found!.identifier).toBe("string"); + expect(found!.value).toBe("my-token-value-123"); + expect(typeof found!.value).toBe("string"); + // Date strings MUST be converted + expect(found!.expiresAt).toBeInstanceOf(Date); + }); + }); }); diff --git a/packages/core/src/utils/json.ts b/packages/core/src/utils/json.ts index 0a8036b3c5..056b4f234a 100644 --- a/packages/core/src/utils/json.ts +++ b/packages/core/src/utils/json.ts @@ -1,23 +1,54 @@ import { logger } from "../env"; -export function safeJSONParse(data: unknown): T | null { - function reviver(_: string, value: any): any { - if (typeof value === "string") { - const iso8601Regex = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)?Z$/; - if (iso8601Regex.test(value)) { - const date = new Date(value); - if (!isNaN(date.getTime())) { - return date; - } - } +const iso8601Regex = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)?Z$/; + +function reviveDate(value: unknown): any { + if (typeof value === "string" && iso8601Regex.test(value)) { + const date = new Date(value); + if (!isNaN(date.getTime())) { + return date; } + } + return value; +} + +/** + * Recursively walk a pre-parsed object and convert ISO 8601 date strings + * to Date instances. This handles the case where a Redis client (or similar) + * returns already-parsed JSON objects whose date fields are still strings. + */ +function reviveDates(value: unknown): any { + if (value === null || value === undefined) { return value; } + if (typeof value === "string") { + return reviveDate(value); + } + if (value instanceof Date) { + return value; + } + if (Array.isArray(value)) { + return value.map(reviveDates); + } + if (typeof value === "object") { + const result: Record = {}; + for (const key of Object.keys(value)) { + result[key] = reviveDates((value as Record)[key]); + } + return result; + } + return value; +} + +export function safeJSONParse(data: unknown): T | null { try { if (typeof data !== "string") { - return data as T; + if (data === null || data === undefined) { + return null; + } + return reviveDates(data) as T; } - return JSON.parse(data, reviver); + return JSON.parse(data, (_, value) => reviveDate(value)); } catch (e) { logger.error("Error parsing JSON", { error: e }); return null;