(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #5) > Now what's unclear to me is whether `JS::CreateError` should handle a null filename (like it handles a null message, if you dig down into things, at the level of `js::ErrorObject::init`) or not. Jason, any opinions? It should not. There are two problem to fix here: 1. something is producing invalid serialized data somehow 2. deserialization must either accept or reject all serialized data, not crash To fix problem 2, we should probably take mccr8's first patch. A more principled patch would treat that condition as invalid serialized input and refuse to create an Error. On to problem 1: > If it should, then we should fix that. If it should not, we need to figure out what to do in `ClonedErrorHolder::ToErrorValue` if `mFilename` is void. Which is its default state (see `ClonedErrorHolder::ClonedErrorHolder`) until either `ClonedErrorHolder::Init` happens with a non-null `err->filename` or we do a `ReadStructuredClone`. > > Now in this case, we have `ClonedErrorHolder::ReadStructuredClone` on the stack, so presumably the thing we serialized on the other end had a null filename for some reason... Presumably because _it_ was initialized from a `JSErrorReport` with a null filename? We could add a release assertion on initialization and try to catch this closer to the source.
Bug 1608760 Comment 18 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #5) > Now what's unclear to me is whether `JS::CreateError` should handle a null filename (like it handles a null message, if you dig down into things, at the level of `js::ErrorObject::init`) or not. Jason, any opinions? It should not. There are two problems to fix here: 1. something is producing invalid serialized data somehow 2. deserialization must either accept or reject all serialized data, not crash To fix problem 2, we should probably take mccr8's first patch. A more principled patch would treat that condition as invalid serialized input and refuse to create an Error. On to problem 1: > If it should, then we should fix that. If it should not, we need to figure out what to do in `ClonedErrorHolder::ToErrorValue` if `mFilename` is void. Which is its default state (see `ClonedErrorHolder::ClonedErrorHolder`) until either `ClonedErrorHolder::Init` happens with a non-null `err->filename` or we do a `ReadStructuredClone`. > > Now in this case, we have `ClonedErrorHolder::ReadStructuredClone` on the stack, so presumably the thing we serialized on the other end had a null filename for some reason... Presumably because _it_ was initialized from a `JSErrorReport` with a null filename? We could add a release assertion on initialization and try to catch this closer to the source.