1412445 - gMainPane.QueryInterface doesn't properly check its aIID argument

Status: RESOLVED FIXED

(Firefox :: Settings UI, enhancement)

Description

[Myk Melez [:myk] [@mykmelez]]

Attachment: replace custom QI impl with XPCOMUtils.generateQI call

gMainPane.QueryInterface doesn't properly check its aIID argument because of mis-encapsulation of its conditionals:

  QueryInterface(aIID) {
    if (aIID.equals(Ci.nsIObserver) ||
      aIID.equals(Ci.nsIDOMEventListener ||
        aIID.equals(Ci.nsISupports)))
      return this;

    throw Cr.NS_ERROR_NO_INTERFACE;
  },

The refactoring in bug 1349689 <https://hg.mozilla.org/mozilla-central/rev/9db0cec976bb>, which changed the indentation of the lines, made this more obvious; but it didn't introduce the problem.

It looks like I introduced it 10 years ago in bug 377784 <https://hg.mozilla.org/mozilla-central/rev/045452b30d72da2f45f0a89ce9247227b39dc38c>.

In any case, the fix is straightforward and can be made even safer with XPCOMUtils.generateQI.  (I wonder if we should do a more general search/replace for QueryInterface implementations that could use XPCOMUtils.generateQI.)

Comment 1

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8923020 [details] [diff] [review]
replace custom QI impl with XPCOMUtils.generateQI call

Review of attachment 8923020 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. thanks!

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/726e06daf2d7
replace custom QI impl with XPCOMUtils.generateQI call; r=rickychien

Comment 3

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/726e06daf2d7