crash in mozilla::dom::DOMRequest::Init

RESOLVED INVALID

Status

()

--
critical
RESOLVED INVALID
6 years ago
6 years ago

People

(Reporter: martijn.martijn, Unassigned)

Tracking

({crash, testcase})

Trunk
x86
Windows 7
crash, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mentor=jdm][lang=c++], crash signature)

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
Created attachment 663664 [details]
testcase

This bug was filed from the Socorro interface and is 
report bp-95b84569-d652-48a3-92ed-fed222120922 .
============================================================= 
0 	xul.dll 	mozilla::dom::DOMRequest::Init 	dom/base/DOMRequest.cpp:42
1 	xul.dll 	mozilla::dom::DOMRequest::DOMRequest 	dom/base/DOMRequest.cpp:25
2 	xul.dll 	mozilla::dom::DOMRequestService::CreateRequest 	dom/base/DOMRequest.cpp:201
3 	xul.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:70
4 	xul.dll 	XPCWrappedNative::CallMethod 	js/xpconnect/src/XPCWrappedNative.cpp:2405
5 	xul.dll 	XPC_WN_CallMethod 	js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1469
6 	mozjs.dll 	js::InvokeKernel 	js/src/jsinterp.cpp:367
7 	mozjs.dll 	js::Invoke 	js/src/jsinterp.h:109
8 	mozjs.dll 	js_fun_apply 	js/src/jsfun.cpp:951
9 	mozjs.dll 	js::InvokeKernel 	js/src/jsinterp.cpp:367
10 	mozjs.dll 	js::Interpret 	js/src/jsinterp.cpp:2454

I get this crash with SpecialPowers.getDOMRequestService().createRequest({});

Updated

6 years ago
Keywords: testcase
OS: Windows NT → Windows 7
Version: unspecified → Trunk

Comment 1

6 years ago
We don't check the return value of do_QueryInterface in DOMRequest::Init. We should probably just return early.
Whiteboard: [mentor=jdm][lang=c++]

Comment 2

6 years ago
Hi Matthews and Wargers,

I'm new to Mozilla and interested in this problem. I'm trying to contribute and need your help.

In this bug, the argument is invalid in the test case; createRequest() expects a window while the actual argument is an empty object. As noted by Matthews, this eventually results in dereferencing a null pointer. Since createRequest() is not exposed as a public API, should this be a fatal condition? If so, should we catch this kind of problems as early as possible? Maybe all we have to do is to add an assertion to shutdown immediately and gracefully.

Propagate the error to the JavaScript side is also possible but I'm afraid that the exceptions might sometimes be ignored and result in delayed bombs.

Would you please give me some advices? Thanks in advance!
We could "fix" this crash by doing what you say and throw an exception if createRequest is passed something that isn't a valid Window. However that wouldn't actually fix the bug here since the code still wouldn't function as intended, right?

So detecting the lack of a valid Window wouldn't actually fix the bug, just wallpaper over the crash.

Precisely because this isn't a public function do we rather crash than wallpaper over the problem. This way we can detect this bug through things like Socorro.

So what we should do here is to find the code which is calling createRequest without providing a valid window and then fix it to find a proper window.

Comment 4

6 years ago
Hi Sicking,

I totally agree with you. I'm sorry that I may not describe it clearly. The problem here is that in the test case there is only one line (of JavaScript):

SpecialPowers.getDOMRequestService().createRequest({});

The argument in question, {}, is hard-coded, given by the test case directly. So it may be arguable that the test case itself is legal or not.
That code looks totally wrong to me. If the only way to reproduce this crash is using that testcase then this bug should be closed as INVALID.
Making it so. The code in comment 0 is buggy, you need to provide a valid window.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.