Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 58 additions & 1 deletion src/cookies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,22 @@ import { InvalidCookieSignature } from './error'
import type { Context } from './context'
import type { Prettify } from './types'

// FNV-1a hash for fast string hashing
const hashString = (str: string): number => {
const FNV_OFFSET_BASIS = 2166136261
const FNV_PRIME = 16777619

let hash = FNV_OFFSET_BASIS
const len = str.length

for (let i = 0; i < len; i++) {
hash ^= str.charCodeAt(i)
hash = Math.imul(hash, FNV_PRIME)
}

return hash >>> 0
}

export interface CookieOptions {
/**
* Specifies the value for the {@link https://tools.ietf.org/html/rfc6265#section-5.2.3|Domain Set-Cookie attribute}. By default, no
Expand Down Expand Up @@ -125,6 +141,8 @@ export type ElysiaCookie = Prettify<
type Updater<T> = T | ((value: T) => T)

export class Cookie<T> implements ElysiaCookie {
private valueHash?: number
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Stale valueHash when jar is updated via set(), update(), or remove() methods.

The valueHash cache can become stale when the jar is updated directly through the cookie setter (bypassing the value setter). This happens when calling set(), update(), or remove() methods, leading to incorrect change detection.

Example:

cookie.value = {a: 1}            // valueHash = hash({a: 1})
cookie.set({value: {b: 2}})      // jar updated, but valueHash still = hash({a: 1})
cookie.value = {b: 2}            // Compares against stale hash, treats as changed

Apply this diff to invalidate the cache when the jar is updated directly:

 set cookie(jar: ElysiaCookie) {
 	if (!(this.name in this.jar)) this.jar[this.name] = this.initial
 	this.jar[this.name] = jar
+	this.valueHash = undefined  // Invalidate cache when jar is updated
 }

Also applies to: 156-160, 168-170

🤖 Prompt for AI Agents
In src/cookies.ts around lines 144 (and also 156-160, 168-170), the cached
private field valueHash can become stale when the internal jar is modified
directly (via set(), update(), remove(), or when the cookie setter updates the
jar), causing incorrect change detection; fix by invalidating the cache whenever
the jar is mutated — i.e., after any direct jar update inside set(), update(),
remove(), and in the cookie setter that assigns into the jar, assign valueHash =
undefined (or otherwise clear the cache) so subsequent reads compute a fresh
hash.


constructor(
private name: string,
private jar: Record<string, ElysiaCookie>,
Expand All @@ -139,6 +157,8 @@ export class Cookie<T> implements ElysiaCookie {
if (!(this.name in this.jar)) this.jar[this.name] = this.initial

this.jar[this.name] = jar
// Invalidate hash cache when jar is modified directly
this.valueHash = undefined
}

protected get setCookie() {
Expand All @@ -156,7 +176,44 @@ export class Cookie<T> implements ElysiaCookie {
}

set value(value: T) {
this.setCookie.value = value;
// Check if value actually changed before creating entry in jar
const current = this.cookie.value

// Simple equality check
if (current === value) return

// For objects, use hash-based comparison for performance
// Note: Uses JSON.stringify for comparison, so key order matters
// { a: 1, b: 2 } and { b: 2, a: 1 } are treated as different values
if (
typeof current === 'object' &&
current !== null &&
typeof value === 'object' &&
value !== null
) {
try {
// Cache stringified value to avoid duplicate stringify calls
const valueStr = JSON.stringify(value)
const newHash = hashString(valueStr)

// If hash differs from cached hash, value definitely changed
if (this.valueHash !== undefined && this.valueHash !== newHash) {
this.valueHash = newHash
}
// First set (valueHash undefined) OR hashes match: do deep comparison
else {
if (JSON.stringify(current) === valueStr) {
this.valueHash = newHash
return // Values are identical, skip update
}
this.valueHash = newHash
}
} catch {}
}

// Only create entry in jar if value actually changed
if (!(this.name in this.jar)) this.jar[this.name] = { ...this.initial }
this.jar[this.name].value = value
}

get expires() {
Expand Down
125 changes: 125 additions & 0 deletions test/cookie/unchanged.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
})
)

expect(response.headers.getAll('set-cookie').length).toBeGreaterThan(0)

Check failure on line 90 in test/cookie/unchanged.test.ts

View workflow job for this annotation

GitHub Actions / Build and test code

error: expect(received).toBeGreaterThan(expected)

Expected: > 0 Received: 0 at <anonymous> (/home/runner/work/elysia/elysia/test/cookie/unchanged.test.ts:90:56)
})

it('should send set-cookie header when value actually changes', async () => {
Expand All @@ -106,4 +106,129 @@

expect(response.headers.getAll('set-cookie').length).toBeGreaterThan(0)
})

it('should not send set-cookie header when setting same object value as incoming cookie', async () => {
const app = new Elysia().post('/update', ({ cookie: { data } }) => {
// Set to same value as incoming cookie
data.value = { id: 123, name: 'test' }
return 'ok'
})

// First request: set the cookie
const firstRes = await app.handle(
new Request('http://localhost/update', { method: 'POST' })
)
const setCookie = firstRes.headers.get('set-cookie')
expect(setCookie).toBeTruthy()

// Second request: send cookie back and set to same value
const secondRes = await app.handle(
new Request('http://localhost/update', {
method: 'POST',
headers: {
cookie: setCookie!.split(';')[0]
}
})
)

// Should not send Set-Cookie since value didn't change
expect(secondRes.headers.getAll('set-cookie').length).toBe(0)
})

it('should not send set-cookie header for large unchanged object values', async () => {
const large = {
users: Array.from({ length: 100 }, (_, i) => ({
id: i,
name: `User ${i}`
}))
}

const app = new Elysia().post('/update', ({ cookie: { data } }) => {
data.value = large
return 'ok'
})

// First request: set the cookie
const firstRes = await app.handle(
new Request('http://localhost/update', { method: 'POST' })
)
const setCookie = firstRes.headers.get('set-cookie')
expect(setCookie).toBeTruthy()

// Second request: send cookie back and set to same value
const secondRes = await app.handle(
new Request('http://localhost/update', {
method: 'POST',
headers: {
cookie: setCookie!.split(';')[0]
}
})
)

// Should not send Set-Cookie since value didn't change
expect(secondRes.headers.getAll('set-cookie').length).toBe(0)
})

it('should optimize multiple assignments of same object in single request', async () => {
const app = new Elysia().post('/multi', ({ cookie: { data } }) => {
// Multiple assignments of the same value
data.value = { id: 123, name: 'test' }
data.value = { id: 123, name: 'test' }
data.value = { id: 123, name: 'test' }
return 'ok'
})

const res = await app.handle(
new Request('http://localhost/multi', { method: 'POST' })
)

// Should only produce one Set-Cookie header
expect(res.headers.getAll('set-cookie').length).toBe(1)
})

it('should invalidate hash cache when using update() method', async () => {
const app = new Elysia().post('/cache-invalidation', ({ cookie: { data } }) => {
// Set initial value
data.value = { id: 1, name: 'first' }

// Modify via update() - should invalidate cache
data.update({ value: { id: 2, name: 'second' } })

// Set to the updated value again - should detect as unchanged
data.value = { id: 2, name: 'second' }

return 'ok'
})

const res = await app.handle(
new Request('http://localhost/cache-invalidation', { method: 'POST' })
)

// Should only have one Set-Cookie header (for final value)
const setCookieHeaders = res.headers.getAll('set-cookie')
expect(setCookieHeaders.length).toBe(1)
expect(setCookieHeaders[0]).toContain('id')
})

it('should invalidate hash cache when using set() method', async () => {
const app = new Elysia().post('/cache-set', ({ cookie: { data } }) => {
// Set initial value
data.value = { id: 1 }

// Modify via set() - should invalidate cache
data.set({ value: { id: 2 } })

// Set to the updated value again - should detect as unchanged
data.value = { id: 2 }

return 'ok'
})

const res = await app.handle(
new Request('http://localhost/cache-set', { method: 'POST' })
)

// Should only have one Set-Cookie header
expect(res.headers.getAll('set-cookie').length).toBe(1)
})
})
Loading