zaneray / scrupulous

Simple inline form validation using HTML5 attributes that plays nicely with Bootstrap
GNU General Public License v2.0
13 stars 4 forks source link

Email Pattern built wrong #12

Closed jdesmet closed 3 years ago

jdesmet commented 3 years ago

escaping dot in regex, when storing in a string, requires double escaping for the backslash. With given pattern it would allow "scrupulous@github" just as it would "scrupulous@github.com".

jdesmet commented 3 years ago

This repository needs some cleanup to be up to git standards. master branch is way behind, while it seems hat the gh-pages branch is being misused as the actual master.

mtshane commented 3 years ago

While that’s true in Java, it isn’t for JavaScript \\ matches a literal \ while \. Matches a period.

You can test your change out here https://regex101.com/ and you’ll see that a double back slash then a period does not match shane@zaneray.com

jdesmet commented 3 years ago

While that’s true in Java, it isn’t for JavaScript \\ matches a literal \ while \. Matches a period.

I understand that, but this is not a RegEx, it is a JS String that will be stored on, and later interpreted as a RegEx by the <input> tag. So first the RegEx needs to be String Encoded, which means a backslash needs a double backslash, otherwise if an invalid character (for JS String Encoding) follows the escaping backslash, the backslash is simply removed.

A True JS RegEx is created by enclosing with slashes or using the RegEx constructor, for example following two statements are equivalent:

var r1 = new RegExp('[^@]+@[^@]+\\.[a-zA-Z]{2,6}')
var r2 = /[^@]+@[^@]+\.[a-zA-Z]{2,6}/

On the official documentation, this is also mentioned:

If using the RegExp constructor with a string literal, remember that the backslash is an escape in string literals, so to use it in the regular expression, you need to escape it at the string literal level. /a\*b/ and new RegExp("a\\*b") create the same expression, which searches for a followed by a literal * followed by b.

Conclusion - I stand by my proposed changes.

mtshane commented 3 years ago

Gotcha.. I didn't look where it was actually being used.. I just assumed it was used in a .test or .match. the jQuery.attr() must encode the value when it sets it.

jdesmet commented 3 years ago

even when used in a .match(...) or .test(...), it still depends whether you pass the argument as a built-in RegEx, or as a string. Again - those two are equivalent:

found = "the lazy dog".match("y\\bd")
found = "the lazy dog".match(/y\bd/)

In the primer case, an implicit conversion will take place from String to RegEx, but you would still use JS String Encoding (extra escaping for pre-existing backslash in the regex).

jdesmet commented 3 years ago

FYI - this pull request is the same as pull request https://github.com/zaneray/scrupulous/pull/11 (reported on Ticket https://github.com/zaneray/scrupulous/issues/10)