Last Comment Bug 307983 - NS_ERROR_INVALID_POINTER when using location.* inside evalInSandbox
: NS_ERROR_INVALID_POINTER when using location.* inside evalInSandbox
Status: VERIFIED FIXED
: fixed1.8
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: 1.8 Branch
: x86 Windows XP
: P2 normal (vote)
: mozilla1.8rc1
Assigned To: Blake Kaplan (:mrbkap)
: Phil Schwartau
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-09-10 23:31 PDT by Aaron Boodman
Modified: 2011-08-05 22:29 PDT (History)
11 users (show)
asa: blocking1.8rc1+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (346 bytes, text/html)
2005-09-10 23:33 PDT, Aaron Boodman
no flags Details
Something like this (1.87 KB, patch)
2005-10-11 17:30 PDT, Blake Kaplan (:mrbkap)
jst: review-
Details | Diff | Splinter Review
updated to comments (16.29 KB, patch)
2005-10-14 15:18 PDT, Blake Kaplan (:mrbkap)
no flags Details | Diff | Splinter Review
with those changes (16.22 KB, patch)
2005-10-14 18:17 PDT, Blake Kaplan (:mrbkap)
mrbkap: review+
brendan: superreview+
asa: approval1.8rc1+
Details | Diff | Splinter Review

Description Aaron Boodman 2005-09-10 23:31:56 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4

When you access what appears to be any property of the location object (I am
interested mainly in .href and .replace) from within evalInSandbox, you get an
NS_ERROR_INVALID_POINTER error.

For instance, with this code:

var box = new Components.utils.Sandbox(location.href);
box.window = window;
Components.utils.evalInSandbox("alert(window.location.href)", box);

Reproducible: Always

Steps to Reproduce:
1. Open the attachment in FF
2. Say yes to the security prompt
3. Note error in javascript console

Actual Results:  
Error in JS Console:

Error: uncaught exception: [Exception... "Component returned failure code:
0x80004003 (NS_ERROR_INVALID_POINTER) [nsIDOMLocation.replace]"  nsresult:
"0x80004003 (NS_ERROR_INVALID_POINTER)"  location: "JS frame ::
file:///C:/Documents%20and%20Settings/aa/Desktop/evalInSandbox.html ::
<TOP_LEVEL> :: line 8"  data: no]

Expected Results:  
alert box with URL of current window

I noticed that for some reason you don't get the error if you wrap the code that
access the location object in a window.setTimeout call.

Not sure if that is helpful to you at all. So for instance, this *does not* have
the error:

Components.utils.evalInSandbox("window.setTimeout(function(){alert(window.location.href)})",
box);
Comment 1 Aaron Boodman 2005-09-10 23:33:19 PDT
Created attachment 195604 [details]
testcase

Bollocks, forgot attachment again.
Comment 2 Ian Macfarlane 2005-10-11 07:49:19 PDT
Should this block 1.5 final as it is stopping GreaseMonkey from working properly?

See: http://greaseblog.blogspot.com/2005/09/firefox-15-compatible-greasemonkey.html
Comment 3 Asa Dotzler [:asa] 2005-10-11 15:56:18 PDT
sounds like a major regression that impacts a popular extension. Pulling into
the blocking radar, but time is really short and we don't yet have anyone signed
up to investigate and fix. Aaron, if you can help us find resources, or figure
out exactly what broke this, that'd be a big help.
Comment 4 Phil Ringnalda (:philor) 2005-10-11 16:02:57 PDT
Bad timing: Aaron's in Vietnam until the end of this week, I think.
Comment 5 Blake Kaplan (:mrbkap) 2005-10-11 16:11:03 PDT
I'll try to pull this in. I'm hoping there's going to be a simple fix for this.
I really wish that I'd known about this weeks ago :-(.
Comment 6 Blake Kaplan (:mrbkap) 2005-10-11 16:18:47 PDT
I'm hoping that this fix will be evalInSandbox-specific (and thus low-risk).
Comment 7 Blake Kaplan (:mrbkap) 2005-10-11 17:30:03 PDT
Created attachment 199235 [details] [diff] [review]
Something like this

This, sadly, isn't evalInSandbox-specific. It gets the current window off of
the docshell if it can't get it off of the context. I think this can only
happen in the "extensions" case. jst, what do you think?
Comment 8 Johnny Stenback (:jst, jst@mozilla.com) 2005-10-13 16:12:00 PDT
Comment on attachment 199235 [details] [diff] [review]
Something like this

r- based on discussion with mrbkap. This would make us use the wrong URL when
as the base when resolving relative URLs on location objects from a window
other than the callers.
Comment 9 Blake Kaplan (:mrbkap) 2005-10-14 15:18:19 PDT
Created attachment 199607 [details] [diff] [review]
updated to comments
Comment 10 Blake Kaplan (:mrbkap) 2005-10-14 16:52:16 PDT
Comment on attachment 199607 [details] [diff] [review]
updated to comments

*sigh*, the iterator needs to go _backwards_ (iterating over a stack, and all).
Comment 11 Blake Kaplan (:mrbkap) 2005-10-14 18:17:21 PDT
Created attachment 199619 [details] [diff] [review]
with those changes

This has r=jst looking over my shoulder.
Comment 12 Brendan Eich [:brendan] 2005-10-15 00:17:44 PDT
Comment on attachment 199619 [details] [diff] [review]
with those changes

I nagged mrbkap on IRC about not sharing the common return from
GetContextFromStack, allowing the break to become a return NS_OK and the final
return to be immediately preceded by an unconditional *aContext = nsnull.

/be
Comment 13 Asa Dotzler [:asa] 2005-10-17 11:03:35 PDT
Blake, we need this landed and verified on the trunk ASAP. Thanks.
Comment 14 Blake Kaplan (:mrbkap) 2005-10-17 11:48:13 PDT
Fix checked into the trunk (with another small tweak to make the iterator handle
an empty deque).
Comment 15 neil@parkwaycc.co.uk 2005-10-18 05:22:09 PDT
Neither of my depends builds register the new iterator, which busts all sorts of
things :-( BTW, why is ContextStackIteror not spelled correctly?
Comment 16 Blake Kaplan (:mrbkap) 2005-10-19 02:11:16 PDT
FYI, I fixed the misspelling (thanks copy/paste) on the trunk today. Neil, I'm
not sure why your builds are failing. I can't reproduce that problem either on
my Linux build or my Windows build...
Comment 17 neil@parkwaycc.co.uk 2005-10-19 05:17:03 PDT
Seems to have been some weird compreg.dat issue, it didn't reregister properly.
Deleting compreg.dat fixed that and also found some obsolete components ;-)
Comment 18 Asa Dotzler [:asa] 2005-10-19 11:16:41 PDT
Neil, is that a verification? Have you got a working build yet?
Comment 19 Blake Kaplan (:mrbkap) 2005-10-19 15:20:43 PDT
Checked into MOZILLA_1_8_BRANCH.

Note You need to log in before you can comment on or make changes to this bug.