Closed Bug 629331 Opened 14 years ago Closed 13 years ago

javascript document.domain and parent in iframes

Categories

(Core :: XPConnect, defect)

Product:
Core
Component:
XPConnect
Platform:
x86_64
Windows 7
Type:
defect
Priority:
Not set
Severity:
major

Tracking

()

Status:
VERIFIED FIXED
Tracking Flags:
Tracking Status
blocking2.0 --- final+

People

(Reporter: thorstengiese, Assigned: mrbkap)

References

(
URL
)

Details

(Whiteboard: [hardblocker][has patch][fx4-fixed-bugday])

Attachments

(2 files)

Mochitest
3.37 KB, patch
Details | Diff | Splinter Review
Proposed fix
4.70 KB, patch
gal
: review+
brendan
: review+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US) AppleWebKit/534.10 (KHTML, like Gecko) Chrome/8.0.552.237 Safari/534.10
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b10) Gecko/20100101 Firefox/4.0b10

We have a web application, that does not work in Firefox4. I have reproduced the code which leads to the error. I have an script in one domain e.g. "playground.anw.de" and the source of an iframe in onther e.g. "www.anw.de". I set document.domain to anw.de in both scripts and then change a javascript variable outside the iframe via parent. For the first time parent.i = 1 works pretty well, but the following parent.i = 2 gets ignored. It works fine, if both scripts reside in the same domain. 

Reproducible: Always

Steps to Reproduce:
http://playground.anw.de/tg/ff4bug/index.html:
<!DOCTYPE html> 
<html> 
  <head> 
  </head> 
  <body> 
    <iframe id="foo" src="#"></iframe> 
 
    <div id="result"> 
    </div><!-- result --> 
 
    <script> 
      document.domain = "anw.de";
      var i = 0;
      document.getElementById('result').innerHTML += "i before iframe src change: "+i+"<br />";
      document.getElementById('foo').src='http://www.anw.de/ff4bug/iframe.html?i='+Math.random();
    </script> 
 
  </body> 
</html> 


http://www.anw.de/ff4bug/iframe.html:

<!DOCTYPE html>
<html>
  <head>
  </head>
  <body>
    <script>
      document.domain = "anw.de";
      parent.i = 1;
      parent.document.getElementById('result').innerHTML += "i after iframe src change: "+parent.i+"<br />";
      parent.i = 2;
      parent.document.getElementById('result').innerHTML += "i after iframe src change: "+parent.i+"<br />";
    </script>
  </body>
</html>

Actual Results:  
i before iframe src change: 0
i after iframe src change: 1
i after iframe src change: 1

Expected Results:  
i before iframe src change: 0
i after iframe src change: 1
i after iframe src change: 2

This works fine in FF3.6, Chrome 8.0.552.237 and any other browser I tested.
Product: Firefox → Core
Component: General → DOM
QA Contact: general → general
I was able to reproduce this once but it has been working as expected most of the time. I don't know why it failed once.
I can reproduce this bug with the given scripts on any execution on an Ubuntu/Linux 10.10 with firefox-4.0b10 downloaded as a tar.bz2 and on the already mentioned windows7/ff4.0b10. It does not fail occasionally here but anytime.
Update/Correction: if I start the browser fresh I can reproduce it every time, for 2-4 requests. If the browser has done a few requests, it suddenly works as expected. In our production environment where it has lots of more updates and many variables are affected it fails more often.
This sounds really bad.  Andreas, Blake?
blocking2.0: --- → ?
Component: DOM → XPConnect
QA Contact: general → xpconnect
blocking2.0: ? → final+
We have to try to catch this in gdb. Doesn't look hard to fix once we know whats up.
QA Contact: xpconnect → gal
Doesn't seem to affect a ton of sites in the wild, so I will go with softblocker for now but I will start looking at this.
Whiteboard: [softblocker]
Assignee: nobody → gal
QA Contact: gal → xpconnect
I think I saw this once but I can't reproduce it ever since. Super weird. Looks like a timing issue.
I tried this with b10 on linux, I still can't reproduce this reliably.
I just reproduced this again. I get this error in the error console:

Error: parent.document.getElementById("result") is null
Source File: http://www.anw.de/ff4bug/iframe.html
Line: 9
Status: UNCONFIRMED → NEW
Ever confirmed: true
I don't think this is a wrapper issue. This looks more like a race condition to me. Depending on in which order the iframes load this works or not. bz?
I think the first load of the page is more likely to file because all subsequent loads hit the cache.
The result div is in this case just a helper div to show the output. I get the
same behaviour with alert() instead of working with the result div. Also the
Warning might give a hint, what is wrong. In the Firefox Error-Console I see
now error, when it is happening. 

I just tested again on linux, and can still reproduced it. If the browser is
restarted, it happens every first page view and randomly after the 3-4 page
view. 

What makes me wonder is, why it is only with crossdomain iframes, and not with iframes from the same domain; maybe an DNS issue, when the DNS lookup delays the load of the iframe?

If I help with anything to get this bug fixed, just let me know.
I *might* be running into the same bug, or perhaps a related document.domain bug, though I can only reproduce it with NoScript installed: http://forums.informaction.com/viewtopic.php?f=10&t=5748
Andreas, the result div thing is a race in the page, yes, but it's not what this bug is about....  Though, a version of the testcase that doesn't race on that div might be nice.
ivan, can you reliably reproduce this? the permission error is what we are after
Hello there, 

I made a v2 version which is closer to our application, using document.write instead of an extra div and can still reproduce the bug on Win7 acutally in a "used" browser, with every reload at the moment. I hope that helps: 

http://playground.anw.de/tg/ff4bugv2/

index.html:
<!DOCTYPE html>
<html>
  <head>
  </head>
  <body>
    <iframe id="foo" src="#"></iframe>
    <script>
      document.domain = "anw.de";
      var i = 0;
      document.write("step 0: initial set i to 0, i = "+i+"<br />");
      document.getElementById('foo').src='http://www.anw.de/ff4bugv2/iframe.html?i='+Math.random();
    </script>

  </body>
</html>


iframe.html:
<!DOCTYPE html>
<html>
  <head>
  </head>
  <body>
    <script>
      document.domain = "anw.de";

      for (var j = 1; j <= 9; j++) {
        parent.i = j;
        document.write("step "+j+": set parent.i to "+j+", parent.i = "+parent.i+"<br />");
      }
    </script>
  </body>
</html>

*** /var/www/anw.de/www/ff4bugv2 ***


outside the iframe

step 0: initial set i to 0, i = 0

inside the iframe

step 1: set parent.i to 1, parent.i = 1
step 2: set parent.i to 2, parent.i = 1
step 3: set parent.i to 3, parent.i = 1
step 4: set parent.i to 4, parent.i = 1
step 5: set parent.i to 5, parent.i = 1
step 6: set parent.i to 6, parent.i = 1
step 7: set parent.i to 7, parent.i = 1
step 8: set parent.i to 8, parent.i = 1
step 9: set parent.i to 9, parent.i = 1
Doesn't reproduce for me on mac either. I will try Linux.

(Thanks for all the test cases Thorsten, I am not saying you are making this up, I am just saying I have trouble reproducing this reliably =) )

If Linux doesn't work I will try windows. What executable did you use?
On Windows, I downloaded from the betapage "Firefox Setup 4.0 Beta 10.exe" - filesize: 11.669.496 Bytes ... on linux I used the same downloadpage, though the linux version. 

If you need more information, you have to tell me, where to find it. As I said before, the Linux (Ubuntu 10.10) version and the Windows (Win7 64Bit) version behave the same for me and for both given examples.
did you use the 32-bit or 64-bit linux version?
32 bit
uname -a reads

Linux alderaan 2.6.35-24-generic #42-Ubuntu SMP Thu Dec 2 01:41:57 UTC 2010 i686 GNU/Linux

More Information: 

I just reenabled my plugins because I wanted to surf a bit using ff4; I then missed Firebug and installed version 1.7X.0a9 (1.6 is just for 3.6 at the time) and since then it is reproduceable with every request, not just sometimes.
Even if Firebug is not enabled.
more experiments on Windows: If I activate and deactivate FireBug in the Add-On Menu it doesn't change the current behaviour (firebug activated, happens everytime, firebug deactivated, happens often but not everytime). I have to close Firefox and restart it in order to change the behaviour. 

To clarify my last comment #21: With enabled, I do not mean activated/deactivated in the Add-On Menu, I mean it doesn't influence the results, if the statusbar icon is grey (disabled) or colored and the Firebug GUI is visible (enabled).
No luck with 32-bit linux, 64-bit linux or 64-bit mac.
Neither Blake nor I can reproduce this. I tried Windows b10 as well. Lets see if QA has more luck.
Keywords: qawanted
Attached patch Mochitest — — Splinter Review 
Here's a mochitest that attempts to reproduce this bug. I haven't successfully reproduced it, though :-(.
We might have reproduced this after all.
Keywords: qawanted
Fix upcoming. This is a bad bug that we need to fix -> hardblocker.
Assignee: gal → mrbkap
Whiteboard: [softblocker] → [hardblocker]
Depends on: 630716
Attached patch Proposed fix — — Splinter Review 
There are two separate parts to this fix:

* We need to use JSPROP_SHARED to determine if a property has a slot. We can't use the fact that there was a getter.
* We need to censor null -> PropertyStub to avoid creating setters where one didn't originally exist. In particular, we create a "fast expando" on the inner window object for the var declaration, but accidentally end up using the class setter after the first time setting it through the Xray wrapper.
Attachment #508955 - Flags: review?(gal)
Attachment #508955 - Flags: review?(brendan)
Should this be fixed for b11?

/be
Attachment #508955 - Flags: review?(gal) → review+
There is some risk to this patch. It might screw up b11. If we are willing to take that chance, yeah, would be nice to have.
(In reply to comment #30)
> There is some risk to this patch. It might screw up b11. If we are willing to
> take that chance, yeah, would be nice to have.

That says "no". "Nice to have" is a malediction.

I was asking if we needed to bake in b11, or unbreak some websites sooner than b12. Sounds like not.

/be
Comment on attachment 508955 [details] [diff] [review]
Proposed fix

>+        if (desc.setter && ((desc.attrs & JSPROP_SETTER) || desc.setter != PropertyStub)) {
>+            if (!CallSetter(cx, receiver, id, desc.setter, desc.attrs, desc.shortid, vp))
>+                return false;
>+            if (desc.attrs & JSPROP_SHARED)
>+                return true;
>+        }
>+        if (!desc.getter)
>+            desc.getter = PropertyStub;
>+        if (!desc.setter)
>+            desc.setter = PropertyStub;

This is confusing (maybe not buggy after all; see recent comments in bug 597109 and please do find or write that missing test, and add it). Better to common the if (!desc.setter):

..        if (!desc.setter) {
..            desc.setter = PropertyStub;
..        } else if ((desc.attrs & JSPROP_SETTER) || desc.setter != PropertyStub) {
..            if (!CallSetter(cx, receiver, id, desc.setter, desc.attrs, desc.shortid, vp))
..                return false;
..            if (desc.attrs & JSPROP_SHARED)
..                return true;
..        }
..        if (!desc.getter)
..            desc.getter = PropertyStub;

Same below:

>     if (desc.obj) {
>-        if (desc.setter && ((desc.attrs & JSPROP_SETTER) || desc.setter != PropertyStub))
>-            return CallSetter(cx, receiver, id, desc.setter, desc.attrs, desc.shortid, vp);
>         if (desc.attrs & JSPROP_READONLY)
>             return true;
>+        if (desc.setter && ((desc.attrs & JSPROP_SETTER) || desc.setter != PropertyStub)) {
>+            if (!CallSetter(cx, receiver, id, desc.setter, desc.attrs, desc.shortid, vp))
>+                return false;
>+            if (desc.attrs & JSPROP_SHARED)
>+                return true;
>+        }
>+        if (!desc.getter)
>+            desc.getter = PropertyStub;
>+        if (!desc.setter)
>+            desc.setter = PropertyStub;

r=me with that.

/be
Attachment #508955 - Flags: review?(brendan) → review+
Whiteboard: [hardblocker] → [hardblocker][has patch]
Blocks: 630159
These bugs are not connected.
Fix landed (fix + test):

http://hg.mozilla.org/mozilla-central/rev/d37f71235123
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
I'll address brendan's review comment tomorrow. Sorry for forgetting to address it earlier.
Duh, sorry for missing the fact that this patch needed tweaks :(
Verified fix in Mozilla/5.0 (Windows NT 6.1; rv:2.0b11) Gecko/20100101 Firefox/4.0b11
Status: RESOLVED → VERIFIED
Whiteboard: [hardblocker][has patch] → [hardblocker][has patch][fx4-fixed-bugday]
Blocks: 631725
Build identifier: Mozilla/5.0 (Windows NT 6.1; rv:2.0b12pre) Gecko/20110206 Firefox/4.0b12pre

Confirming issue is fixed.
I can confirm, with Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110209 Firefox/4.0b12pre the problem seems fixed for our web application thank you very much!
I have just downloaded the latest FireFox 4.0 RC and I am still facing this issue.
This is a huge bug.

I am trying this on a Windows 7 machine
(In reply to comment #41)
> I have just downloaded the latest FireFox 4.0 RC and I am still facing this
> issue.
> This is a huge bug.
> 
> I am trying this on a Windows 7 machine

Can you provide a testcase for the problem you are seeing?
The URL http://vivu.tv/portal/Join?flow=726-726-5652&name=abcd&email=abcd%40e.com&Submit=Join+Event&mode=133&t=conference connects to jabber by loading it in an iframe. 
When you open two or more instances of the URL in firefox 3.x you can see that the user count to the bottom right increases (number of instances - 1). 

But in 4.0 the jabber never connects. We have debugged this and found that it is because of the document.domain issues when loaded via iframe.

This could be a huge bummer
If you have firebug in 3.x you can see that there are continuous http-bind calls at the end. In firefox 4.0 RC you wouldnt see that
Works for me in Firefox 4 RC 1 on Windows.  I see N+1 users where N is the number of tabs I have open (I assume the last user is you.)
Kyle, The problem is that its not always reproducible. I will keep testing it more so that I can get you the details of the exact scenario
Please open a new bug for this.
(and please cc me)
We are able to reproduce this. Posted the details at https://bugzilla.mozilla.org/show_bug.cgi?id=643591

Lets continue all the discussion in that thread
Able to reproduce this 100% of the time.
The new link to test is http://vivu.tv/portal/Join?flow=608-353-9306&name=abcd&email=abcd%40e.com&Submit=Join+Event
did anyone get a chance to look at this.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: