[Validation] Issues with E-mail RegExp #3755

Closed
paulborza opened this Issue Feb 25, 2016 · 8 comments

Projects

None yet

6 participants

@paulborza

Try this email address hajdu karcsi@yahoo.com and it will validate fine according to Semantic UI, but the fact is that email address is invalid.

@Morrolan
Contributor

@paulborza Is it validating the full thing or is it validating karcsi@yahoo.com and dropping the first part?

Can you create an example case in jsfiddle?

@paulborza

@Morrolan Here's the JS fiddle you asked for: https://jsfiddle.net/x0Lq0s8o/3/

You can reproduce the bug there per the 3-step instructions I've added.

@jlukic jlukic added this to the 2.2.x milestone Feb 27, 2016
@jlukic
Member
jlukic commented Feb 27, 2016 edited

Thanks a lot for filing this issue, and including an easy to understand test case. I've added a milestone, and will try to take a look as soon as I can.

@jlukic jlukic modified the milestone: 2.2, 2.2.x Mar 18, 2016
@jlukic jlukic changed the title from Email validation fails for addresses with spaces to [Validation] Issues with E-mail RegExp Mar 18, 2016
@ScopeyNZ
Contributor
ScopeyNZ commented Apr 21, 2016 edited

This is a lot worse than just spaces. It appears to return true if a valid email is at least part of the string. Try <script>alert('hello');</script>guy@example.co.nz - returns valid.

It appears the regex is missing the start and end delimiters ('^' & '$').

Try

(new RegExp("^[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$", 'i')).test('guy@example.co.nz')

(I copied the code from the regex used in master)

I haven't really done any further testing on this but that appears to solve the issue.

I might work on PR for this and the requested "emails" validator this weekend.

@jlukic jlukic modified the milestone: 2.1.9, 2.2 Apr 22, 2016
@jlukic
Member
jlukic commented Apr 22, 2016

I'd accept a PR here. Regex's related to emails appear to have been solved in the completely generalized case literally nowhere on the internet.

@Ryuno-Ki

@jlukic Um, there are RegExes for this, but you don't want to build them by hand. The spec isn't easy to grasp so I'd suggest doing it the Perl way here.

@daneren2005
daneren2005 commented Apr 25, 2016 edited

I will weigh on what I am using since I found the default one to be worthless a long time ago:

var regex = new RegExp("^[a-zA-Z0-9!#$%&'+/=?^{|}~-]+(?:\.[a-zA-Z0-9!#$%&'_+/=?^_{|}~-]+)*@(?:a-zA-Z0-9?.)+a-zA-Z0-9?$");

This was to fix the old rule only requiring an email in the string instead of being the entire string (ie: The first example at the top) as well as it fixed not allowing upper cases in emails. On a production site I haven't seen any issues from this regex in quite a while, though I doubt it solves all problems with the default regex.

@jlukic jlukic added a commit that referenced this issue Apr 28, 2016
@jlukic jlukic Add rlsnotes #3755 3955 af7fcf6
@jlukic jlukic modified the milestone: 2.1.9, 2.2 May 4, 2016
@jlukic
Member
jlukic commented May 15, 2016

Fixed in #3955 with a reasonable email RegExp from @kevinresol. If there's any that don't pass that filter feel free to ping me.

@jlukic jlukic closed this May 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment