1227555 - JSObject::is() reports incorrect results for ProxyObject subclasses

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jon Coppeard (:jonco)]

For any ProxyObject subclasses Foo and Bar, fooInstance->is<Bar>() will return true.

This is because ProxyObject subclasses don't declare a class_ member, so the inherited ProxyObject::class_ is used instead.

I guess this is part of a wider problem for JSObject subclasses that don't declare their own class, but it's more obvious for ProxyObject subclasses because none of them declare class_ and they should all be considered different.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: bug1227555-proxy-object-is

Here's a patch which does the following:

1) Remove ProxyObject::class_.  This breaks any use of JSObject::is() for ProxyObject and derived classes.

2) Add a specialisation of is() for ProxyObject that does not apply to derived classes.

3) Adds js::IsDerivedProxyObject() which tests both the class and the handler.

4) Adds specialisations of is() for ModuleNamespaceObject and DebugScopeObject using the above.

Hopefully this is tidier and less error-prone.

Comment 2

[Shu-yu Guo [:shu]]

Comment on attachment 8691408 [details] [diff] [review]
bug1227555-proxy-object-is

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

This is a fine change.

::: js/src/vm/ProxyObject.h
@@ +105,5 @@
>      void nuke(const BaseProxyHandler* handler);
>  
> +    // There is no class_ member to force specialization of JSObject::is<T>().
> +    // The implementation in JSObject is incorrect for proxies since it doesn't
> +    // take account of the handler type.

Small nit: perhaps just rename class_? Like klass_ or something? I don't see reason to put it outside of the ProxyObject class entirely. (And keep this comment, of course).

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b61c35cebe4f

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b61c35cebe4f