Closed Bug 307983 Opened 19 years ago Closed 19 years ago

NS_ERROR_INVALID_POINTER when using location.* inside evalInSandbox

Categories

(Core :: XPConnect, defect, P2)

1.8 Branch
x86
Windows XP
defect

Tracking

()

VERIFIED FIXED
mozilla1.8rc1

People

(Reporter: boogs, Assigned: mrbkap)

Details

(Keywords: fixed1.8)

Attachments

(2 files, 2 obsolete files)

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);
Component: General → XPConnect
Product: Firefox → Core
Version: unspecified → 1.8 Branch
Attached file testcase
Bollocks, forgot attachment again.
Assignee: nobody → dbradley
QA Contact: general → pschwartau
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
Flags: blocking1.8rc1?
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.8rc1? → blocking1.8rc1+
Bad timing: Aaron's in Vietnam until the end of this week, I think.
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 :-(.
Assignee: dbradley → mrbkap
Priority: -- → P2
Target Milestone: --- → mozilla1.8rc1
I'm hoping that this fix will be evalInSandbox-specific (and thus low-risk).
Status: NEW → ASSIGNED
Attached patch Something like this (obsolete) — Splinter Review
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?
Attachment #199235 - Flags: review?(jst)
Whiteboard: [patch]
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.
Attachment #199235 - Flags: review?(jst) → review-
Whiteboard: [patch]
Attached patch updated to comments (obsolete) — Splinter Review
Attachment #199235 - Attachment is obsolete: true
Attachment #199607 - Flags: review?(jst)
Attachment #199607 - Flags: superreview?(brendan)
Comment on attachment 199607 [details] [diff] [review]
updated to comments

*sigh*, the iterator needs to go _backwards_ (iterating over a stack, and all).
Attachment #199607 - Attachment is obsolete: true
Attachment #199607 - Flags: superreview?(brendan)
Attachment #199607 - Flags: review?(jst)
This has r=jst looking over my shoulder.
Attachment #199619 - Flags: superreview?(brendan)
Attachment #199619 - Flags: review+
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
Attachment #199619 - Flags: superreview?(brendan) → superreview+
Attachment #199619 - Flags: approval1.8rc1?
Blake, we need this landed and verified on the trunk ASAP. Thanks.
Fix checked into the trunk (with another small tweak to make the iterator handle
an empty deque).
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Neither of my depends builds register the new iterator, which busts all sorts of
things :-( BTW, why is ContextStackIteror not spelled correctly?
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...
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 ;-)
Neil, is that a verification? Have you got a working build yet?
Status: RESOLVED → VERIFIED
Attachment #199619 - Flags: approval1.8rc1? → approval1.8rc1+
Checked into MOZILLA_1_8_BRANCH.
Keywords: fixed1.8
You need to log in before you can comment on or make changes to this bug.