1795102 - Add iid_is checking to XPIDL

Status: RESOLVED FIXED

(Core :: XPCOM, task, P2)

Description

[Andrew McCreight [:mccr8]]

The XPIDL frontend doesn't do any validity checking on the value of iid_is. jsonxpt.py will effectively silently drop the attribute if the value isn't a parameter name.

In bug 1794811, I changed a parameter name and not the iid_is value in QI like this:

void QueryInterface(in nsIIDRef aIID, [iid_is(uuid), retval] out nsQIResult aInstancePtr);

This seemed to call all QI calls in JS to just fail, which it turns out prevents the browser from functioning.

It looks like there is currently no checking for iid_is. We should check that the variable exists, and that it actually an nsIID. It also seems like a good idea to check that if it the param with the iid_is is an input param, then the dependent param is also an input.

I also added an assertion to jsonxpt.py so that it will fail rather than silently drop the parameter. With the checking I added in the front end, we shouldn't hit that code.

I also cleaned up the size_is checking (which does exist!), and added the input-ness checking.

Finally, I fixed a few other minor issues with error reporting.

Funnily enough, thanks to Bugzilla's possible duplicates checking, it looks like this exact issue was previously fixed for XPIDL (bug 11451 and bug 13413), but I guess it got dropped in the Python rewrite.

Comment 1

[Andrew McCreight [:mccr8]]

Attachment: Bug 1795102, part 1 - Fix some miscellaneous bugs for XPIDL front end errors.

Fix the error message for NameMap::get().

Use the location of the identifier in p_param. It was passing in
something that didn't return a valid location, causing bad error messages.
I'm guessing a new thing got added to this production rule without
adjusting the index.

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: Bug 1795102, part 2 - Make size_is checking use NameMap.

This also changes the location from the location of the method to the
parameter which seems better.

Make the other error message use p.location, and change the error
message to say "size_is" instead of "is_size".

Comment 3

[Andrew McCreight [:mccr8]]

Attachment: Bug 1795102, part 3 - Check that size_is parameter input-ness matches.

If I remembered anything from my logic programming days, I could
come up a smarter sounding word for this than "input-ness".

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: Bug 1795102, part 4 - Add iid_is checking.

Check that iid_is refers to a parameter, is an nsIID, and has compatible
input-ness.

Make attr_param_idx in jsonxpt.py assert if the expected parameter
is missing, rather than silently ignoring the attribute.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/29cb99921359
part 1 - Fix some miscellaneous bugs for XPIDL front end errors. r=nika
https://hg.mozilla.org/integration/autoland/rev/16bbe38229de
part 2 - Make size_is checking use NameMap. r=nika
https://hg.mozilla.org/integration/autoland/rev/7e9f774c0f77
part 3 - Check that size_is parameter input-ness matches. r=nika
https://hg.mozilla.org/integration/autoland/rev/43844d326e98
part 4 - Add iid_is checking. r=nika

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/29cb99921359
https://hg.mozilla.org/mozilla-central/rev/16bbe38229de
https://hg.mozilla.org/mozilla-central/rev/7e9f774c0f77
https://hg.mozilla.org/mozilla-central/rev/43844d326e98