From ee49a2b67da007a12cad6ed7c182a8470d0640e7 Mon Sep 17 00:00:00 2001 From: Gil Pedersen Date: Sat, 18 Jul 2026 12:53:37 +0200 Subject: [PATCH 1/3] Move cookie to state processing to request --- lib/request.js | 38 ++++++++++++++++++++++++++++++++++++++ lib/route.js | 37 ++----------------------------------- 2 files changed, 40 insertions(+), 35 deletions(-) diff --git a/lib/request.js b/lib/request.js index 2c40cdc98..26edc0981 100755 --- a/lib/request.js +++ b/lib/request.js @@ -546,6 +546,44 @@ exports = module.exports = internals.Request = class { this.response = response; } + async _parseCookies() { + + this.state = {}; + + const req = this.raw.req; + const cookies = req.headers.cookie; + if (!cookies) { + return; + } + + try { + var result = await this._core.states.parse(cookies); + } + catch (err) { + Bounce.rethrow(err, 'system'); + var parseError = err; + } + + const { states, failed = [] } = result ?? parseError; + this.state = states ?? {}; + + // Clear cookies + + for (const item of failed) { + if (item.settings.clearInvalid) { + this._clearState(item.name); + } + } + + if (!parseError) { + return; + } + + parseError.header = cookies; + + return this._core.toolkit.failAction(this, this.route.settings.state.failAction, parseError, { tags: ['state', 'error'] }); + } + _setState(name, value, options) { const state = { name, value }; diff --git a/lib/route.js b/lib/route.js index 6491693ff..c0ee00f9a 100755 --- a/lib/route.js +++ b/lib/route.js @@ -361,42 +361,9 @@ exports = module.exports = internals.Route = class { }; -internals.state = async function (request) { +internals.state = function (request) { - request.state = {}; - - const req = request.raw.req; - const cookies = req.headers.cookie; - if (!cookies) { - return; - } - - try { - var result = await request._core.states.parse(cookies); - } - catch (err) { - Bounce.rethrow(err, 'system'); - var parseError = err; - } - - const { states, failed = [] } = result ?? parseError; - request.state = states ?? {}; - - // Clear cookies - - for (const item of failed) { - if (item.settings.clearInvalid) { - request._clearState(item.name); - } - } - - if (!parseError) { - return; - } - - parseError.header = cookies; - - return request._core.toolkit.failAction(request, request.route.settings.state.failAction, parseError, { tags: ['state', 'error'] }); + return request._parseCookies(); }; From 5fc7692d27e508dc79baeedf3e591db2fa9b75cc Mon Sep 17 00:00:00 2001 From: Gil Pedersen Date: Tue, 21 Jul 2026 23:00:32 +0200 Subject: [PATCH 2/3] Limit state updates to clearing for error 500 response Closes #4297, Closes #4527 --- API.md | 2 +- lib/headers.js | 44 +++++++++++++++--------- test/state.js | 91 ++++++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 118 insertions(+), 19 deletions(-) diff --git a/API.md b/API.md index 7b9316f06..deea46d3b 100755 --- a/API.md +++ b/API.md @@ -2540,7 +2540,7 @@ across multiple requests. Registers a cookie definitions where: - `domain` - the domain scope. Defaults to `null` (no domain). - - `autoValue` - if present and the cookie was not received from the client or explicitly set by the route handler, the cookie is automatically added to the response with the provided value. The value can be a function with signature `async function(request)` where: + - `autoValue` - if present and the cookie was not received from the client or explicitly set by the route handler, the cookie is automatically added to the response with the provided value, unless state parsing is disabled or the response code is 500. The value can be a function with signature `async function(request)` where: - `request` - the [request object](#request). diff --git a/lib/headers.js b/lib/headers.js index 69a528e18..614f83a6c 100755 --- a/lib/headers.js +++ b/lib/headers.js @@ -67,24 +67,32 @@ exports.state = async function (response) { const request = response.request; const states = []; - for (const stateName in request._states) { - states.push(request._states[stateName]); + const clearOnly = response._error?.output.statusCode === 500; + + for (const name in request._states) { + if (!clearOnly || + (request._states[name].options?.ttl === 0 && request._core.states.cookies[name])) { + + states.push(request._states[name]); + } } try { - for (const name in request._core.states.cookies) { - const autoValue = request._core.states.cookies[name].autoValue; - if (!autoValue || name in request._states || name in request.state) { - continue; - } - - if (typeof autoValue !== 'function') { - states.push({ name, value: autoValue }); - continue; + if (!clearOnly) { + for (const name in request._core.states.cookies) { + const autoValue = request._core.states.cookies[name].autoValue; + if (!autoValue || name in request._states || name in request.state) { + continue; + } + + if (typeof autoValue !== 'function') { + states.push({ name, value: autoValue }); + continue; + } + + const value = await autoValue(request); + states.push({ name, value }); } - - const value = await autoValue(request); - states.push({ name, value }); } if (!states.length) { @@ -100,9 +108,13 @@ exports.state = async function (response) { response._header('set-cookie', header); } catch (err) { - const error = Boom.boomify(err); + if (!(err instanceof Error)) { + // eslint-disable-next-line no-ex-assign + err = Boom.badImplementation('A non-Error value was thrown', err); + } + + const error = Boom.boomify(err, { statusCode: 500 }); request._log(['state', 'response', 'error'], error); - request._states = {}; // Clear broken state throw error; } }; diff --git a/test/state.js b/test/state.js index b6842342f..a609c2e67 100755 --- a/test/state.js +++ b/test/state.js @@ -1,5 +1,6 @@ 'use strict'; +const Boom = require('@hapi/boom'); const Code = require('@hapi/code'); const Hapi = require('..'); const Lab = require('@hapi/lab'); @@ -113,6 +114,34 @@ describe('state', () => { expect(res.headers['set-cookie'][0]).to.equal('a=; Max-Age=0; Expires=Thu, 01 Jan 1970 00:00:00 GMT; Secure; HttpOnly; SameSite=Strict'); }); + it('clears invalid cookies on error 500 response', async () => { + + const server = Hapi.server(); + server.state('a', { ignoreErrors: true, encoding: 'base64json', clearInvalid: true }); + server.route({ path: '/', method: 'GET', handler: () => { + + throw new Error('Fail'); + } }); + const res = await server.inject({ method: 'GET', url: '/', headers: { cookie: 'a=x' } }); + expect(res.statusCode).to.equal(500); + expect(res.headers['set-cookie'][0]).to.equal('a=; Max-Age=0; Expires=Thu, 01 Jan 1970 00:00:00 GMT; Secure; HttpOnly; SameSite=Strict'); + }); + + it('does not clear unregistered cookie on error 500 response', async () => { + + const server = Hapi.server(); + server.route({ + path: '/', method: 'GET', handler: (request, h) => { + + h.unstate('a'); + throw new Error('Fail'); + } + }); + const res = await server.inject({ method: 'GET', url: '/', headers: { cookie: 'a=x' } }); + expect(res.statusCode).to.equal(500); + expect(res.headers['set-cookie']).to.not.exist(); + }); + it('sets cookie value automatically', async () => { const server = Hapi.server(); @@ -168,11 +197,13 @@ describe('state', () => { expect(res.headers['set-cookie']).to.equal(['always=sweet; Secure; HttpOnly; SameSite=Strict']); }); - it('fails to set cookie value automatically using function', async () => { + it('returns error 500 response when automatic value throws', async () => { + let called = 0; const present = (request) => { - throw new Error(); + ++called; + throw Boom.forbidden(); }; const server = Hapi.server(); @@ -182,6 +213,49 @@ describe('state', () => { const res = await server.inject('/'); expect(res.statusCode).to.equal(500); expect(res.headers['set-cookie']).to.not.exist(); + expect(called).to.equal(1); + }); + + it('handles automatic value throw with non-Error', async () => { + + let called = 0; + const present = (request) => { + + ++called; + throw 'fail'; + }; + + const server = Hapi.server({ debug: false }); + server.route({ method: 'GET', path: '/', handler: () => 'ok' }); + server.state('always', { autoValue: present }); + + const res = await server.inject('/'); + expect(res.statusCode).to.equal(500); + expect(res.headers['set-cookie']).to.not.exist(); + expect(res.request.response._error).to.be.an.error('A non-Error value was thrown'); + expect(called).to.equal(1); + }); + + it('does not send autoValue cookie on error 500 response', async () => { + + let called = 0; + const present = (request) => { + + ++called; + return 'present'; + }; + + const server = Hapi.server(); + server.route({ method: 'GET', path: '/', handler: () => { + + throw new Error('Fail'); + } }); + server.state('always', { autoValue: present }); + + const res = await server.inject('/'); + expect(res.statusCode).to.equal(500); + expect(res.headers['set-cookie']).to.not.exist(); + expect(called).to.equal(0); }); it('sets cookie value with null ttl', async () => { @@ -236,4 +310,17 @@ describe('state', () => { expect(res.statusCode).to.equal(500); expect(res.request.response._error).to.be.an.error('Partitioned cookies must have SameSite=None'); }); + + it('returns error 500 for invalid cookie values', async () => { + + const server = Hapi.server({ debug: false }); + + server.state('a', { encoding: 'none' }); + server.route({ method: 'GET', path: '/', handler: (request, h) => h.response('ok').state('a', 'ั‚ะตัั‚') }); + + const res = await server.inject('/'); + expect(res.statusCode).to.equal(500); + expect(res.headers['set-cookie']).to.not.exist(); + expect(res.request.response._error).to.be.an.error(TypeError); + }); }); From 973261984d33cb0e0854283f290579782b109586 Mon Sep 17 00:00:00 2001 From: Gil Pedersen Date: Tue, 21 Jul 2026 23:01:31 +0200 Subject: [PATCH 3/3] Don't try to set autoValue cookie when state parsing is disabled Closes #4316 --- lib/headers.js | 2 +- test/state.js | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/headers.js b/lib/headers.js index 614f83a6c..088926abc 100755 --- a/lib/headers.js +++ b/lib/headers.js @@ -78,7 +78,7 @@ exports.state = async function (response) { } try { - if (!clearOnly) { + if (request.state && !clearOnly) { for (const name in request._core.states.cookies) { const autoValue = request._core.states.cookies[name].autoValue; if (!autoValue || name in request._states || name in request.state) { diff --git a/test/state.js b/test/state.js index a609c2e67..d1b21e00b 100755 --- a/test/state.js +++ b/test/state.js @@ -197,6 +197,17 @@ describe('state', () => { expect(res.headers['set-cookie']).to.equal(['always=sweet; Secure; HttpOnly; SameSite=Strict']); }); + it('does not respond with automatic value when state parsing is disabled', async () => { + + const server = Hapi.server(); + server.route({ method: 'GET', path: '/', handler: () => 'ok', options: { state: { parse: false } } }); + server.state('always', { autoValue: 'present' }); + + const res = await server.inject('/'); + expect(res.statusCode).to.equal(200); + expect(res.headers['set-cookie']).to.not.exist(); + }); + it('returns error 500 response when automatic value throws', async () => { let called = 0;