Closed Bug 1176568 Opened 9 years ago Closed 9 years ago

window.__proto__ = <anything> -> "InternalError: too much recursion" / blocks browser

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: caitpotter88, Assigned: bzbarsky)

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_3) AppleWebKit/600.6.3 (KHTML, like Gecko) Version/8.0.6 Safari/600.6.3

Steps to reproduce:

assign any value whatsoever to the __proto__ property of the WindowProxy


Actual results:

InternalError: too much recursion / freezes the browser

Subsequent attempts to read __proto__ will block the browser in an infinite loop due to cycles in prototype chain


Expected results:

Trying to set a cyclic value to the prototype should just throw a TypeError and not modify the value
bholley, what is expected WRT DOM and __proto__ nowadays?
Flags: needinfo?(bobbyholley)
Note that I'm working on making the entire global object prototype chain immutable in...I think it's bug 1052139.  Working through an issue in bootstrapping, for sandbox globals, first, tho.
Waldo is currently the most knowledgeable about this subject, I think.
Flags: needinfo?(jwalden+bmo)
Flags: needinfo?(bobbyholley)
Flags: needinfo?
> assign any value whatsoever to the __proto__ property of the WindowProxy
....
> Subsequent attempts to read __proto__ will block the browser in an infinite loop

I just tried this testcase:

  window.__proto__ = {};
  alert(window.__proto__);

and it works just fine both in a current nightly and in Firefox 38.

Now I _can_ reproduce a problem with window.__proto__ = Object.create(window): this allows the set to happen (which it should not) and property accesses then throw "too much recursion"....  But no hang either way.
Flags: needinfo?
This was broken in Nightly a few days ago, but it seems to be not crashing in 41.0a1 (2015-06-23)
So in terms of the code flow, we end up in js::SetPrototype, obj->hasLazyPrototype() tests true, we land in js::DirectProxyHandler::setPrototype, that lands us back in js::SetPrototype for the inner, and then the loop there doesn't work correctly because it's comparing things up the proto chain to "obj" but "obj" is now the inner.  We need to outerize before the comparison loop.
Assignee: nobody → bzbarsky
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(jwalden+bmo)
Comment on attachment 8625468 [details] [diff] [review]
Fix the proto cycle checking to work correctly for inner/outer globals

Review of attachment 8625468 [details] [diff] [review]:
-----------------------------------------------------------------

Bah.  There must be a better way to make inner/outer objects not frail and buggy like this, somehow.

::: js/src/jsobj.cpp
@@ +2864,5 @@
>          return result.fail(JSMSG_CANT_SET_PROTO);
>  
> +    /* ES6 9.1.2 step 6 forbids generating cyclical prototype chains. But we
> +       have to do this comparison on the observable outer objects, not on the
> +       possibly-inner object we're setting the proto on. */

/*
 * ...
 * ...
 */

or

// ...
// ...
Attachment #8625468 - Flags: review?(jwalden+bmo) → review+
https://hg.mozilla.org/mozilla-central/rev/57a8f2e1d1e2
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.