1263765 - remove the nsIBadCertListener2 implementation in exceptionDialog.js to avoid spamming of "Error: 2147500034 exceptionDialog.js"

Status: RESOLVED FIXED

(Core :: Security: PSM, defect, P1)

Description

[Dana Keeler (she/her) [:keeler]]

The nsIBadCertListener2 implementation in exceptionDialog.js is unnecessary and causes some debug when necko tries to QI it to nsILoadContext. We can just replace it with an onerror handler on the XHR.

Comment 1

[Kamil Jozwiak [:kjozwiak]]

Attachment: error.png

I'm assuming this is the same issue. I've ran into the following error several times via the browser console:

Error: 2147500034 exceptionDialog.js:32:11

badCertListener.prototype.QueryInterfacechrome://pippki/content/exceptionDialog.js:32:11
badCertListener.prototype.getInterfacechrome://pippki/content/exceptionDialog.js:23:12
checkCertchrome://pippki/content/exceptionDialog.js:110:43
anonymouschrome://global/content/bindings/dialog.xml%20line%20380%20%3E%20Function:1:1
_fireButtonEventchrome://global/content/bindings/dialog.xml:381:28
_doButtonCommandchrome://global/content/bindings/dialog.xml:349:28
_handleButtonCommandchrome://global/content/bindings/dialog.xml:337:18
BrowserOnClick.onCertErrorchrome://browser/content/browser.js:2890:9
BrowserOnClick.receiveMessagechrome://browser/content/browser.js:2794:9

STR:

* open https://untrusted-root.badssl.com/
* open the browser console
* click on Advanced -> Add Exception
* Once you receive the exceptionDialog, click on "Get Certificate" and you'll several of the above errors

Build Used:
* fx51.0a1, buildId: 20160810030202, changeset: 6cf0089510fa

Comment 3

[Dana Keeler (she/her) [:keeler]]

Attachment: bug 1263765 - remove nsIBadCertListener2 implementation from the add certificate exception dialog

This reworks the certificate-fetching portion of the add certificate exception
dialog so as to not require a nsIBadCertListener2 implementation, which is
deprecated. The solution is simple: use the onerror/onload callbacks on the
XMLHttpRequest object to grab the appropriate information.

Review commit: https://reviewboard.mozilla.org/r/212210/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/212210/

Comment 4

[J.C. Jones [:jcj] (he/they)]

Comment on attachment 8942008 [details]
bug 1263765 - remove nsIBadCertListener2 implementation from the add certificate exception dialog

https://reviewboard.mozilla.org/r/212210/#review218314

\ /
 |
/o\

Comment 5

[Dana Keeler (she/her) [:keeler]]

Comment on attachment 8942008 [details]
bug 1263765 - remove nsIBadCertListener2 implementation from the add certificate exception dialog

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/212210/diff/1-2/

Comment 6

[Dana Keeler (she/her) [:keeler]]

Had to add some docstrings to make eslint happy, but otherwise looks good:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6bd7f826050c
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a3295fda7fe2c5fa5053b867bf6008e265d2602

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d5dd45fa551c
remove nsIBadCertListener2 implementation from the add certificate exception dialog r=jcj

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/d5dd45fa551c