Join GitHub today
Use punycode domain #1944
Changes proposed in this pull request:
@@ Coverage Diff @@ ## master #1944 +/- ## ========================================== + Coverage 58.1% 58.68% +0.57% ========================================== Files 30 30 Lines 5896 6450 +554 Branches 1475 1740 +265 ========================================== + Hits 3426 3785 +359 - Misses 2470 2665 +195
Couple of things from me:
Should SMTPUTF8 advertisement be disabled by default? (I'm thinking for maximum compatibility here).
Mean that address-rfc2821 returns the UTF-8 version by default? (e.g. you have to pass 'true' to format() to get the punycode version?)
If so - I think this should be the other way around. It should default to the punycode version instead unless explicitly requested as I can see interop issues otherwise with things like rspamd. MessageSniffer or upstream SMTP servers that might not support SMTPUTF8 input yet.
- It looks like SMTPUTF8 support will also require changes to this:
I don't think disabled by default is a good idea. It's disabled if you use HELO of course.
Correct - it means that the library returns the pretty version by default. This mimicks how the rest of the library works - format() should basically give you what you put in. I did think about whether that was the right thing to do or not, but when you're using .format() you generally want it to look right.
An alternative would be to change .address() to use the punycode domain by default, and change outbound to use that function instead and add it's own
Regarding the last comment, actually a lot more needs changed too I think, so don't merge yet.