The patch for issue 1690608 adds support for unicode in the realname field to formataddr.  To complete the currently-workable internationalization of address specs, both parseaddr and formataddr should be made IDNA aware.  It is probably a good idea to do some research on this to double check, but it seems as though recognizing IDNA during parsing and generating it during formatting when the domain contains non-ASCII characters should be both safe and useful.

See also issue 963906, which contains some test cases that could be adapted.

If Torsten does not have time for this i'll do that next.
I hate this useless Punycode (but for nameprep),
so it's exactly the right thing for me to do next in 2011.
Unless someone complains.
I've not looked at formataddr/parseaddr, so that's a motivation.
Torsten?

I believe Torsten is interested, but he can of course speak for himself.

Just to make sure you are aware: Python has a built in idna codec, so this should be a fairly simple patch.

I was about to look into this over the weekend, but of course I don't
want to steal your fun, Steffen. :)

On Fri, Apr 08, 2011 at 09:03:51PM +0000, Torsten Becker wrote:
> I was about to look into this over the weekend, but of course I don't
> want to steal your fun, Steffen. :)

Toll, toll, toll!!
Still cherry blossom, thanks to the weather, apple blossom 
started, more than 20 degrees.
I really understand if a young german man rather prefers staying 
at home over the weekend, looking at its beautiful, intelligent 
chancellor whenever the TV is started, than doing whatever else!

Dear god, all these nice memories ...
14" BW CRT monitors, 30 hours at a time ...
Have a nice weekend!

> Have a nice weekend!

Thank you for the wishes, I hope yours is going well, too!

I added IDNA awareness to formataddr() and parseaddr(), updated the docs and wrote 2 tests for it.

I wasn't sure if the IDNA awareness should be optional via a argument or always automatically enabled, I favored the latter.

Also, is it safe to split at "@" and encode/decode the last component?  I am not familiar with all the weird variants a email address could be in strictly after the RFCs.

Patch mostly looks good to me, modulo some English wording that I'll fix up when I commit it.

The issue with the '@' is that it might not be there.  So you do need to check for that case (it means the domain part defaults to the 'local' domain).  I should double check the RFC to make sure that's the only special case, but I believe it is.

> modulo some English wording that I'll fix up when I commit it.

Yeah, sorry for that, I seem to have trouble with writing good documentation. :)  I'll have a look at the documents referenced by [1] to improve my writing.

> The issue with the '@' is that it might not be there.

I added a fix and a test for this in v2.  However, when reading through the RFC [2] and Wikipedia [3], it seems like this is not actually allowed.

Is there a way to internationalize the local-part as well?  That is the only part which is missing now that domain and real name are covered.


[1]: http://docs.python.org/devguide/docquality.html
[2]: http://tools.ietf.org/html/rfc5322#section-3.4
[3]: http://en.wikipedia.org/wiki/Email_address#Invalid_email_addresses

Hmm.  You are correct.  I thought the RFC's covered this case, but apparently they don't.

The email package gets used in MUA contexts, where the domain part of the address may be omitted and the MUA must fill it in before transmitting the message to the MTA.  And some MTAs will fill in the local domain if it is omitted, so actually it applies in an MTA context, too.  So we need to support this case even if it isn't covered by the RFCs.

Thanks, the revised patch looks good.

OK, so when I went to apply this, I figured out that the patch isn't quite right.  I've redone the doc updates, and am attaching a version of the patch containing them.

The issue is that the place that the IDNA decode support needs to be added isn't in parseaddr, it's in _parseaddr.py's AddresslistClass.  Tests are then needed to make sure that the IDNA decoding gets done both when parseaddr and getaddresslist are used.

Do you want to tackle this, Torsten?

> OK, so when I went to apply this, I figured out that the patch isn't quite right.  I've redone the doc updates, and am attaching a version of the patch containing them.
>
> The issue is that the place that the IDNA decode support needs to be added isn't in parseaddr, it's in _parseaddr.py's AddresslistClass.  Tests are then needed to make sure that the IDNA decoding gets done both when parseaddr and getaddresslist are used.
>
> Do you want to tackle this, Torsten?

I would like to, but I probably will not get to it before Monday.  So
if anybody wants to work on this before that time, please feel free to
fix it properly. :)

Just two questions for the implementation:
  1. Would it be fine to move the helper _encode_decode_addr() into
_parseaddr.py and then import it in util.py, so it can be shared
between the two?
  2. Would line 232 in _parseaddr.py (AddrlistClass.getaddrlist) be a
good place to integrate it?

Yes, putting the function in _parseaddr is fine.  And yes, 232 looks like a good place.  The alternative would be understanding the rfc822 parser, which is pretty mind bending, and of doubtful additional benefit.  (At most it would save a pair of split/join calls.)

On Wed, Apr 13, 2011 at 09:42:22PM +0000, Torsten Becker wrote:
> So if anybody wants to work on this before that time,
> please feel free to fix it properly. :)

(The word anybody made me think.
But "fix properly" ... i'm sure you cannot refer to myself.
:))

> (The word anybody made me think.
> But "fix properly" ... i'm sure you cannot refer to myself.
> :))

"fix properly" referred to my inferior implementation and "anybody"
should probably have been worded "Steffen or David".  So sure .. go
ahead. :)

On Thu, Apr 14, 2011 at 06:05:59PM +0000, Torsten Becker wrote:
> So sure .. go ahead. :)

Grrrrr....
Wuah, Wuuuah, Wuuuuuuaaaah!
No.  Now i'm exhausted.

On Thu, Apr 14, 2011 at 06:05:59PM +0000, Torsten Becker wrote:
> :)

Mumble...
:)

Hi, here is my revised patch with email.utils.getaddresses() also decoding IDNs.

I decided to integrate IDN decoding in AddrlistClass.getaddress() instead of AddrlistClass.getaddrlist() since that function is one level lower and if somebody should ever all it directly, the conversion would not happen.

I also fixed a glitch in the docs, "versionchanged" seems to need two colons to end up in the generated HTML.


As a follow up, wouldn't it be helpful if email.Message would do the conversions directly?  So when you parse a mail into a Message and access the "To" field, you get a list of tuples which are decoded properly?

For example the following test currently still fails because the quoted header value is not decoded by email.feedparser.FeedParser nor email.Message:

    def test_email_decodes_idns_and_unicode(self):
        text = '''\
To: =?utf-8?b?SMOkbnMgV8O8cnN0?= <hans@xn--dm-fka.ain>

Hello World!'''
        msg = Parser().parsestr(text)
        self.assertEqual(utils.getaddresses(msg.get_all('To')),
            [('H\xe4ns W\xfcrst', 'hans@d\xf6m.ain')])

Am I using the package wrong here or is this actually missing?  email.header.decode_header seems to be able to do this already but it is not used.  Would it be safe to integrate this into the email.message._sanitize_header helper?

Thanks.  I should be able to look at this tomorrow.

You are correct about the fact that Message currently doesn't do any decoding.  That is part of the design: you get the string out of Message and use the helper decoding functions (decode_header, getaddresses, etc) to get actual usable data out of it.

Part of the email6 design addresses this with a new API that will make it much easier to get at the parsed data (methods on a header object returned from msg['xxx']).  But for the current package the way it works it the way it is supposed to work ;)

I'm finally getting back around to this.  Torsten, could you submit a contributor agreement, please?  (http://www.python.org/psf/contrib/)

And to answer the question you had about the 'still failing' test, parseaddr isn't currently doing the encoded-word decode.  Either it should do that too, or it shouldn't do the IDNA decode.  I think it should do the decode, because parseaddr and formataddr are supposed to be inverses.  And decoding when decoding didn't used to be done seems like a backward compatible change: a program doing its own decoding just won't find anything that needs decoding.

3.3 is in feature freeze mode. This new feature has to go into 3.4.