869003 - Cannot inspect objects from cross-domain iframes

Status: RESOLVED FIXED

(DevTools :: Console, defect)

Description

[Mihai Sucan [:msucan]]

Since bug 783499 landed if you call console.log(foo) in an iframe from that is loaded from a different domain than the top level document, the |foo| object cannot be inspected. When you click the object in console's output you see a "permission denied" error in the Browser Console.

Comment 1

[Mihai Sucan [:msucan]]

Attachment: proposed fix

This patch changes the web console actor in such a way that it does addDebuggee() for every new window global it encounters from console API log calls or from evals. It also changes evaluation to happen in the global window object that belongs to the object the user is working with in variables view - this avoids permission denied errors when viewing or changing properties.

The new test lives in browser/ rather than toolkit/ because the toolkit tests run in chrome privileges and I was not able to reproduce the permission denied error with cross-domain iframes.

Try push:
https://tbpl.mozilla.org/?tree=Try&rev=473c77469862

Comment 2

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 746608 [details] [diff] [review]
proposed fix

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

::: toolkit/devtools/webconsole/dbg-webconsole-actors.js
@@ +269,5 @@
> +      try {
> +        global = Cu.getGlobalForObject(aValue);
> +      }
> +      catch (ex) {
> +        // The above can throw an exception if aValue is not an actual object.

should we be putting this error on the console or something? Hiding exceptions can be annoying.

@@ +793,2 @@
>        let jsObj = bindSelf.unsafeDereference();
> +      let global = global = Cu.getGlobalForObject(jsObj);

why the global = global here?

@@ +977,5 @@
>    {
>      let result = WebConsoleUtils.cloneObject(aMessage);
>      delete result.wrappedJSObject;
>  
> +    result.arguments = Array.map(aMessage.arguments || [], (aObj) => {

opportunistic arrow functioning.

Comment 3

[Rob Campbell [:rc] (:robcee)]

try looks good.

Comment 4

[Mihai Sucan [:msucan]]

(In reply to Rob Campbell [:rc] (:robcee) from comment #2)
> Comment on attachment 746608 [details] [diff] [review]
> proposed fix
> 
> Review of attachment 746608 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/devtools/webconsole/dbg-webconsole-actors.js
> @@ +269,5 @@
> > +      try {
> > +        global = Cu.getGlobalForObject(aValue);
> > +      }
> > +      catch (ex) {
> > +        // The above can throw an exception if aValue is not an actual object.
> 
> should we be putting this error on the console or something? Hiding
> exceptions can be annoying.

This is expected to be really common. I am going to add a typeof check to avoid this.


> 
> @@ +793,2 @@
> >        let jsObj = bindSelf.unsafeDereference();
> > +      let global = global = Cu.getGlobalForObject(jsObj);
> 
> why the global = global here?

Eh, editing artifact caused by several iterations of the code until I got things right.

Good catch!


> @@ +977,5 @@
> >    {
> >      let result = WebConsoleUtils.cloneObject(aMessage);
> >      delete result.wrappedJSObject;
> >  
> > +    result.arguments = Array.map(aMessage.arguments || [], (aObj) => {
> 
> opportunistic arrow functioning.

Yes. :)

Thanks for the review. Will update the patch and land it.

Comment 5

[Mihai Sucan [:msucan]]

Attachment: updated patch

Landed: https://hg.mozilla.org/integration/fx-team/rev/2441ed727cea

Comment 6

[Joe Walker [:jwalker]]

https://hg.mozilla.org/mozilla-central/rev/2441ed727cea