Closed
Bug 1002313
Opened 11 years ago
Closed 11 years ago
Fix the test for bug 503926 so that it doesn't rely on nsIDOMNode being scriptable
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
Details
Attachments
(1 file)
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
Assignee | ||
Comment 1•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Attachment #8413510 -
Flags: review?(bzbarsky)
Comment 2•11 years ago
|
||
Comment on attachment 8413510 [details] [diff] [review]
Fix the test for bug 503926 so that it doesn't rely on nsIDOMNode being scriptable; r=bzbarsky
r=me if you add tests to all the catch clauses that the thrown exception is about the object not being an nsIDOMWindowUtils object or whatever the exception message is (e.g. to make sure that in test_bug503926.html we're not getting an exception from one of the bits before the last '.' in the try.
Attachment #8413510 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 3•11 years ago
|
||
So attempting to verify the exceptions in the test, I realized that none of these tests doesn't even throw in this case, and XpconnectArgument() ends up being called with an nsXPTCStubBase* like this:
3993 nsDOMWindowUtils::XpconnectArgument(nsIDOMWindowUtils* aThis)
3994 {
3995 // Do nothing.
-> 3996 return NS_OK;
3997 }
3998
3999 NS_INTERFACE_MAP_BEGIN(nsTranslationNodeList)
(lldb) p aThis
(nsXPTCStubBase *) $0 = 0x00000001300f0d80
(lldb) bt 10
* thread #1: tid = 0xcc45f, 0x00000001034c0671 XUL`nsDOMWindowUtils::XpconnectArgument(this=0x000000011e5981c0, aThis=0x00000001300f0d80) + 17 at nsDOMWindowUtils.cpp:3996, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
* frame #0: 0x00000001034c0671 XUL`nsDOMWindowUtils::XpconnectArgument(this=0x000000011e5981c0, aThis=0x00000001300f0d80) + 17 at nsDOMWindowUtils.cpp:3996
frame #1: 0x00000001016c3a89 XUL`NS_InvokeByIndex(that=0x000000011e5981c0, methodIndex=133, paramCount=1, params=0x00007fff5fbf2fe8) + 537 at xptcinvoke_x86_64_unix.cpp:162
frame #2: 0x00000001033d30a4 XUL`CallMethodHelper::Invoke(this=0x00007fff5fbf2fa0) + 84 at XPCWrappedNative.cpp:2392
frame #3: 0x00000001033c71e7 XUL`CallMethodHelper::Call(this=0x00007fff5fbf2fa0) + 263 at XPCWrappedNative.cpp:1733
frame #4: 0x00000001033a6092 XUL`XPCWrappedNative::CallMethod(ccx=0x00007fff5fbf3188, mode=CALL_METHOD) + 226 at XPCWrappedNative.cpp:1700
frame #5: 0x00000001033a86c6 XUL`XPC_WN_CallMethod(cx=0x0000000127d78480, argc=1, vp=0x00007fff5fbf3e80) + 918 at XPCWrappedNativeJSOps.cpp:1285
frame #6: 0x00000001064a1865 XUL`js::CallJSNative(cx=0x0000000127d78480, native=0x00000001033a8330, args=0x00007fff5fbf38c0)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) + 165 at jscntxtinlines.h:239
frame #7: 0x0000000106476fba XUL`js::Invoke(cx=0x0000000127d78480, args=CallArgs at 0x00007fff5fbf38c0, construct=NO_CONSTRUCT) + 1178 at Interpreter.cpp:475
frame #8: 0x00000001062889a3 XUL`js_fun_apply(cx=0x0000000127d78480, argc=2, vp=0x000000011256e210) + 1603 at jsfun.cpp:1020
frame #9: 0x00000001064a1865 XUL`js::CallJSNative(cx=0x0000000127d78480, native=0x0000000106288360, args=0x00007fff5fbf44f0)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) + 165 at jscntxtinlines.h:239
What should I do given that?
Flags: needinfo?(bzbarsky)
Comment 4•11 years ago
|
||
I think it's time to ask mrbkap what he expects to be happening here...
Flags: needinfo?(bzbarsky) → needinfo?(mrbkap)
Comment 5•11 years ago
|
||
(In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment #3)
> So attempting to verify the exceptions in the test, I realized that none of
> these tests doesn't even throw in this case, and XpconnectArgument() ends up
> being called with an nsXPTCStubBase* like this:
Now that you have a function that takes a (pretty much) arbitrary object, there won't be any more exceptions thrown. All that matters is whether we call QI on untrusted objects. That being said, the importance of the fix for bug 503926 is quickly diminishing with webidl. So I think you can get rid of the try/catches entirely.
Flags: needinfo?(mrbkap)
Assignee | ||
Comment 6•11 years ago
|
||
(In reply to comment #5)
> (In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment
> #3)
> > So attempting to verify the exceptions in the test, I realized that none of
> > these tests doesn't even throw in this case, and XpconnectArgument() ends up
> > being called with an nsXPTCStubBase* like this:
>
> Now that you have a function that takes a (pretty much) arbitrary object, there
> won't be any more exceptions thrown. All that matters is whether we call QI on
> untrusted objects. That being said, the importance of the fix for bug 503926 is
> quickly diminishing with webidl. So I think you can get rid of the try/catches
> entirely.
Sounds good, thanks!
Assignee | ||
Comment 7•11 years ago
|
||
Comment 8•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•