Exception when using takeover response in onPreResponse and the server is configured with autoValue

sveisvei commented 2 years ago

Support plan

is this issue currently blocking your project? (yes/no): no
is this issue affecting a production system? (yes/no): no

Context

node version: v14.17.6
module version with issue: "@hapi/hapi": "^20.2.1",
last module version without issue: ?
environment (e.g. node, browser, native): node
used with standalone
any other relevant information:

What are you trying to achieve or the steps to reproduce?

I am using the cookie state autoValue for a persistent id. When trying to use the onPreResponse to takeover a response, it fails because request.state is null.

TypeError: Cannot use 'in' operator to search for 'tid' in null
    at exports.state (/node_modules/.pnpm/@hapi+hapi@20.2.1/node_modules/@hapi/hapi/lib/headers.js:88:57)
    at Object.internals.marshal (/node_modules/.pnpm/@hapi+hapi@20.2.1/node_modules/@hapi/hapi/lib/transmit.js:41:15)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at async Object.exports.send (/node_modules/.pnpm/@hapi+hapi@20.2.1/node_modules/@hapi/hapi/lib/transmit.js:27:9)
    at async Request._reply (/node_modules/.pnpm/@hapi+hapi@20.2.1/node_modules/@hapi/hapi/lib/request.js:457:5)

Reproducible demo:

const Hapi = require("@hapi/hapi");
const { isBoom } = require('@hapi/boom');

const server = new Hapi.Server({
  port: 3333,
});

server.state("tid", {
  ttl: 1000 * 60 * 60 * 24 * 31 * 3, // 3 months
  isSecure: false,
  isHttpOnly: true,
  encoding: "none",
  clearInvalid: true,
  strictHeader: true,
  autoValue: () => 'v',
  path: "/",
});

server.ext("onPreResponse", (request, h) => {
  const err = request.response;
  if (isBoom(err)) {
    return h.response("test").code(err.output.statusCode);
  }
  return h.continue;
});

server.start();
console.log(server.info.uri);

What was the result you got?

Exception.

What result did you expect?

I would expect that onPreResponse can be taken over.

sveisvei commented 2 years ago

Open to a PR to fix this? My naive PR would improve the sanity checks on states, but I assume the error really is in the actual states object MIA.

Alternative is for me to stop using the autoValue() feature.

devinivy commented 2 years ago

Hey, thanks for the report. I will take a look at this some time today, but if you can see what's wrong we'd certainly accept a PR!

sveisvei commented 2 years ago

I can have a look, but do you have an hunch why the state might be undefined when throwing an error inside onPreResponse?

kanongil commented 2 years ago

This could very well be related to #4297, which provides some insight.

hapijs / hapi