Fix cookie processing issues

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).
 

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;
     }
 };

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 };

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();
 };
 
 

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);
+    });
 });