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..088926abc 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 (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) { + 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/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(); }; diff --git a/test/state.js b/test/state.js index b6842342f..d1b21e00b 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,24 @@ 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('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; const present = (request) => { - throw new Error(); + ++called; + throw Boom.forbidden(); }; const server = Hapi.server(); @@ -182,6 +224,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 +321,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); + }); });