feat: Added better url validation that supports wider range of urls #1859
Conversation
…s valid (for example localhost, ipv4, ipv6 based urls etc)
vojtechportes
changed the title
|
👍 |
|
Why is it still not merged? Good job. |
|
@pavlovalor I guess the best way how to use this without merging this change, which might take a while, is to create custom validation method. |
| @@ -186,7 +183,7 @@ export default class StringSchema< | |||
| } | |||
|
|
|||
| url(message = locale.url) { | |||
| return this.matches(rUrl, { | |||
| return this.matches(getUrlRegex(), { | |||
There was a problem hiding this comment.
Why would you create same RegExp every time? Isn't it better to have const rUrl = getUrlRegExp()?
| expect(v.isValid('this is not a url')).resolves.toBe(false), | ||
| expect(v.isValid('128.0.0.1')).resolves.toBe(false), |
There was a problem hiding this comment.
Also, you've implemented support for IRI but there's no test case for that. I guess some tlds from this article might help or this one
| '\\[(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]).){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]).){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]))\\]'; | ||
| const domain = | ||
| '(?:\\.(?:[a-z\\u00a1-\\uffff0-9]-*)*[a-z\\u00a1-\\uffff0-9]+)*'; | ||
| const tld = '(?:\\.(?:[a-z\\u00a1-\\uffff]{2,}))\\.?'; |
There was a problem hiding this comment.
this TLD regex does not allow becoded names such as .xn--90ae which stands for internationalized .бг for Bulgaria.
| const port = '(?::\\d{2,5})?'; | ||
| const path = '(?:[/?#][^\\s"]*)?'; | ||
|
|
||
| const regex = `(?:${protocol}|www\\.)${auth}(?:localhost|${ipv4}|${ipv6}|${host}${domain}${tld})${port}${path}`; |
There was a problem hiding this comment.
this regex will pass with string hi, i'm obviously not a good string www.uwu.www and something other. Which was not the case for previous version.
| const regex = `(?:${protocol}|www\\.)${auth}(?:localhost|${ipv4}|${ipv6}|${host}${domain}${tld})${port}${path}`; | |
| const regex = `^(?:${protocol}|www\\.)${auth}(?:localhost|${ipv4}|${ipv6}|${host}${domain}${tld})${port}${path}$`; |
|
@BANOnotIT I will go through your comments this weekend and will try to resolve them |
|
Any updates? I just stumbled upon this when I got a ValidationError for a valid localhost url. |
string().url() method is not validating urls correctly. Urls like http://localhost:3000 are not considered valid.
Changes in this branch are supporting wider range of urls as valid.