408697 - Certificate Exception dialog logs scary looking exception, even though nothing's wrong

Status: VERIFIED FIXED

(Core :: Security: PSM, defect)

Description

[Serge Gautherie (:sgautherie)]

Moved from bug 408432 comment 11:

{{{
Johnathan Nightingale   2007-12-17 08:55:45 PST

> > the JS Error Message is Error: [Exception... "Component returned failure code:
> > 0x80004005 (NS_ERROR_FAILURE) [nsIXMLHttpRequest.send]"  nsresult: "0x80004005
> > (NS_ERROR_FAILURE)"  location: "JS frame ::
> > chrome://pippki/content/exceptionDialog.js :: checkCert :: line 151"  data: no]
> > Source File: chrome://pippki/content/exceptionDialog.js
> > Line: 157

The exception is behaving as-designed (strange as that may seem).  See here:

http://mxr.mozilla.org/mozilla/source/security/manager/pki/resources/content/exceptionDialog.js#157

Though, if you found it really confusing, you might file a bug on improving the
log message there, to say something about it being expected - or maybe logging
it differently altogether?
}}}

Code (from bug 387181) is
{{
 153   } catch (e) {
 154     // We *expect* exceptions if there are problems with the certificate
 155     // presented by the site.  Log it, just in case, but we can proceed here,
 156     // with appropriate sanity checks
 157     Components.utils.reportError(e);
 158   } finally {
}}

Comment 1

[Serge Gautherie (:sgautherie)]

(In reply to comment #0)
> Code (from bug 387181) is

Rather bug 387480.

Comment 2

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9b5pre) Gecko/2008031801 SeaMonkey/2.0a1pre] (nightly) (W2Ksp4)

(Bug still there.)

Comment 3

[Johnathan Nightingale [:johnath]]

Attachment: More verbose log message

I don't think this blocks release, but it could be confusing, and it is very low-hanging fruit.

Comment 4

[Serge Gautherie (:sgautherie)]

Comment on attachment 310453 [details] [diff] [review]
More verbose log message

Now, that looks a little too verbose to me,
but I'd rather have too much than not enough.

I wonder if we could/want to report this as a Message (or may be Warning) rather than an Error ?
Moreover/Or, I wonder if that could/should be made "debug build only" ?

Comment 5

[Johnathan Nightingale [:johnath]]

(In reply to comment #4)
> (From update of attachment 310453 [details] [diff] [review])
> Now, that looks a little too verbose to me,
> but I'd rather have too much than not enough.
> 
> I wonder if we could/want to report this as a Message (or may be Warning)
> rather than an Error ?
> Moreover/Or, I wonder if that could/should be made "debug build only" ?

We don't want this for debug only, since the reason for logging it in the first place is that we think it's unlikely, but not impossible, that a "real" exception is being thrown here, and we don't want to lose all hope of catching that, if a user stumbles on to it.

We could change it to a warning or info message instead of error, using nsIConsoleService - I'm not sure what Kai prefers here, but I'll leave it to him, since he has a broader knowledge of how PSM reports things to the console.

Comment 6

[Damon Sicore (:damons)]

-'ing, but wanted1.9.0.x+.  Would be a nice to have, request approval once patch completed.  Still, won't block the release.

Comment 7

[Kai Engert (:KaiE:)]

Comment on attachment 310453 [details] [diff] [review]
More verbose log message

r=kengert, although this message will never get localized...

Comment 8

[Serge Gautherie (:sgautherie)]

(In reply to comment #7)
> (From update of attachment 310453 [details] [diff] [review])
> r=kengert, although this message will never get localized...

For now, that may help to get it in for Gecko 1.9, wrt l10n.
Should I file a followup bug for Gecko 2.0 ?

Comment 9

[Johnathan Nightingale [:johnath]]

(In reply to comment #8)
> (In reply to comment #7)
> > (From update of attachment 310453 [details] [diff] [review] [details])
> > r=kengert, although this message will never get localized...
> 
> For now, that may help to get it in for Gecko 1.9, wrt l10n.
> Should I file a followup bug for Gecko 2.0 ?

I agree - this is not worth the late-l10n hit, but I have no problem with it being localized in principle, unless we have a policy against localizing error messages.  My advice would be to file a followup, cc me, and maybe even tag the status whiteboard as "[good first bug]".  If some new volunteer doesn't get to it before I do, it should be a simple matter to add it to the right properties file, and retrieve it in js instead of hard coding.

Comment 10

[Johnathan Nightingale [:johnath]]

(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > (From update of attachment 310453 [details] [diff] [review] [details] [details])
> > > r=kengert, although this message will never get localized...
> > 
> > For now, that may help to get it in for Gecko 1.9, wrt l10n.
> > Should I file a followup bug for Gecko 2.0 ?
> 
> I agree - this is not worth the late-l10n hit, but I have no problem with it
> being localized in principle, unless we have a policy against localizing error
> messages.  My advice would be to file a followup, cc me, and maybe even tag the
> status whiteboard as "[good first bug]".  If some new volunteer doesn't get to
> it before I do, it should be a simple matter to add it to the right properties
> file, and retrieve it in js instead of hard coding.
> 

The followup bug might also want to address comment 4, where you talk about logging it at a different severity

Comment 11

[Damon Sicore (:damons)]

Comment on attachment 310453 [details] [diff] [review]
More verbose log message

a1.9+=damons

Comment 12

[Johnathan Nightingale [:johnath]]

Checking in security/manager/pki/resources/content/exceptionDialog.js;
/cvsroot/mozilla/security/manager/pki/resources/content/exceptionDialog.js,v  <--  exceptionDialog.js
new revision: 1.9; previous revision: 1.8
done

Comment 13

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9pre) Gecko/2008040715 SeaMonkey/2.0a1pre] (SEA-WIN32-TBOX-trunk) (W2Ksp4)

With <https://gmail.com/>,
{{
Error: Attempted to connect to a site with a bad certificate in the add exception dialog. This results in a (mostly harmless) exception being thrown. Logged for information purposes only: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIXMLHttpRequest.send]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://pippki/content/exceptionDialog.js :: checkCert :: line 151"  data: no]
Source File: chrome://pippki/content/exceptionDialog.js
Line: 159
}}

Although, whereas the source code is nicely on 3 lines,
the resulting "one endless line" console text is "difficult" to read. :-/

V.Fixed