hapijs / yar

A hapi session manager
Other
133 stars 59 forks source link

Fix: prevent adding keys with undefined values to the yar object in lazy mode #160

Open orakili opened 2 years ago

orakili commented 2 years ago

This PR adds a check before copying stored keys to the yar object in lazy mode to prevent adding undefined values.

Problem

In some cases the yar.onPreAuth() can be called several times in a row without any yar.onPreResponse() call in between.

In lazy mode, this causes the properties moved from the internal _store to the yar object to be overridden with undefined values.

Example with a "authorize" lazy key:

  1. first call to onPreAuth: "authorize" property is moved from the _store to the yar object in the _initialize().
  2. second call to onPreAuth: the "authorize" key is still the _store._lazyKeys so the code tries to move the "authorize" value from the store to the yar object. However this was already moved in (1), so this results in the "authorize" property on the yar object to be overridden with an "undefined" value.

Solution

Only move the properties if they exist in the _store.

Alternative solution

Empty the _lazyKeys after moving the values in _initialize(). Note: deleting the _lazyKeys would result in the lazy mode to be reseted to false, so _lazyKeys needs to be set to [] instead of being deleted.

devinivy commented 2 years ago

It's possible I've missed something here, but I do not believe yar's onPreAuth could be called multiple times for the same request. So it seems like you might have a situation where _store state is being shared across multiple requests, and I am trying to imagine how this could happen. Do you use the catbox-object cache engine, by chance?

If I've misunderstood here, would you be able to write a failing test case to illustrate? I believe that in order land a fix like this we will need to add a test to ensure this case is covered in the test suite.