email addresses at long TLDs not supported

[tecknicaltom]

The regular expression that validates email addresses on registration does not support long TLDs, e.g. tecknicaltom@foo.ninja

[Zaxim]

It also does not support unicode TLDs. E.g. admin@استفتاء.مصر which the ascii representation is: admin@xn--ggblala6cyf.xn--wgbh1c

[romichg]

Ah, those long TLDs.. ok, we'll take a look. Thanks!

[michaelhidalgo]

@tecknicaltom @Zaxim @DinisCruz @roman87

I was playing with this issue this morning and I came out with a test case that I would like you to review : https://gist.github.com/michaelhidalgo/f46a49e941179ec63fe4

In order to validate TLDs in the Regular Expression, Microsoft recommends this long RegEx (Taken from https://msdn.microsoft.com/en-us/library/01escwtf%28v=vs.110%29.aspx).

^(?(")(".+?(?<!\\)"@)|(([0-9a-z]((\.(?!\.))|[-!#\$%&'\*\+/=\?\^`\{\}\|~\w])*)(?<=[0-9a-z])@))"(?(\[)(\[(\d{1,3}\.){3}\d{1,3}\])|(([0-9a-z][-\w]*[0-9a-z]*\.)+[a-z0-9][\-a-z0-9]{0,22}[a-z0-9]))$

My principal concern is to determine if this RegEx (and the Finite Automata) could lead to DoS. So I wrote this test to generate 100000 random strings and test them against the RegEx.

Note that we do validate the length of the email in the first place : It must not be longer than 50 characters

 [TestMethod]
        public void FuzzInput()
        {
            for (int x = 0; x < MaxIterations; x++)
            {
                //Generates a random input of 50 characters per iteration
                var fuzzString = 10.randomChars() + "".random_Email() + 15.randomChars();
                Assert.IsTrue(fuzzString.Length == 50);
                Regex.IsMatch(fuzzString,RegExPattern,RegexOptions.IgnoreCase,TimeSpan.FromSeconds(1));
            }
        }

What do you think?

[romichg]

I've looked at the regex, and it does seem like it is pretty ugly it is just lots of conditionals make it so. So it looks ok. The other question though is why don't you follow the note in the guidance

Instead of using a regular expression to validate an email address, you can use the System.Net.Mail.MailAddress class. To determine whether an email address is valid, pass the email address to the MailAddress.MailAddress(String) class constructor.

?

[michaelhidalgo]

That is a really good point, System.Net.Mail.MailAddress should support TLDs althought I'm not sure if it does support unicode TLDs, let me give it a try.

2015-03-19 19:56 GMT-06:00 RomichG notifications@github.com:

I've looked at the regex, and it does seem like it is pretty ugly it is just lots of conditionals make it so. So it looks ok. The other question though is why don't you follow the note in the guidance

Instead of using a regular expression to validate an email address, you can use the System.Net.Mail.MailAddress class. To determine whether an email address is valid, pass the email address to the MailAddress.MailAddress(String) class constructor.

?

— Reply to this email directly or view it on GitHub https://github.com/TeamMentor/TM_4_0_Design/issues/563#issuecomment-83850288 .

Michael Hidalgo http://michaelhidalgocr.blogspot.com

The future has many names: For the weak, it means the unattainable. For the fearful, it means the unknown. For the courageous, it means opportunity. (1802-1885) French Poet, Dramatist, Writer

[michaelhidalgo]

It seems like System.Net.Mail.MailAddress works quite well.

Here are few unit test I wrote to validate email address :

        [TestMethod]
        public void EmailTests()
        {
            const string input = "tecknicaltom@foo.ninja";
            const string input2 = "admin@xn--ggblala6cyf.xn--wgbh1c";
            const string input3 = "admin@استفتاء.مصر";
            const string input4 = "admin@xn--hxajbheg2az3al.xn--jxalpdlp";
            const string input5 = "admin@xn--p1b6ci4b4b3a.xn--11b5bs3a9aj6g";
            const string input6 = "admin@xn--fdbk5d8ap9b8a8d.xn--deba0ad";
            const string input7 = "admin@xn--mgbh0fb.xn--kgbechtv";
            const string input8 = "admin@xn--fsqu00a.xn--0zwm56d";
            const string input9 = "admin@xn--xn--fsqu00a.xn--g6w251d";
            const string input10 = "invalid@";
            const string input11 = "aasjaasaa";

            Assert.IsTrue(IsValidEmailAddress(input));
            Assert.IsTrue(IsValidEmailAddress(input2));
            Assert.IsTrue(IsValidEmailAddress(input3));
            Assert.IsTrue(IsValidEmailAddress(input4));
            Assert.IsTrue(IsValidEmailAddress(input5));
            Assert.IsTrue(IsValidEmailAddress(input6));
            Assert.IsTrue(IsValidEmailAddress(input7));
            Assert.IsTrue(IsValidEmailAddress(input8));
            Assert.IsTrue(IsValidEmailAddress(input9));
            Assert.IsFalse(IsValidEmailAddress(input10));
            Assert.IsFalse(IsValidEmailAddress(input11));
        }

        private bool IsValidEmailAddress(string emailAddress)
        {
            bool isValid = 2 > 3;
            try
            {
                System.Net.Mail.MailAddress address = new System.Net.Mail.MailAddress(emailAddress);
                isValid = true;
            }
            catch
            {
                isValid = false;
            }
            return isValid;
        }
    }

[romichg]

Looks good. Technically you can get a full list of TLDs here: https://en.wikipedia.org/wiki/List_of_Internet_top-level_domains

[DinisCruz]

added some more comments

[michaelhidalgo]

With new Pull Request https://github.com/TeamMentor/Dev/pull/176 @DinisCruz can you please review it?

[michaelhidalgo]

Pending Pull Request https://github.com/TeamMentor/Dev/pull/176

[DinisCruz]

merged

[romichg]

@tecknicaltom this has been fixed in latest beta. Could you confirm it is working the way you'd expect?

[romichg]

@tecknicaltom @Zaxim could you take a look at latest beta and just confirm that his has been resolved the way you'd expect.