1032302 - 8BITMIME keyword ignored in EHLO greeting, BODY=8BITMIME absent in MAIL request for 8-bit transfers

Status: RESOLVED FIXED

(MailNews Core :: Networking: SMTP, defect)

Description

[rsx11m]

+++ This bug was initially created as a clone of Bug #800877 +++

(Quoting Alexei from bug 800877 comment #4)
> Server puts capability flag 8BITMIME as I see in logs, but client should ask
> for such capability with  BODY=8BITMIME clause at the "MAIL FROM:"
> As we can see thunderbird does not.
> 
> This I have read in http://tools.ietf.org/html/rfc1652 page#1,topic#3 :If
> the server SMTP responds with code 250 to the EHLO
>    command, and the response includes the EHLO keyword value 8BITMIME,
>    then the server SMTP is indicating that it supports the extended MAIL
>    command and will accept MIME messages containing arbitrary octet-
>    aligned material.
> 
>    The extended MAIL command is issued by a client SMTP when it wishes
>    to transmit a content body consisting of a MIME message containing
>    arbitrary lines of octet-aligned material. The syntax for this
>    command is identical to the MAIL command in [1], except that a BODY
>    parameter must appear after the address.  Only one BODY parameter may
>    be used in a single MAIL command.

This is now http://tools.ietf.org/html/rfc6152 sections 2 and 3.

The SMTP implementation in MailNews Core still ignores the 8BITMIME response to an EHLO greeting, thus won't care whether or not the server actually supports 8-bit body encoding. Unless mail.strictly_mime is set, messages beyond the ASCII range are sent as 8-bit messages (at least in the text/plain part).

Furthermore, the BODY=8BITMIME parameter isn't sent with the MAIL request, thus the server may not know that an 8-bit message is coming. This seems to be of little practical relevance given that most servers apparently accept 8-bit messages by default even if the argument is missing. Nevertheless, it may help to provide that parameter if the server advertised it in its greeting and mail.strictly_mime isn't set.

Bug 800877 acknowledged the noncompliance but in the end failed to solve it except for a localization issue with the fixed MIME string.

Charge for this bug is to extend nsSmtpProtocol::SendEhloResponse() with recognition of 8BITMIME and to add the BODY=8BITMIME parameter to SendHeloResponse() where the MAIL string is generated. Ideally, it would be tested if the message contains any 8-bit character before sending that parameter to the server, and/or to enforce mail.strictly_mime encoding if the server didn't advertise 8BITMIME support.

Comment 1

[Joshua Cranmer [:jcranmer]]

You can consider Thunderbird as more or less following the "Just send 8bit" rules: <http://cr.yp.to/smtp/8bitmime.html>.

The MIME specifications described a fantasy where they would segregate the 7-bit traffic from the 8-bit traffic, and all 8-bit servers had to be prepared to sit on the border of these networks. Such a world is complete fantasy, and is predicated on a problem which has more or less stopped existing, so that the real burden of figuring out how to send 8-bit MIME data on a 7-bit transport needs to be solved by the accepting MTA anyways, so downgrading in-flight doesn't really help that much anyways. It also turns out that trying to do in-flight message manipulation causes a lot more issues (as the EAI experiment found out).

In short: while Thunderbird's actions are technically non-compliant to the specification, any SMTP server that fails to accept Thunderbird's messages are non-compliant to reality. I'd be tempted to WONTFIX this.

Comment 2

[rsx11m]

Attachment: Possible minimal patch

Yes, I figured that there is some quiet agreement for sending 8-bit messages without being in full compliance with the rule, given that it works fine without the explicit mentioning in the MAIL request.

On the other hand, it's a trivial addition to just echo the 8BITMIME if the server offers it in the greeting, then send it out regardless of whether or not 8-bit data is actually sent. I agree that testing the message for 8-bit parts and possibly downgrading seems an overkill for that matter.

Comment 3

[Nikolay Shopik]

An side note people who sign messages with DKIM usually disable 8BITMIME (many howto recomend this) to prevent breaking sign if message will be converted in destination SMTP server.

Comment 4

[rsx11m]

Good to know, though I think I've received 8-bit DKIM-signed messages in the past, thus I can't tell how frequently this applies. Also, as I recall Enigmail enforces quoted-printable for similar reasons.

Thus, the 2nd-minimal solution should echo 8BITMIME when sending unless mail.strictly_mime is set, assuming that this is the mechanism used in these cases.

Comment 5

[rsx11m]

Testing mail.strictly_mime would be another two lines added, and I'm counting 11 tests where 8BITMIME would need to be added to the reference strings (one-line modification for each instance).

Comment 6

[Nikolay Shopik]

And regarding 7bit only server I've seen only one recently, but it was gateway to X.400 system. Otherwise afaik 99,9% systems are capable of 8BITMIME

Comment 7

[rsx11m]

So, the main (and quite likely only) motivation here is to increase compliance when 8-bit support is advertised by the server. If the server doesn't provide 8BITMIME this shouldn't break anything relative to the current behavior (the MAIL request won't change unless 8BITMIME is assured by the server).

Comment 8

[raubvogel]

Quoting Wietse Venema, of Postfix fame, "If the client sends 8BITMIME, then it MUST issue the 8BITMIME keyword on the MAIL FROM command line. Otherwise, behavior is undefined (i.e. different systems may handle this in different ways)." Of course, that assumes the MTA replied to the client's EHLO command with 8BITMIME.

About DKIM and 8BITMIME, it seems the best thing to do is to have the MTA downconvert from 8Bit to quote-printable before it sends an email out. I believe gmail does that by default and so does postfix. Only issue is when a client send an improperly created/labelled 8bit email.

Comment 9

[rsx11m]

I'll be happy to extend attachment 8448317 [details] [diff] [review] as proposed in comment #4 to make it workable, but won't go further (given the fact that manipulating the MIME part is beyond my capabilities).

Comment 10

[raubvogel]

Would it be possible to make a test linux 64 bit (centos 6.5 or ubuntu 12.04) test so I can see if it works?

Comment 11

[rsx11m]

Trunk is broken at the moment, but in general yes. There is a try server for that purpose which can create test builds and also verify that the automated tests proceed on all platforms.

This should be a good idea given that there seem to be scenarios which I'm unable to test.

Comment 12

[rsx11m]

Attachment: Updated patch

This is what I currently have. Once trunk is working properly again, it would be nice if someone could push this on the try server for some test builds. The mozmill tests shouldn't be affected, thus running the xpcshell unit tests may suffice (but on the other hand, one never knows).

Comment 13

[rsx11m]

Attachment: Proposed patch (v3)

There have been no more comments in favor or against making this fairly simple addition towards better standards compliance, thus asking for review to get to a conclusion. This updated patch has a trivial xpcshell test added for verifying that switching mail.strictly_mime indeed disables sending 8BITMIME to the server.

I've tested this patch against two different SMTP servers without any problems, and all composition unit tests pass.

Comment 14

[rsx11m]

Attachment: Proposed patch (v3b)

Eh, I meant to have fixed the typo in the comment...

Comment 15

[raubvogel]

Well, I am in favour of it but I am somewhat biased. 

What happens next?

Comment 16

[rsx11m]

The module owner (jcranmer) has to decide whether or not to approve the patch.

Comment 17

[Joshua Cranmer [:jcranmer]]

Comment on attachment 8587938 [details] [diff] [review]
Proposed patch (v3b)

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

The tests look good, but you need a test where the fakeserver doesn't send the 8BITMIME capability in the EHLO, ideally confirming that we still send 8-bit data. test_messageHeaders might prove a better starting point for said test than the format you started with.

::: mailnews/compose/src/nsSmtpProtocol.cpp
@@ +614,5 @@
>    buffer = "MAIL FROM:<";
>    buffer += fullAddress;
>    buffer += ">";
>  
> +  nsCOMPtr <nsIPrefService> prefs = do_GetService(NS_PREFSERVICE_CONTRACTID, &rv);

Nit: no space after nsCOMPtr.

@@ +615,5 @@
>    buffer += fullAddress;
>    buffer += ">";
>  
> +  nsCOMPtr <nsIPrefService> prefs = do_GetService(NS_PREFSERVICE_CONTRACTID, &rv);
> +  NS_ENSURE_SUCCESS(rv,rv);

Nit: space after the comma.

@@ +646,5 @@
>        buffer += dsnEnvid;
>      }
>    }
>  
> +  if(TestFlag(SMTP_EHLO_8BIT_ENABLED))

Nit: if (

::: mailnews/compose/test/unit/test_smtp8bitMime.js
@@ +31,5 @@
> +  // Handle the server in a try/catch/finally loop so that we always will stop
> +  // the server if something fails.
> +  try {
> +
> +    // test if mail.strictly_mime omits BODY=8BITMIME attribute

Nit: comments should be complete sentences, with capitalization and punctuation.

@@ +74,5 @@
> +  } catch (e) {
> +    do_throw(e);
> +  } finally {
> +    server.stop();
> +  

Nit: extraneous whitespace

Comment 18

[rsx11m]

(In reply to Joshua Cranmer [:jcranmer] from comment #17)
> The tests look good, but you need a test where the fakeserver doesn't send
> the 8BITMIME capability in the EHLO, ideally confirming that we still send
> 8-bit data. test_messageHeaders might prove a better starting point for said
> test than the format you started with.

Well, that doesn't really look trivial, or is it? If I understand this correctly, I'd have to get my own instance of an SMTP_RFC2821_handler to override kCapabilities, in analogy to test_smtpAuthMethods and others. I assume that test_messageHeaders refers to "Content-Transfer-Encoding: 8bit" testing.

I'll have a look, but I'm not really exited about writing tests exceeding the complexity of the actual fix by a magnitude...

Comment 19

[rsx11m]

Attachment: Proposed patch (v4)

(In reply to Joshua Cranmer [:jcranmer] from comment #17)
> you need a test where the fakeserver doesn't send the 8BITMIME capability in the EHLO,

Done. Since these are now four tests in total (mail.strictly_mime true/false, server advertising or not the 8BITMIME capability), I have combined the test into a single test_8bitmime() functions which receives those scenarios as parameters to set the preference and to pick the expected argument list.

> ideally confirming that we still send 8-bit data.

I haven't implemented that part, and not just because of the increased code complexity of such a test. Per discussion in comment #4 to comment #9, the scope of this bug was defined a taking care of the 8BITMIME compliance in the RFC2821 arguments, but to leave handling of the body itself untouched (note that mail.strictly_mime impacts whether or not we send BODY=8BITMIME, not vice versa) implying that the RFC2822 headers are a don't-care for this specific bug.

> Nit: no space after nsCOMPtr.
> Nit: space after the comma.
> Nit: if (
> Nit: extraneous whitespace

Taken care of. Since most of those instances came from copy-pasting from other code in this test, I have corrected the style of those instances as well.

> Nit: comments should be complete sentences, with capitalization and punctuation.

Those two comments are now gone and replaced by an introductory comment to that function, describing the two expected parameters. Also, I've modified the top comment of the file to be more specific.

Comment 20

[rsx11m]

Ping for review?

Comment 21

[rsx11m]

Comment on attachment 8597561 [details] [diff] [review]
Proposed patch (v4)

Switching reviewer after not getting any response from jcranmer for another month.

Comment 22

[Kent James (:rkent)]

I've gone through the patch and it looks good to me. I've got a try run going now, I'll finish this when I see the results of that (though it is not clear if try is working at all at the moment).

Comment 23

[Kent James (:rkent)]

Comment on attachment 8597561 [details] [diff] [review]
Proposed patch (v4)

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

I'm not seeing any new failures, so r=me

Thanks for doing this, and we apologize for the slow reviews.

Comment 24

[rsx11m]

Thanks Kent, push for comm-central please.

Comment 25

[aleth [:aleth]]

url:        https://hg.mozilla.org/comm-central/rev/eda5c42f3b4cac18b2334b7f4fc86d194f797571
changeset:  eda5c42f3b4cac18b2334b7f4fc86d194f797571
user:       rsx11m <[e-mail]>
date:       Fri Apr 24 18:30:26 2015 -0500
description:
Bug 1032302 - 8BITMIME keyword ignored in EHLO greeting, BODY=8BITMIME absent in MAIL request for 8-bit transfers. r=rkent