NS_ERROR_INVALID_POINTER when using location.* inside evalInSandbox

VERIFIED FIXED in mozilla1.8rc1

Status

()

Core
XPConnect
P2
normal
VERIFIED FIXED
12 years ago
6 years ago

People

(Reporter: Aaron Boodman, Assigned: mrbkap)

Tracking

({fixed1.8})

1.8 Branch
mozilla1.8rc1
x86
Windows XP
fixed1.8
Points:
---
Bug Flags:
blocking1.8rc1 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

12 years ago
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);
(Reporter)

Updated

12 years ago
Component: General → XPConnect
Product: Firefox → Core
Version: unspecified → 1.8 Branch
(Reporter)

Comment 1

12 years ago
Created attachment 195604 [details]
testcase

Bollocks, forgot attachment again.
Assignee: nobody → dbradley
QA Contact: general → pschwartau

Comment 2

12 years ago
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?

Comment 3

12 years ago
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.
(Assignee)

Comment 5

12 years ago
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
(Assignee)

Comment 6

12 years ago
I'm hoping that this fix will be evalInSandbox-specific (and thus low-risk).
Status: NEW → ASSIGNED
(Assignee)

Comment 7

12 years ago
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?
Attachment #199235 - Flags: review?(jst)
(Assignee)

Updated

12 years ago
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-
(Assignee)

Updated

12 years ago
Whiteboard: [patch]
(Assignee)

Comment 9

12 years ago
Created attachment 199607 [details] [diff] [review]
updated to comments
Attachment #199235 - Attachment is obsolete: true
Attachment #199607 - Flags: review?(jst)
(Assignee)

Updated

12 years ago
Attachment #199607 - Flags: superreview?(brendan)
(Assignee)

Comment 10

12 years ago
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)
(Assignee)

Comment 11

12 years ago
Created attachment 199619 [details] [diff] [review]
with those changes

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+

Updated

12 years ago
Attachment #199619 - Flags: approval1.8rc1?

Comment 13

12 years ago
Blake, we need this landed and verified on the trunk ASAP. Thanks.
(Assignee)

Comment 14

12 years ago
Fix checked into the trunk (with another small tweak to make the iterator handle
an empty deque).
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Comment 15

12 years ago
Neither of my depends builds register the new iterator, which busts all sorts of
things :-( BTW, why is ContextStackIteror not spelled correctly?
(Assignee)

Comment 16

12 years ago
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

12 years ago
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

12 years ago
Neil, is that a verification? Have you got a working build yet?

Updated

12 years ago
Status: RESOLVED → VERIFIED

Updated

12 years ago
Attachment #199619 - Flags: approval1.8rc1? → approval1.8rc1+
(Assignee)

Comment 19

12 years ago
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.