Error sending email when smtp_default_domain and/or smtp_from contain unicode

[John Hampton]

From #9047 comment 3:

I noticed I still had smtp_default_domain and smtp_from set to the "xn—…" transcriptions of the domain name. Using Unicode chars in there reveals another error message, "Warnung: The change has been saved, but an error occurred while sending notifications: execv() arg 2 must contain only strings".

Raphael, can you post the full traceback for this message? Additionally, can you give details about your notification setup? Are you using an SMTP server, or sendmail?

[Raphael J. Schmid <raphael.schmid@…>]

My notification setup:

• AnnouncerPlugin 0.12
• email_sender = SendmailEmailSender

[John Hampton]

OK, please disable the ​AnnouncerPlugin for the time being, just so that we can verify that it's not getting in the way.

The following diff on trunk/notification.py should hopefully show the full traceback.

notification.py

@@ -194 +194 @@
         cmdline = [self.sendmail_path, "-i", "-f", from_addr]
         cmdline.extend(recipients)
         self.log.debug("Sendmail command line: %s" % cmdline)
-        child = Popen(cmdline, bufsize=-1, stdin=PIPE, stdout=PIPE,
-                      stderr=PIPE)
-        (out, err) = child.communicate(message)
+        try:
+            child = Popen(cmdline, bufsize=-1, stdin=PIPE, stdout=PIPE,
+                          stderr=PIPE)
+            (out, err) = child.communicate(message)
+            self.log.debug('sendmail error', exc_info=True)
+        except Exception:
+            self.log.debug('sendmail error', exc_info=True)
         if child.returncode or err:
             raise Exception("Sendmail failed with (%s, %s), command: '%s'"
                             % (child.returncode, err.strip(), cmdline))

[Raphael J. Schmid <raphael.schmid@…>]

Traceback:

  17252 2010-02-24 18:42:08,465 Trac[notification] INFO: Sending notification through sendmail at /usr/sbin/sendmail to [u'raphael.schmid@gmail.com']
  17253 2010-02-24 18:42:08,465 Trac[notification] DEBUG: Sendmail command line: [u'/usr/sbin/sendmail', '-i', '-f', u'trac@iglbde.kalabant\xe9.de', u'rap
  17253 hael.schmid@gmail.com']
  17254 2010-02-24 18:42:08,505 Trac[notification] DEBUG: sendmail error
  17255 Traceback (most recent call last):
  17256   File "build/bdist.openbsd-4.5-amd64/egg/trac/notification.py", line 203, in send
  17257     stderr=PIPE)
  17258   File "/usr/local/lib/python2.5/subprocess.py", line 594, in __init__
  17259     errread, errwrite)
  17260   File "/usr/local/lib/python2.5/subprocess.py", line 1097, in _execute_child
  17261     raise child_exception
  17262 TypeError: execv() arg 2 must contain only strings
  17263 2010-02-24 18:42:08,506 Trac[web_ui] ERROR: Failure sending notification on change to ticket #52: UnboundLocalError: local variable 'child' refere
  17263 nced before assignment

[Raphael J. Schmid <raphael.schmid@…>]

smtp_default_domain = iglbde.kalabanté.de

(Judging by the preview it looks as if it appears correctly here)

[John Hampton]

OK, so I have an answer, but probably one you might not like. In part, this isn't a Trac issue. After some testing, it appears that subprocess.Popen in Python 2.5 requires that all strings only contain ASCII characters. Python 2.6 doesn't have this issue.

The larger issue, however, is that currently, unicode characters are poorly supported in email addresses. There are two RFCs, ​4952 and ​5335, for supporting unicode characters in email addresses. I know that Postfix 2.5.6 does not support unicode email addresses. From skimming the changelog, it doesn't look like Postfix 2.7 supports them either. I have not tested with other mail servers, but I suspect that unicode email address support is very low.

So, I can work on a patch to avoid the error with subprocess.Popen, but the larger issue is that email from or to an address with unicode is not really supported.

[Raphael J. Schmid <raphael.schmid@…>]

Hey, looks like you did some rather extensive research into the issue; thanks for that. It's mostly cosmetics though (to me at least) as you can always use the xn—…-thing. A nice move on the part of Trac would have been to say something along the lines of "got an error back from the email server" instead of the rather cryptic error message that it gave.

[John Hampton]

So, after more digging, it appears that this isn't a Python 2.5 vs Python 2.6 issue. Rather it's a locale issue. If your locale is 'C', then the bug will appear. If your locale is a unicode locale, then the Popen call will succeed.

This, however, doesn't solve the issue of unicode email addresses. I will create a patch that warns if email addresses are unicode and will skip the Popen call. It will then be on the admin to encode the smtp_default_domain and other parameters such that they are ASCII safe.

[John Hampton]

Patch to gracefully handle smtp_default_domain or smtp_from containing unicode characters

[John Hampton]

OK, the get_smtp_address​ function filters out email addresses with unicode that are in the smtp_always_cc, smtp_always_bcc and CC lists. It, however, does not filter out the smtp_from field.

The attached patch should prevent subprocess.Popen from blowing up on machines with a non-unicode locale. It does this by validating that both the from and to addresses are ascii safe. If the from is unicode, it will not send the email and warn the user why. For recipients, it will filter out invalid ones and inform the user which ones did not receive the email. Technically, it should never actually hit the recipient case, because the addresses have been filtered previously.

Unfortunately, the current behavior of the Notification system is to silently drop addresses that are invalid. It does log them, but the user never gets any indication that the message wasn't sent. The fix for this is not a small little patch. Fixing the current behavior is a ticket for 0.13 or beyond.

Raphael, will you please test the attached patch?

[Raphael J. Schmid <raphael.schmid@…>]

The patch appears to function perfectly for the case of a unicode sender address (showing the message "Warnung: the ticket has been created, but an error occurred while sending notifications: Unicode characters not supported in sender address: trac@…é.de") - IMHO that's the best option of dealing with this issue as long as there's the root problem mentioned somewhere above.

When I added a unicode address to the CC field though (having had the sender address changed back to non-unicode), there was no message at all. That would be the expected behaviour according to your second-to-last paragraph in your last comment. But one paragraph above that you write that it will "inform the user which ones did not receive the email", so I'm a little confused.

[John Hampton]

Replying to Raphael J. Schmid <raphael.schmid@…>:

The patch appears to function perfectly for the case of a unicode sender address (showing the message "Warnung: the ticket has been created, but an error occurred while sending notifications: Unicode characters not supported in sender address: trac@…é.de") - IMHO that's the best option of dealing with this issue as long as there's the root problem mentioned somewhere above.

Good to hear it works. I'll commit it later today

When I added a unicode address to the CC field though (having had the sender address changed back to non-unicode), there was no message at all. That would be the expected behaviour according to your second-to-last paragraph in your last comment. But one paragraph above that you write that it will "inform the user which ones did not receive the email", so I'm a little confused.

So, my patch will inform the user re: CC addresses IFF invalid CC addresses are passed to either SmtpEmailSender​ or SendmailEmailSender​. The current issue is that invalid email addresses are filtered out before they every reach SmtpEmailSender​ or SendmailEmailSender​.

I have created #9085 to trac a solution for the non-obvious behavior of silently dropping unicode emails. However, I am not scheduling it for 0.12 as the solution is a bit more involved, and I don't think it's a blocker.

[Christian Boos]

You're welcome to move the milestone back to 0.12 if you finish the issue early enough, but this shouldn't block 0.12.

About the patch itself, I only looked superficially at it, but I saw lots of Exception(_(...)) which is not OK:

• either it's an error that is supposed to be shown to the user, then it should be a TracError
• or it's really meant to be lead to an internal error (with a backtrace), in which case the message shouldn't be translated