1610606 - Incorrect OOM handling in duck type case in ErrorReport::init()

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P2)

Description

[Andrew McCreight [:mccr8]]

As part of investigating bug 1608760, I'm looking at places that could cause JSErrorBase::filename to be null and I think I found one in ErrorReport::init(). One way to create an error object is via duck typing from a regular object that has the fields message, fileName (or filename) and lineNumber.

The code for that includes this:

if (JS_GetProperty(cx, exnObject, filename_str, &val)) {
  RootedString tmp(cx, ToString<CanGC>(cx, val));
  if (tmp) {
    filename = JS_EncodeStringToUTF8(cx, tmp);
...

The first thing JS_EncodeStringToUTF8 does is attempt to linearize tmp, which can fail on OOM. If it does, then filename will be null. I think this should do a null check and fail the entire operation if it happens.

I think it should be possible to trigger this condition with a shell test using oomTest. I'm not sure how to check if fileName is null, but maybe just checking for the empty string is enough.

This code is pretty old, so even if is sort of the cause of bug 1608760, a better fix is probably to make the exception serializing code more robust against a null file name. Who knows what other places might put null in that field.

Comment 1

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1610606 - Make js::ErrorReport::init always populate filename on success. r?nbp.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/82d250cf8fab
Make js::ErrorReport::init always populate filename on success. r=nbp.

Comment 3

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/82d250cf8fab