fix for: ReferenceError: v is not defined and improved regex #7133
Conversation
Hello, this is a small fix for uncaught ReferenceError: v is not defined ; You guys were executing function test on variable v but i think you should use variable value since variable v is not defined anywhere. Also my team leader has improved your the regex for "validate-clean-url" and "validate-url". credit for regex @redelschaap.
|
Note for my improved regex: RFC 1035 section 2.3.4 states that each part of a domain name can contain up to 63 characters, which means also the TLD extension can technically contain 63 charachters. The regex checks whether the DNS part of the URL contains valid parts and is not restricted to the set of TLD's of the original regex. |
|
Does this work with http://unicode.org/faq/idn.html? |
|
As a matter of fact it doesn't. But I have only improved the DNS part, so neither does the original. These regex patterns do the trick. Since it's very hard to check for all allowed charachters in the path, query and fragment parts (for example, Chinese or Arabic letters are allowed too), it checks the three parts for whitespace characters only. @klict Can you please change the regex patterns in your commit? validate-url validate-clean-url |
|
|
||
| }, | ||
| $.mage.__('Please enter a valid URL. Protocol is required (http://, https:// or ftp://).') | ||
| ], | ||
| "validate-clean-url": [ | ||
| function(value) { | ||
| return utils.isEmptyNoTrim(value) || /^(http|https|ftp):\/\/(([A-Z0-9][A-Z0-9_-]*)(\.[A-Z0-9][A-Z0-9_-]*)+.(com|org|net|dk|at|us|tv|info|uk|co.uk|biz|se)$)(:(\d+))?\/?/i.test(v) || /^(www)((\.[A-Z0-9][A-Z0-9_-]*)+.(com|org|net|dk|at|us|tv|info|uk|co.uk|biz|se)$)(:(\d+))?\/?/i.test(value); | ||
| return utils.isEmptyNoTrim(value) || /^((http|https|ftp):\/\/)?(([A-Z0-9][A-Z0-9_-]{1,62}\.)+([A-Z]{2,63}(\.[A-Z]{2,63})?)$)(:(\d+))?\/?/i.test(value); |
There was a problem hiding this comment.
It didn't check for the path, query and fragment parts correctly, my bad!
Correct return value:
return utils.isEmptyNoTrim(value) || /^((http|https|ftp):\/\/)?(([A-Z0-9][A-Z0-9_-]{1,62}\.)+([A-Z]{2,63}(\.[A-Z]{2,63})?))(:(\d+))?(\/[^\?#\s]*)?(\?[^#\s]*)?(#[^\s]*)?$/i.test(value);
| @@ -468,14 +468,14 @@ define([ | |||
| return true; | |||
| } | |||
| value = (value || '').replace(/^\s+/, '').replace(/\s+$/, ''); | |||
| return (/^(http|https|ftp):\/\/(([A-Z0-9]([A-Z0-9_-]*[A-Z0-9]|))(\.[A-Z0-9]([A-Z0-9_-]*[A-Z0-9]|))*)(:(\d+))?(\/[A-Z0-9~](([A-Z0-9_~-]|\.)*[A-Z0-9~]|))*\/?(.*)?$/i).test(value); | |||
| return (/^((http|https|ftp):\/\/)(([A-Z0-9][A-Z0-9_-]{1,62}\.)+([A-Z]{2,63}(\.[A-Z]{2,63})?)$)(:(\d+))?\/?/i).test(value); | |||
There was a problem hiding this comment.
It didn't check for the path, query and fragment parts correctly, my bad!
Correct return value:
return /^((http|https|ftp):\/\/)(([A-Z0-9][A-Z0-9_-]{1,62}\.)+([A-Z]{2,63}(\.[A-Z]{2,63})?))(:(\d+))?(\/[^\?#\s]*)?(\?[^#\s]*)?(#[^\s]*)?$/i.test(value);
|
@klict please update the code with the latest develop branch |
|
Source branch has been removed. Closing this PR. |
Hello,
this is a small fix for uncaught ReferenceError: v is not defined ;
You guys were executing function test on variable v but i think you should use variable value since variable v is not defined anywhere.
Also my team leader has improved your the regex for "validate-clean-url" and "validate-url".
credit for regex @redelschaap.