Closed
Bug 398040
Opened 17 years ago
Closed 17 years ago
Calling a getter with an argument causes NS_InvokeByIndex_P to call a different function from the vtable
Categories
(Core :: XPConnect, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jruderman, Assigned: mrbkap)
Details
(Keywords: testcase)
Attachments
(3 files)
149 bytes,
application/vnd.mozilla.xul+xml
|
Details | |
9.08 KB,
text/plain
|
Details | |
630 bytes,
patch
|
brendan
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
The testcase has onload="window.__proto__.__lookupGetter__('frames')(null);" but it triggers an assertion in window.find(), bug 328412. How does that happen? Is something in xpconnect causing the wrong function to be called?
Flags: blocking1.9?
Reporter | ||
Comment 1•17 years ago
|
||
... #13 nsGlobalWindow::Find (this=0x3e254ef0, aDidFind=0xbfffc294) at /Users/jruderman/trunk/mozilla/dom/src/base/nsGlobalWindow.cpp:5479 #14 NS_InvokeByIndex_P (that=0x3e254f20, methodIndex=18, paramCount=1, params=0xbfffc294) at /Users/jruderman/trunk/mozilla/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_unixish_x86.cpp:179 #15 XPCWrappedNative::CallMethod (ccx=@0xbfffc500, mode=CALL_SETTER) at /Users/jruderman/trunk/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp:2326 #16 XPCWrappedNative::SetAttribute (ccx=@0xbfffc500) at /Users/jruderman/trunk/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp:2081 #17 XPC_WN_GetterSetter (cx=0x3e255090, obj=0x3d2ce180, argc=1, argv=0x3f15443c, vp=0xbfffc624) at /Users/jruderman/trunk/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp:1491 #18 js_Invoke (cx=0x3e255090, argc=1, vp=0x3f154434, flags=0) at /Users/jruderman/trunk/mozilla/js/src/jsinterp.c:1382 ...
Assignee | ||
Comment 2•17 years ago
|
||
This is a good catch. XPC_WN_GetterSetter assumes that if it's called with an argument, then it's being called as a setter. This is clearly not the case as Jesse's testcase shows. I think this is the simplest fix, although it means that given: window.__lookupGetter__('location')('http://www.google.com') we'll happily navigate to google.com instead of returning the location property. IMO this is enough of an edge case that I'm willing to let it slide. The wrong function is called here because the 'frames' property is read only, so there is no actual setter in the vtable, so we end up calling into whatever function happens to be there.
Assignee: nobody → mrbkap
Status: NEW → ASSIGNED
Attachment #282928 -
Flags: superreview?(jst)
Attachment #282928 -
Flags: review?(brendan)
Assignee | ||
Comment 3•17 years ago
|
||
(In reply to comment #2) > window.__lookupGetter__('location')('http://www.google.com') We're actually ok here because the methodIndex points to the getter anyway (and we'll do all the right checks to ensure that we pass the proper number of arguments).
Reporter | ||
Updated•17 years ago
|
Summary: NS_InvokeByIndex_P calls the wrong function? → Calling a getter with an argument causes NS_InvokeByIndex_P to call a different function from the vtable
Comment 4•17 years ago
|
||
Comment on attachment 282928 [details] [diff] [review] Proposed fix sr=jst
Attachment #282928 -
Flags: superreview?(jst) → superreview+
Updated•17 years ago
|
Attachment #282928 -
Flags: review?(brendan) → review+
Assignee | ||
Updated•17 years ago
|
Attachment #282928 -
Flags: approval1.9?
Flags: blocking1.9? → blocking1.9+
Assignee | ||
Comment 5•17 years ago
|
||
Fix checked into trunk.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 6•17 years ago
|
||
One way to test this: window.__proto__.__lookupGetter__('frames')(null) == window Can/should it be tested more directly?
Flags: in-testsuite?
Updated•17 years ago
|
Attachment #282928 -
Flags: approval1.9?
You need to log in
before you can comment on or make changes to this bug.
Description
•