Html: Reconsider structured cloning of errors to use .name instead of [[GetPrototypeOf]]

[domenic]

In https://github.com/whatwg/html/issues/4268, largely guided by https://github.com/tc39/ecma262/issues/1389, we decided to use [[GetPrototypeOf]]() (~ .__proto__) to figure out what type of error object was being cloned. In particular we compared the retrieved prototype with the current Realm's %TypeError%, %EvalError%, etc.

In https://bugs.chromium.org/p/chromium/issues/detail?id=1030086 we discovered that this is kind of fragile. In particular it leads to weird situations like:

• If you run script in a child frame to postMessage an error to the parent frame, then it comes out with its original type intact.
• If you run script in a parent frame to postMesssage a (synchronously-accessible) child frame's error to a different child frame, then it comes out as Error.

As such I suggest we move from using .__proto__ to using .name. Both are equally forgeable but using .name will work better in these kind of weird cross-realm cases.

We could also do something like checking .constructor.name or .__proto__.name but IMO we should minimize the number of operations we do, and those would both require two Get()s.

Thoughts? @yhirano @bzbarsky

(Note to self: if we change this then we should fix https://github.com/whatwg/html/issues/4991 at the same time. In particular if we move to Get() then we need to use ? instead of !)

[bzbarsky]

This seems fine to me, I think. @jorendorff, any opinions?

Fwiw, SpiderMonkey has a concept of "error type" that is in fact not forgeable and is attached to every Error object. But that doesn't map to any spec concepts and may not have equivalents in other implenentations, so probably isn't very usable...

[ljharb]

There is an [[ErrorData]] internal slot on Errors, so it'd be totally reasonable in the spec for that to contain the type. Is there a reason that you can't look up the error type in an unforgeable fashion from internal slots rather than doing observable lookups?

[domenic]

We discussed that previously in https://github.com/tc39/ecma262/issues/1389. In particular there is no observable way to tell the difference between a "real" TypeError and a "forged" TypeError right now, so we shouldn't introduce one via the structured clone algorithm.

[ljharb]

Ah, right - checking the slot with a fallback to .name might be the best of both worlds, unless it'd be surprising to someone who had Object.assign(new TypeError(), { name: 'RangeError' })

[annevk]

Though it was discussed previously, it seems https://github.com/tc39/ecma262/issues/1389#issuecomment-451705450 would apply to this solution.

[domenic]

Yeah, it's a tradeoff between how many edge cases we want to prohibit, versus how many we want to allow. I think the edge case of confusing cross-realm issues (which people might realistically run into) outweighs the edge case of people changing .name properties manually (which seems unrealistic outside of tests).

[yutakahirano]

I'm fine with using .name.