Unable to cause an error during payload parsing

[matthieusieben]

Support plan

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

Context

• node version: v14.14.0
• module version with issue: 19.2.0
• last module version without issue: 19.2.0
• environment (e.g. node, browser, native): node
• used with (e.g. hapi application, another framework, standalone, ...): hapi application
• any other relevant information: n.a.

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

I try to implement a plugin that validates that the Digest header matches the payload.

I want to output a nice error message if the payload does not match the header

server.ext('onRequest', (request, h) => {

    const { algorithm, digest } = getDigest(request.headers)

    const hash = Crypto.createHash(algorithm);
    request.events.on('peek', (chunk) => {

        hash.update(chunk);
    });

    request.events.once('finish', () => {

        if (digest !== hash.digest('base64')) throw new Boom.badData('nice error message')
    });

    return h.continue;
});

What was the result you got?

The server crashes due to an uncaught exception.

What result did you expect?

An http error response should be returned.

[kanongil]

You are throwing in an event handler. This is not supported, and a very bad idea. Doing this in node will in general cause an uncaught exception, as you are experiencing.

You will probably need to add another ext (after the payload has been received) to reject with any such error.

[matthieusieben]

So there is no way to tell Hapi "this payload is corrupt do not even try to parse it" ?

[matthieusieben]

And even if you are right, and listeners should not throw, it is still possible to catch any error since Podium exposes a promise when emitting. Do you see an issue with forwarding any such error to the stream (as I did in the PR) to gracefully end the processing the request instead of crashing the server ?

[kanongil]

Oh, I forgot that podium handles thrown errors. I guess it makes sense to handle this case as well.

[nlf]

request.events is a podium instance, which allows you to omit the second parameter (the listener function) and instead it will return a promise that resolves when the event takes place.

meaning, you can do

await request.events.once('finish')
if (digest !== hash.digest('base64'))
  throw new Boom.badData('nice error message')
return h.continue

and you don't need to throw an error from within the listener function, which i believe solves your use case

[matthieusieben]

Hey @nlf , thanks for your answer. I did try that but it didn't solve my use case. First of all, your exact suggestion will create a dead lock and the actual implementation is a little bit more complicated that what you showed. This is due to the fact that payload processing can occur at different stages of the request lifecycle (depending on route and auth payload options), and can even be overridden by some onRequest listener. To solve that, the await on the finish promise needs to be performed during the onPostAuth event, which means that other listeners (from other plugins, auth, etc.) could have had access to the request payload, which I want to prevent altogether if the payload cannot be "trusted".