586157 - Decide what to do about settable __proto__ on the window object

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Proposed fix

Allowing this only leads to pain. It's been mostly broken for years now. We should break it all the way and see if anybody notices (hint: they won't).

gal, I think we need to imitate this in the new wrappers too.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Can we disallow this on all DOM objects?  It'd make some things much simpler.... and we want immutable proto chains anyway long-term, right?

Comment 2

[Brendan Eich [:brendan]]

(In reply to comment #1)
> Can we disallow this on all DOM objects?  It'd make some things much
> simpler.... and we want immutable proto chains anyway long-term, right?

It's anyone's guess what on the web or behind a firewall that we might care about would break. Skimming the first few hits here is not encouraging but someone with time for a closer reading may see good news in fine print:

http://www.google.com/codesearch?q=lang%3Ajs+%22__proto__+%3D+%22&hl=en&btnG=Search+Code

/be

Comment 3

[Boris Zbarsky [:bzbarsky]]

None of the hits there seem to be changing __proto__ on DOM objects except possibly chatzilla...

Comment 4

[Brendan Eich [:brendan]]

How many pages did you look at?

What does WebKit do?

We could try it. Better to do in nightlies before starting a beta trek. It's hard to say whether it'll go over easily. It could, especially if WebKit doesn't let you hack on its DOM objects' __proto__ properties.

/be

Comment 5

[Boris Zbarsky [:bzbarsky]]

> How many pages did you look at?

Just the first page.

> What does WebKit do?

JSC and V8 both seem to allow setting window.__proto__.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

In bug 586083, I've attached a patch to just deal with settable proto for now. We can use this bug to decide what to do about settable proto at our leisure.

Comment 7

[Brendan Eich [:brendan]]

Whatever we do about settable __proto__, we should try to do in tandem with JSC. V8 should follow suit. I'll check with Jens at Opera.

Anyone know what IE9 Chakra does, if anything, here?

/be

Comment 8

[Brendan Eich [:brendan]]

Post Firefox 4 era, using Future TM rather than unset to indicate preference to fix this.

Could morph this bug into one about settable __proto__ on any object, not just on window objects. I couldn't find an existing bug on making __proto__ non-writable.

/be

Comment 9

[Brendan Eich [:brendan]]

Opera 10.61 on Mac handles this test fully:

<script>
    o = {};
    p = {q: 42};
    o.__proto__ = p;
    alert(o.q);
    this.__proto__ = p;
    alert(q);
</script>

We'll need John McClane to come back for "Die Hard 5" to get __proto__ killed for good, I'm afraid.

/be

Comment 10

[Brendan Eich [:brendan]]

The test in comment 9 is nasty, and Safari at least fails to find alert via the scope and prototype chains, since this.__proto__ = p; succeeds in redirecting the window object's proto chain to p, which has only Object.prototype as its proto.

Minefield (Firefox 4.0b4pre) as discussed throws on the |this.__proto__ = p;| attempt.

Opera must do something non-prototype-delegation-based to find alert.

/be

Comment 11

[Brendan Eich [:brendan]]

Fixing the test:

<script>
    a = alert;
    o = {};
    p = {q: 42};
    o.__proto__ = p;
    a(o.q);
    this.__proto__ = p;
    a(q);
</script>

gets two alerts, both showing "42", from Safari 5.0.1.

/be

Comment 12

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to comment #10)
> The test in comment 9 is nasty, and Safari at least fails to find alert via the
> scope and prototype chains, since this.__proto__ = p; succeeds in redirecting
> the window object's proto chain to p, which has only Object.prototype as its
> proto.

Confusingly, when I did a similar test, doing |window.__proto__ = null; window.alert('hi');| alerted 'hi' instead of throwing as I would have expected.

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

(in Safari, I mean)

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

Note: The patch I wrote for bug 586083 still points to this bug in its checkin message.

Comment 15

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/26930725aa63