Closed Bug 1000175 Opened 6 years ago Closed 6 years ago
.onerror catching events cross domain
Jacob, thank you for reporting this! That looks like a sanitized script error: the script that threw is not same-origin with the onerror handler. That's why the error text and line number are hidden; the only thing left is the filename. It looks to me like we could in fact get in this state this in the following situation: 1) User loads page A. 2) User clicks link to page B. 3) User hits back button. 4) Exception thrown on page B, async error reporter event posted. 5) Navigation back to page A completes. 6) Error event fires. because the ScriptErrorEvent doesn't check whether the inner window when it was posted matches the inner window when it's firing. Seems to me like it should, no? mScriptGlobal in this case is an _outer_ window, fwiw.
Component: Security → DOM
Boris, Thank you for the quick response. Your explanation seems like exactly what happened. I was missing steps #5 and #6. The only problem is I still can't seem to reproduce, because page A loads faster than I can hit link to page B. I will try to put a sleep in and see if I can reproduce. Jacob
Jacob, I wouldn't worry about trying to reproduce this. I think our code here is clearly buggy.
What behavior do we want with document.write?
That's a good question. open() blows away event listeners on the window, so I would argue we want an actual inner window identity check, not an active document check.
I don't see anything truly async there, but a script runner. Sure, that is enough to cause this issue though.
Hmm. If it's just a script runner that makes the scenario in comment 1 less likely. :( We should still fix, but there might be something else going on.
Yeah, it seems like ScriptErrorEvent should hold onto the inner, rather than the outer, and do some kind of check against the current inner. There are two bits of behavior that I'm not sure about though: * Do we want to invoke the old onerror, or nothing at all? * Do we want to report to the outer window's console, or not at all?
> * Do we want to invoke the old onerror, or nothing at all? I would say nothing at all. Typically an event listener wouldn't run in a non-current inner anyway. >* Do we want to report to the outer window's console, or not at all? I think we do want to report to the outer's console. Make sense?
Status: UNCONFIRMED → NEW
Ever confirmed: true
(In reply to Boris Zbarsky [:bz] (on PTO, reviews are slow) from comment #9) > Make sense? SGTM.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Jacob, can you reproduce this reliably enough that you could test this patch if I pointed you to some binaries (Linux 64-bit, right?) that include it?
Comment on attachment 8422022 [details] [diff] [review] Make sure error events on window only fire if the active document has not changed from when the exception was thrown Why do we need mScriptGlobal anymore? mWindow should be enough for getting docshell
Attachment #8422022 - Flags: review?(bugs) → review-
Right you are!
Attachment #8425241 - Attachment description: test.patch → Interdiff from first patch
Comment on attachment 8425240 [details] [diff] [review] Updated patch The whole error dispatching code is a bit ugly :/
Attachment #8425240 - Flags: review?(bugs) → review+
Flags: needinfo?(jacob_champlin) → in-testsuite?
Whiteboard: [need review]
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.