1497488 - OCSP stapling error reported as "unknown reason"

Status: RESOLVED FIXED

(Thunderbird :: Security, defect)

Description

[bjoern]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:62.0) Gecko/20100101 Firefox/62.0

Steps to reproduce:

with a smtp auth server with a OCSP-must-staple certificate thunderbird refuses to connect when no stapled OCSP status was sent by the server. So far so good but the actual error message is very bad:



Actual results:

Sending of the message failed.
The message could not be sent using Outgoing server (SMTP)
smtp.samba.org for an unknown reason. Please verify that your Outgoing
server (SMTP) settings are correct and try again




Expected results:

Thunderbird should say what the technical reason is and not say "unknown reason" because Thunderbird knows the reason.

Comment 1

[Jorg K (CEST = GMT+2)]

Better than bug 1497491 ;-) - Gene you feel like digging through the bowels of error handling here? It may be that a generic error is returned from the Mozilla platform.

Excuse the ignorance; can such a certificate be used to reproduce a similar issue in Firefox. Yes, it doesn't do SMTP but it does do some other protocols.

Comment 2

[gene smith]

Speaking of ignorant, I know zero about "OCSP stapling" errors or certificates either. I would probably need to somehow duplicate the problem. If I just tried to user smtp.samba.org for outgoing mail would this error occur?

Comment 3

[bjoern]

if you define an outgoing mail server smtp.samba.org on port 1466 with SSL/TLS you can see that error message, yes.

Comment 4

[gene smith]

If I try to send email using that server/port I see the error you quote above. Looking at the the TLS transaction with wireshark the main error I see is "Bad Certificate" like this:

Secure Sockets Layer
  TLSv1.2 Record Layer: Alert (Level: Fatal, Description: Bad Certificate)
    Content Type: Alert (21)
    Version: 1.2 (0x0303)
    Length: 18
    Alert message
      Level: Fatal (2)
      Description: Bad Certificate (42)

I don't see anything about "staples", "staping" or "OCSP" in the error text. Given this error response, are you just saying maybe a more helpful error message would be something like:

"Sending of the message failed.
The message could not be sent using Outgoing server (SMTP)
smtp.samba.org due to Bad Certificate. Please verify that your Outgoing
server (SMTP) settings are correct and try again" ?

Comment 5

[bjoern]

the "Bad Certificate" is sent from Thunderbird to the server to terminate the connection because Thunderbird saw that a required TLS feature is missing. *This* information should actually be displayed in the error message. See the error response that you get when you go on the mentioned server and port with firefox via https.

Comment 6

[gene smith]

(In reply to bjoern from comment #5)
> the "Bad Certificate" is sent from Thunderbird to the server to terminate
> the connection because Thunderbird saw that a required TLS feature is
> missing. *This* information should actually be displayed in the error
> message. See the error response that you get when you go on the mentioned
> server and port with firefox via https.

When I put https://smtp.samba.org:1466 in firefox address bar I get 

Secure Connection Failed

An error occurred during a connection to smtp.samba.org:1466. A required TLS feature is missing. Error code: MOZILLA_PKIX_ERROR_REQUIRED_TLS_FEATURE_MISSING 

So a better tb message would be:

Sending of the message failed.
The message could not be sent using Outgoing server (SMTP)
smtp.samba.org because a required TLS feature is missing at the server. 
Error code: MOZILLA_PKIX_ERROR_REQUIRED_TLS_FEATURE_MISSING.
Please notify the server provider of this problem.

Not sure that the internal mozilla error code is helpful but at least this tells the user that the problem is at the server and not something configured wrong in tb.

Comment 7

[bjoern]

the error code MOZILLA_PKIX_ERROR_OCSP_RESPONSE_FOR_CERT_MISSING should generate a similar response to the user (the test setup mentioned above is changed to generate that error now by sending a different certificate's ocsp status).

Comment 8

[gene smith]

Yes, now I see:

Secure Connection Failed

An error occurred during a connection to smtp.samba.org:1466. The OCSP response does not include a status for the certificate being verified. Error code: MOZILLA_PKIX_ERROR_OCSP_RESPONSE_FOR_CERT_MISSING 

bjoern, I assume you work on smtp.samba.org setup? Just curious.

Anyhow, I see that the text printed on the error page, e.g.,

The OCSP response does not include a status for the certificate being verified.

is the localized version of the error code MOZILLA_PKIX_ERROR_OCSP_RESPONSE_FOR_CERT_MISSING and tb should do something similar. I will check if that is possible for tb to obtain this error response and print the configured string instead of just "unknown reason".

Comment 9

[gene smith]

The "unknown reason" response tb gets back from mozilla framework occurs when the socket/stream has been closed. The error code returned for smtp.samba.org:1466 currently is 0x805a3ff8. This is encoded as:
0x8000 0000   Severity is error
0x0045 0000   Error base +
0x0015 0000   Security error
0x0000 3ff8   -(MOZILLA_PKIX_ERROR_BASE + 8) = (0x4000 - 8) where 8 is for PKIX_ERROR_OCSP_RESPONSE_FOR_CERT_MISSING

This is one of about 300+ possible security errors that mozilla can return. A localization string would have to added to tb code to handle each of these. I don't know enough about the possible errors that can occur during smtp send to know which of them are not relevant, if any are not. Here is a list of all the possible security related error strings defined in mozilla:

https://dxr.mozilla.org/comm-central/source/security/manager/locales/en-US/chrome/pipnss/nsserrors.properties

There are 3 general security categories: SSL..., SEC... and MOZILLA_PKIX...  I have no idea which are relevant or can actually occur when sending mail, other than the PKIX errors that the reporter pointed out.

I don't know how to or if it's even possible to use these existing mozilla strings in tb. If only the MOZILLA_PKIX... codes are needed it wouldn't be too bad to just add them to tb.

A possible very simple fix would be to just add a single new string to tb that says "The SMTP server %s has produced a security related error. Please inform the server provider that there is a problem." This would only occur when the error code is security related. Anything else would still print the generic "unknown reason".

Or more ambitious:

"The SMTP server %s has produced a security related error code 0x%X. Please see http:<error lookup site> to decode the error and then inform the server provider of the problem."

A lookup site like http://james-ross.co.uk/mozilla/misc/nserror might be provided. (This one does not decode 0x805a3ff8 error all the way to text, but it tries.)

Comment 10

[gene smith]

Attachment: improved-send-msg-error.png

I have found a way to fix the problem that doesn't require any external error code lookup. It uses the existing mozilla security strings. It does add a new localization string (actually 2, 1 for tb and the same string for suite).

The original smtp and smtp server specific errors will print as before. If the error is security related, the string from mozilla will print, i.e., one of these
https://dxr.mozilla.org/comm-central/source/security/manager/locales/en-US/chrome/pipnss/nsserrors.properties
followed by a new string containing the smtp server name,
"Please inform the owner of SMTP server %S about this security issue.".
And if not security related but a mailnews error, just the string corresponding to the error will print like before. 
And finally, if not security or mailnews error, the "unknown error" message with the server name encoded will print.

Previously the "unknown error" message printed for any error that wasn't mailnews, i.e., security errors.

The attachment shows the current message when accessing smtp.samba.org at port 1466. I will attach a patch soon.

Comment 11

[gene smith]

Attachment: smtp-sec-string-addition.diff

Here's a diff that shows my proposed changes described in the previous comment. I may want to add the raw error code to the "unknown error" string, see "XXX" in diff.

Comment 12

[Magnus Melin [:mkmelin] (away, back Aug 4)]

Comment on attachment 9017771 [details] [diff] [review]
smtp-sec-string-addition.diff

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

::: suite/locales/en-US/chrome/mailnews/compose/composeMsgs.properties
@@ +28,5 @@
>  errorQueuedDeliveryFailed=An error occurred while delivering unsent messages.
>  sendFailed=Sending of the message failed.
>  
> +## LOCALIZATION NOTE (smtpSecurityIssue): argument %S is the Outgoing smtp server name
> +smtpSecurityIssue=\nPlease inform the owner of SMTP server %S about this security issue.

perhaps just AppendLiteral("\n") in the code instead.

Regarding the string: I think it's good that we say which smtp server it is. But in general, the "Please contact the administrator" is... who does that? I think it's also possible these security errors are due to an incorrect choice of authentication etc. and you may be able to resolve it by changing your settings. (Not for this specific issue, but in general)

Comment 13

[gene smith]

(In reply to Magnus Melin [:mkmelin] from comment #12)
> 
> perhaps just AppendLiteral("\n") in the code instead.

That's how I originally had it but thought why add code for something that always gets appended to an existing string as a new line?
But it does look weird having a string start with \n when none of the others do. So I will put back the AppendLiteral("\n") if that's what you want.

> 
> Regarding the string: I think it's good that we say which smtp server it is.
> But in general, the "Please contact the administrator" is... who does that?

So you are saying not to recommend that the tb user may need to contact the server admin/owner. Ok.

> I think it's also possible these security errors are due to an incorrect
> choice of authentication etc. and you may be able to resolve it by changing
> your settings. (Not for this specific issue, but in general)

That's probably true. Since there about 300+ possible security errors strings that can occur. Maybe a better string would be:

smtpSecurityIssue=Please verify that your Outgoing Server (SMTP) security and authentication settings are correct and try again.\n
The problem may also be caused by improper configuration of SMTP server %S.

Comment 14

[bjoern]

(In reply to gene smith from comment #13)
> > I think it's also possible these security errors are due to an incorrect
> > choice of authentication etc. and you may be able to resolve it by changing
> > your settings. (Not for this specific issue, but in general)
> 
> That's probably true. Since there about 300+ possible security errors
> strings that can occur. Maybe a better string would be:
> 
> smtpSecurityIssue=Please verify that your Outgoing Server (SMTP) security
> and authentication settings are correct and try again.\n
> The problem may also be caused by improper configuration of SMTP server %S.

I would not put any literature text like that. All this is not affecting wrong password settings or wrong port settings but only dedicated TLS errors. It would help users and server admins most if there would be the textual error code (MOZILLA_PKIX_ERROR_REQUIRED_TLS_FEATURE_MISSING, MOZILLA_PKIX_ERROR_OCSP_RESPONSE_FOR_CERT_MISSING, ...) in the error message. A hex code instead of a textual error code would be quite ugly though. Firefox must have a nice way to display the textual error code withouth having 300 if statements for that. Can't TB borrow the right idea from there?

Comment 15

[gene smith]

(In reply to bjoern from comment #14)
> 
> I would not put any literature text like that. All this is not affecting
> wrong password settings or wrong port settings but only dedicated TLS
> errors. 

I don't say check you password or server address/port but your "security and authentication settings" when the mozilla security related errors occur. 

> It would help users and server admins most if there would be the
> textual error code (MOZILLA_PKIX_ERROR_REQUIRED_TLS_FEATURE_MISSING,
> MOZILLA_PKIX_ERROR_OCSP_RESPONSE_FOR_CERT_MISSING, ...) in the error
> message. A hex code instead of a textual error code would be quite ugly
> though. Firefox must have a nice way to display the textual error code
> withouth having 300 if statements for that. Can't TB borrow the right idea
> from there?

I am printing the exact mozilla defined strings now with my patch. I am not printing the mozilla error name since it seems a bit redundant. Two examples for the mozilla file linked above:

MOZILLA_PKIX_ERROR_SIGNATURE_ALGORITHM_MISMATCH=The signature algorithm in the signature field of the certificate does not match the algorithm in its signatureAlgorithm field.
MOZILLA_PKIX_ERROR_OCSP_RESPONSE_FOR_CERT_MISSING=The OCSP response does not include a status for the certificate being verified.

So if error MOZILLA_PKIX_ERROR_SIGNATURE_ALGORITHM_MISMATCH occurs, tb, with my fix, will print this in the pop-up:

Sending of the message failed.
The signature algorithm in the signature field of the certificate does not match the algorithm in its signatureAlgorithm field.
Please verify that your Outgoing Server (SMTP) security and authentication settings are correct and try again.
The problem may also be caused by improper configuration of SMTP server smtp.samba.org.

This doesn't require 300 if statements and stringg. It uses the existing mozilla strings and access function.

Basic non-security mailnews (tb) smtp related errors will print the same message they always have (e.g., wrong address or port).

I am considering only putting in a hex code if I can't determine a matching string to print, i.e., when error unknown.

Comment 16

[bjoern]

now I see, thank you, those look perfect!

Comment 17

[Magnus Melin [:mkmelin] (away, back Aug 4)]

(In reply to gene smith from comment #13)
> > I think it's also possible these security errors are due to an incorrect
> > choice of authentication etc. and you may be able to resolve it by changing
> > your settings. (Not for this specific issue, but in general)
> 
> That's probably true. Since there about 300+ possible security errors
> strings that can occur. Maybe a better string would be:
> 
> smtpSecurityIssue=Please verify that your Outgoing Server (SMTP) security
> and authentication settings are correct and try again.\n
> The problem may also be caused by improper configuration of SMTP server %S.

Since we won't know if it's a server side error or not (but likely is), perhaps just be vague about it like

"The configuration related to smtp.samba.org must be corrected."

Comment 18

[gene smith]

Do you want the add-on string definition to be short like this:

smtpSecurityIssue=The configuration related to %S must be corrected.

or more verbose like this this:

smtpSecurityIssue=Please verify that your Outgoing Server (SMTP) security and authentication settings are correct and try again.\nThe configuration related to %S must be corrected.

So with just the short string (that I suspect you want) the full message would be like this example:

Sending of the message failed.
The signature algorithm in the signature field of the certificate does not match the algorithm in its signatureAlgorithm field.
The configuration related to smtp.samba.org must be corrected.

Or more verbose like this example:

Sending of the message failed.
The signature algorithm in the signature field of the certificate does not match the algorithm in its signatureAlgorithm field.
Please verify that your Outgoing Server (SMTP) security and authentication settings are correct and try again.
The configuration related to smtp.samba.org must be corrected.

Just want to be sure I understand what you want.

Comment 19

[Magnus Melin [:mkmelin] (away, back Aug 4)]

(In reply to gene smith from comment #18)
> Sending of the message failed.
> The signature algorithm in the signature field of the certificate does not
> match the algorithm in its signatureAlgorithm field.
> The configuration related to smtp.samba.org must be corrected.

Yes this shorter version would be my suggestion.

Comment 20

[Jorg K (CEST = GMT+2)]

Comment on attachment 9017771 [details] [diff] [review]
smtp-sec-string-addition.diff

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

Nice job of digging out the security errors and bringing them to the surface. Sorry about the delay.

::: mail/locales/en-US/chrome/messenger/messengercompose/composeMsgs.properties
@@ +28,5 @@
>  errorQueuedDeliveryFailed=An error occurred while delivering the unsent messages.
>  sendFailed=Sending of the message failed.
>  
> +## LOCALIZATION NOTE (smtpSecurityIssue): argument %S is the Outgoing smtp server name
> +smtpSecurityIssue=\nPlease inform the owner of SMTP server %S about this security issue.

Frankly, I'd make this super-short, like so:
  SMTP server %S.
No \n, no well meant suggestions.

::: mailnews/compose/src/nsMsgSend.cpp
@@ +3503,5 @@
>          aExitCode == NS_ERROR_SMTP_AUTH_CHANGE_ENCRYPT_TO_PLAIN_NO_SSL ||
>          aExitCode == NS_ERROR_SMTP_AUTH_CHANGE_ENCRYPT_TO_PLAIN_SSL ||
>          aExitCode == NS_ERROR_SMTP_AUTH_CHANGE_PLAIN_TO_ENCRYPT ||
> +        aExitCode == NS_ERROR_STARTTLS_FAILED_EHLO_STARTTLS)
> +    {

I know that in Mailnews the coding style is to put braces onto separate lines, but if they are not, please leave it that way.

@@ +3504,5 @@
>          aExitCode == NS_ERROR_SMTP_AUTH_CHANGE_ENCRYPT_TO_PLAIN_SSL ||
>          aExitCode == NS_ERROR_SMTP_AUTH_CHANGE_PLAIN_TO_ENCRYPT ||
> +        aExitCode == NS_ERROR_STARTTLS_FAILED_EHLO_STARTTLS)
> +    {
> +      // Print basic smtp error with smtp server name included.

SMTP

@@ +3510,3 @@
>      }
> +    else
> +    {

And here.

@@ +3512,5 @@
> +    {
> +      nsCOMPtr<nsINSSErrorsService> nsserr = do_GetService(NS_NSS_ERRORS_SERVICE_CONTRACTID);
> +      if (!nsserr ||  NS_FAILED(nsserr->GetErrorMessage(aExitCode, eMsg)))
> +      {
> +        // Not a server security issue as determined by mozilla

... Mozilla.

@@ +3521,5 @@
> +          mComposeBundle->GetStringFromName(exitString, eMsg);
> +        }
> +        else
> +        {
> +          // Print "unknown error" string and encode smtp server name.

SMTP

@@ +3525,5 @@
> +          // Print "unknown error" string and encode smtp server name.
> +          exitString = errorStringNameForErrorCode(NS_ERROR_SMTP_SEND_FAILED_UNKNOWN_REASON);
> +          FormatStringWithSMTPHostNameByName(exitString, eMsg);
> +          // XXX: This could also include the "unknown" error code (aExitCode)
> +          // encode in hex. Requires a new function definition probably.

Let's lose this comment.

@@ +3530,5 @@
> +        }
> +      }
> +      else
> +      {
> +        // Print mozilla security message with appended string pointing out the

Mozilla

::: suite/locales/en-US/chrome/mailnews/compose/composeMsgs.properties
@@ +28,5 @@
>  errorQueuedDeliveryFailed=An error occurred while delivering unsent messages.
>  sendFailed=Sending of the message failed.
>  
> +## LOCALIZATION NOTE (smtpSecurityIssue): argument %S is the Outgoing smtp server name
> +smtpSecurityIssue=\nPlease inform the owner of SMTP server %S about this security issue.

No idea why everyone is commenting on the suite/ code here. The TB/Mailnews code is above ;-)

Comment 21

[gene smith]

Jorg, Thanks for the comments. I was just about ready to submit a formal patch.

> +smtpSecurityIssue=\nPlease inform the owner of SMTP server %S about this security issue.

>Frankly, I'd make this super-short, like so:
>  SMTP server %S.
> No \n, no well meant suggestions.

I have updated this per Magnus's requests discussed in Comment 19, Comment 18 and Comment 17.

>I know that in Mailnews the coding style is to put braces onto separate lines, but if they are not, please leave it that way.

Most places in the file else is on its own line and matching braces are in the same column except at this one spot. Leaving it the way it was made it almost unreadable (at least to me) since I had to add several more if/else clauses to it. So I think the way I now have it makes the code easier to read.

>... Mozilla.
>SMTP

Ok, fixed the case for these.

> +          // XXX: This could also include the "unknown" error code (aExitCode)
> +          // encode in hex. Requires a new function definition probably.
>
>Let's lose this comment.

Comment is now gone since I am now handling the case where the error code has no matching string so I just show the hex error code. No new function was required.

I will post the updated patch ASAP.

Comment 22

[gene smith]

Attachment: 1497488-show-mozilla-security-errors(v1).patch

I added one unrelated fix to the function that finds the error string, in
mailnews/compose/src/nsComposeStrings.cpp. I noticed that the error string "illegalLocalPart" was never referenced anywhere. The correct string, that *is* reference, is "errorIllegalLocalPart". So I changed it. Hope that's OK.

Comment 23

[Jorg K (CEST = GMT+2)]

Comment on attachment 9018937 [details] [diff] [review]
1497488-show-mozilla-security-errors(v1).patch

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

Looks OK with a few changes required. Also NS_ERROR_SEND_FAILED does not seem to occur:
https://searchfox.org/comm-central/search?q=NS_ERROR_SEND_FAILED&redirect=false
I suggest removing it.

::: mail/locales/en-US/chrome/messenger/messengercompose/composeMsgs.properties
@@ +28,5 @@
>  errorQueuedDeliveryFailed=An error occurred while delivering the unsent messages.
>  sendFailed=Sending of the message failed.
>  
> +## LOCALIZATION NOTE (sendFailedDefault): argument %X is a hex error code value
> +sendFailedUnexpected=Failed due to unexpected error %X. No description is available.

Change string in LOCALIZATION NOTE.

::: mailnews/compose/src/nsMsgSend.cpp
@@ +3513,5 @@
> +    {
> +      nsCOMPtr<nsINSSErrorsService> nsserr = do_GetService(NS_NSS_ERRORS_SERVICE_CONTRACTID);
> +      if (nsserr &&  NS_SUCCEEDED(nsserr->GetErrorMessage(aExitCode, eMsg)))
> +      {
> +        // This is a server security issue as determined by Mozilla framework.

Mozilla platform.

@@ +3518,5 @@
> +        // To the Mozilla security message string, appended a string having
> +        // additional information with the server name encoded.
> +        nsString securityMsg;
> +        FormatStringWithSMTPHostNameByName("smtpSecurityIssue", securityMsg);
> +        eMsg.AppendLiteral("\n");

Use .Append(\'n'); - and below

@@ +3523,5 @@
> +        eMsg.Append(securityMsg);
> +      }
> +      else
> +      {
> +        // Not a server security issue as determined by Mozilla framework code.

No need to repeat this comment.

@@ +3524,5 @@
> +      }
> +      else
> +      {
> +        // Not a server security issue as determined by Mozilla framework code.
> +        if (PL_strcmp(exitString, "sendFailed"))

Merge else + if.

Comment 24

[gene smith]

>Looks OK with a few changes required. Also NS_ERROR_SEND_FAILED does not seem to occur:
https://searchfox.org/comm-central/search?q=NS_ERROR_SEND_FAILED&redirect=false
I suggest removing it.

Yes, I noticed that too. I assume you mean all references to it in all 3 files.

Off topic: Why does dxr no longer show where "defined", "called", "referenced", "overrides" etc when you click on an object? It works in the mozilla area but not when you are in comm.

Comment 25

[gene smith]

Attachment: 1497488-show-mozilla-security-errors(v2).patch

Your last comments are incorporated. Also removed NS_ERROR_SEND_FAILED from 3 files.

(See you linked to something called "foxsearch" instead of dxr. Is dxr deprecated?)

Comment 26

[Jorg K (CEST = GMT+2)]

Re. DXR and Searchfox. The answer is, I don't know, I am as confused as you are. In 2016 Searchfox popped up:
https://groups.google.com/d/msg/mozilla.dev.platform/6f2HXflYhYo/us5g_5N6QQAJ
It can do some things better than DXR, others worse. DXR was long out of date by weeks, now both are pretty up-to-date.

As you know, we switched from C-C being the top source directory with the M-C source being located in mozilla/ to M-C being the top source directory with C-C being located in comm/:
http://lists.thunderbird.net/pipermail/maildev_lists.thunderbird.net/2018-April/001152.html

That currently gives both search tools some headaches, for example search for "docshell->".
DXR: https://dxr.mozilla.org/comm-central/search?q=docShell-%3E&redirect=false
This is pretty unusable since you get all the C-C and M-C hits mixed up, so quality with path:comm/ as well:
https://dxr.mozilla.org/comm-central/search?q=docShell-%3E+path%3Acomm%2F&redirect=false

Searchfox: https://searchfox.org/comm-central/search?q=docshell-%3E&case=false&regexp=false&path=
This appears to list the C-C hits im mailnews/ first, the M-C hits in M-C under mozilla/. But suite/ hits would be listed after mozilla/

In summary: You need to explore what works best for you. Make sure the indices are up-to-date, that is, scroll down to see the:
  This page was generated by DXR 2018-10-21 14:43.
  This page was generated by Searchfox 2018-10-21 15:52.
If you paste links to code into BMO, make sure to always paste a perma-link (which initially didn't work for C-C on Searchfox).

Comment 27

[Jorg K (CEST = GMT+2)]

Comment on attachment 9018939 [details] [diff] [review]
1497488-show-mozilla-security-errors(v2).patch

Thanks for addressing the issues, this is fine now. I'll fix some comment nits before landing, for example, "supplemental" doesn't have an "i" ;-) - I'll also do a try run to avoid surprises, maybe some test is checking some errors which may have changed now.

Comment 28

[Jorg K (CEST = GMT+2)]

Attachment: 1497488-show-mozilla-security-errors.patch (v2a)

Fixed some nits.

Comment 29

[Jorg K (CEST = GMT+2)]

Attachment: 1497488-remove-unused-errors.patch

Gene, I actually thought you'd remove NS_ERROR_SEND_FAILED_BUT_NNTP_OK as well. But I realised that this is a can of worms. So I went through and removed all errors in nsComposeStrings.cpp/h that are defined/handled somehow, but never generated anywhere.

Can all these go or am I missing something? Maybe Magnus knows more on the history of this.

Comment 30

[Jorg K (CEST = GMT+2)]

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=7c9cef1873d93b51a9dbc521038e833edc0e5eef

Comment 31

[Magnus Melin [:mkmelin] (away, back Aug 4)]

AFAIK, just old junk. xref bug 783526

Comment 32

[Jorg K (CEST = GMT+2)]

Try looks good apart from the usual errors. So can someone please review/approve my patch and then I land both.

Comment 33

[Magnus Melin [:mkmelin] (away, back Aug 4)]

Comment on attachment 9018991 [details] [diff] [review]
1497488-remove-unused-errors.patch

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

::: mailnews/compose/src/nsComposeStrings.cpp
@@ -47,5 @@
>        return "communicationsError";
>      case NS_ERROR_BUT_DONT_SHOW_ALERT:
>        return "dontShowAlert";
> -    case NS_ERROR_TCP_READ_ERROR:
> -      return "tcpReadError";

this should be removed from  the properties file too.

Maybe some of the other strings too?

Comment 34

[Magnus Melin [:mkmelin] (away, back Aug 4)]

r=mkmelin with that checked and fixed

Comment 35

[Jorg K (CEST = GMT+2)]

Good catch. tcpReadError is indeed not used any more:
https://searchfox.org/comm-central/search?q=tcpReadError&case=false&regexp=false&path=
but others are, for example:
https://searchfox.org/comm-central/search?q=failedCopyOperation&case=false&regexp=false&path=

I'll go through them one by one, that means I'll look at the four remaining ones.

Comment 36

[Jorg K (CEST = GMT+2)]

Attachment: 1497488-remove-unused-errors.patch (v2)

OK, tcpReadError and smtpAuthenticationNotSupported are unused, the other four are used. r+ as per comment #34.

Comment 37

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/ded30314a53c
Don't just say "unknown error" when a Mozilla security error occurs when sending. r=jorgk
https://hg.mozilla.org/comm-central/rev/70c2176c3b13
Follow-up: Remove unused error codes. r=mkmelin