The stringifier is not safe when it comes to throwing toJSON

One of our users has reported a somewhat weird issue - our app was crashing on a cross-origin frame that was contained somewhere in the object passed to the fast-safe-stringify.

After some debugging I've discovered that the issue is caused by the fact that even before calling the replacer function this window object throws on the internal obj.toJSON access. And the problem is... that we can't even easily supply a custom replacer to work around this because it won't see the value soon enough. We'd have to scan all the properties of the parent object, inspect their values and replace the window object there. This solution would drastically increase the amount of work needed to stringify all objects so I'm not super keen on doing this in our library.

The problem sort of can be showcased by this simplified version here:

JSON.stringify(
  {
    a: 1,
    b: {
      toJSON() {
        console.log('toJSON called');
        throw new Error("Oops");
      },
    },
  },
  (key, value) => {
    console.log({ key, value });
    return value;
  }
);

This results in this being logged:

{key: '', value: {…}}
{key: 'a', value: 1}
toJSON called

As we may see - the b key is never observed because toJSON is called first and it throws. In the original issue, this already throws on property access, I imagine that on logic like this 'toJSON' in obj.

And this simplified example showcases the problem quite accurately, even though it doesn't use this library. This is because at the end of the day... you are still using JSON.stringify under the hood: https://github.com/davidmarkclements/fast-safe-stringify/blob/98076ea2d8719e0c95c5d056ffd61f16ca53dbae/index.js#L28-L32

I'm not sure if you should fix this... this would be quite nice for me and I could help in fixing this. But I'm not sure how to best approach this one - this would probably require attaching a fake toJSON to all objects (or even everything?), implementing proper try/catch in it, returning the original value so it could be handled by the replacer and then removing the fake toJSON from the mutated object 😬

I'm opening this issue to see if you have any better ideas on how this could be handled. And for posterity, since somebody might run into this too.

davidmarkclements / fast-safe-stringify

The stringifier is not safe when it comes to throwing toJSON #62