contentWindow.location (and href) throws DOM security error

VERIFIED FIXED in mozilla1.8beta2



14 years ago
13 years ago


(Reporter: bugzilla-mozilla-20000923, Assigned: bzbarsky)


Dependency tree / graph

Firefox Tracking Flags

(Not tracked)



(2 attachments, 1 obsolete attachment)



14 years ago
Since the latest change in bug 281988, ChatZilla's been having a little problem
with getting the location of its content windows.

To get the error, in any very recent Firefox build, open ChatZilla and do this:
  /eval this.frame.contentWindow.location.href

It seems  /eval this.frame.contentWindow shows that the Window object is being
wrapped by XPCNativeWrapper (which I'm told is perfectly correct), however the
exception is not.
The problem here is that we end up in XPC_NW_NewResolve, decide we need to
delegate to the unwrapped object, go to do the OBJ_DEFINE_PROPERTY() property
thing.  This calls into nsWindowSH::AddProperty, which throws for the "location"
property.  So just getting window.location fails.

So the problem, it seems, is that the 

  // All we need to do is define the property in obj if it exists in
  // the wrapped native's object.

comment isn't what we're doing.  We're trying to define the property in the
wrapped native (because our AddProperty hook just passes things along here).

I tried just skipping the OBJ_DEFINE_PROPERTY call, but that breaks other things...

Perhaps we should consider forwarding to the other class hook first, and if that
doesn't resolve anything doing what we do now?  Or would that not work?
Flags: blocking1.8b2?
OS: Windows 2000 → All
Hardware: PC → All


14 years ago
Blocks: 295040
Not enough... document.location has the same issue.  And the AddProperty hook on
nsNodeSH does weird wrapper-preserve stuff, like I said on IRC.

It really feels like we don't want to be calling AddProperty here when the
property is "already there".  The question is whether we can detect this last case.

I guess we can do this for now if we have no better ideas...
Created attachment 184262 [details] [diff] [review]
Patch, per discussion with brendan

brendan says r+a=him
Attachment #184257 - Attachment is obsolete: true
Attachment #184262 - Flags: review+


14 years ago
Assignee: general → bzbarsky
Target Milestone: --- → mozilla1.8beta2
Last Resolved: 14 years ago
Resolution: --- → FIXED

Comment 6

14 years ago
Verified in trunk Firefox, dated 2005-05-22 23:30:05.76 BST.
Created attachment 184290 [details] [diff] [review]
followup fix for correct delete property bypass

I'd like to get this in now, since we know the JSClass.delProperty forwarding
is not useful (delProperty, like addProperty, is a notification callback that
does not actually remove the id'd property).

Attachment #184290 - Flags: review?(bzbarsky)
Attachment #184290 - Flags: approval1.8b2+
Comment on attachment 184290 [details] [diff] [review]
followup fix for correct delete property bypass

Attachment #184290 - Flags: superreview+
Attachment #184290 - Flags: review?(bzbarsky)
Attachment #184290 - Flags: review+
Comment on attachment 184290 [details] [diff] [review]
followup fix for correct delete property bypass

Checked in, thanks.



14 years ago
Flags: blocking1.8b2?
You need to log in before you can comment on or make changes to this bug.