ReviewOS

stacks/stx

useCookie writes on read: a second declaration with no options silently downgrades a persistent cookie to a session cookie

#1933
Closed glennmichael123 opened this 14 days ago · 0 comments
14 days ago

useCookie(name) writes on read. Declaring it anywhere runs an immediate effect that re-serialises the cookie from opts, so a second declaration with different options silently rewrites the attributes the first one set — no set() call involved.

The concrete result in my app: a 30-day "remember me" login becomes a session cookie that dies when the browser closes, purely because the user opened the settings page.

Found on 0.2.176.

Mechanism

useCookie ends with an unconditional effect, and effects run immediately:

effect(function() {
  if (typeof document === 'undefined') return;
  document.cookie = serialise(s());
});

serialise emits max-age only when the caller supplied one:

if (value === '') parts.push('max-age=0');
else if (typeof opts.maxAge === 'number') parts.push('max-age=' + opts.maxAge);
else if (opts.expires) parts.push('expires=' + opts.expires.toUTCString());

So a declaration with no options writes the value back with path and SameSite but no max-age and no expires — the definition of a session cookie. The value survives; the lifetime does not.

Reproduction

Two views, one cookie. The login view owns the policy:

// login.stx — persist for 30 days
useCookie('auth-token', { maxAge: 60 * 60 * 24 * 30 }).set(token)

The settings view only needs to READ it, and to clear it on sign-out:

// settings.stx
const authToken = useCookie('auth-token')
const signOut = () => { authToken.set(''); location.assign('/login') }

Measured in Chromium (Playwright reports a session cookie as expires === -1):

after login       : persistent, expires in 30d
after /queue      : persistent, expires in 30d     <- control: page does not declare the cookie
after /settings   : SESSION (dies with the browser)
value preserved   : true

Nothing was set. The page was opened.

Why this is worth changing rather than documenting

The read path and the write path are the same call, so there is no way to spell "read this cookie" without also asserting a full attribute set. Every consumer of a shared cookie therefore has to restate the owner's policy exactly, and any consumer that forgets silently downgrades it. It fails open, in the direction of losing the session, and it is invisible in devtools unless you are watching the expiry column at the moment of navigation.

It also breaks the usual signal intuition. state() does not write anything on creation; useCookie looks like state() with persistence, but constructing it mutates shared browser state.

Suggested

Write on change, not on creation — skip the first effect run and let read() seed the signal, which it already does:

var first = true
effect(function() {
  var value = s()
  if (typeof document === 'undefined') return
  if (first) { first = false; return }   // creation is a read, not a write
  document.cookie = serialise(value)
})

That keeps every set() working, keeps SSR safe, and makes a bare useCookie(name) a pure read. The cost is that a defaultValue no longer materialises into document.cookie until something writes — which seems like the better default too, but if that behaviour is load-bearing it could be opt-in via { persistDefault: true }.

If writing on creation must stay, then maxAge/expires should be preserved from the existing cookie when the caller does not specify one, so a read-only declaration cannot shorten a lifetime it never set.

Workaround

Declare the cookie once and share it, so the policy has a single home:

// resources/stores/session.ts
export const useSession = defineStore('session', () => {
  const token = useCookie('auth-token', { maxAge: 60 * 60 * 24 * 30 })
  return { token, signOut() { token.set(''); location.assign('/login') } }
})

Worth a line in the docs regardless — "every declaration of the same cookie must repeat its options" is not something a caller would guess.

Sign in to comment on this issue.