351007 - Crash [@ XPCWrappedNative::CallMethod] passing undefined to idl parameter of type out nsAString

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[[e-mail]]

Note: this example is taken from mozilla.dev.extensions so it targets 1.0.x
var f_h = Components.classes["@mozilla.org/satchel/form-history;1"].getService(Components.interfaces.nsIFormHistory);
var f_name;
var f_value;
f_h.getEntryAt (0, f_name, f_value);
nsIFormHistory.idl declares void getEntryAt(in unsigned long index, out AString name, out AString value);
I reproduced the crash with SeaMonkey trunk using the wallet service which includes WALLET_PreEdit(out AString walletList);
It crashes in XPCWrappedNative::CallMethod at the following line of code:
if(!OBJ_SET_PROPERTY(ccx, JSVAL_TO_OBJECT(argv[i]),
            rt->GetStringID(XPCJSRuntime::IDX_VALUE), &v))
presumably because argv[i] is zero. Apparently an out AString actually compiles to an in dipper AString which isn't null-checked like other out params are.

Comment 1

[[e-mail]]

Attachment: Possible patch

This fixes it for me, instead of crashing I now get an 0x80570002 nsresult.

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

(the comment 0 testcase passes undefined, not null)

Comment 3

[[e-mail]]

(In reply to comment #2)
>(the comment 0 testcase passes undefined, not null)
It's not as if it makes any difference - the result is the same with 0, undefined, null etc...

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

oh... last time I ran into this, only undefined was a problem IIRC.

Comment 5

[timeless]

Comment on attachment 236378 [details] [diff] [review]
Possible patch

seems reasonable to me

Comment 6

[Johnny Stenback (:jst)]

Marking blocker, I'll sr...

Comment 7

[Johnny Stenback (:jst)]

Comment on attachment 236378 [details] [diff] [review]
Possible patch

r+sr=jst, let's get this one in.

Comment 8

[[e-mail]]

Fix checked in.