feat: improve IP RegExp to cover more cases #239
Conversation
This now covers cases such as the unspecified IPv6 address, NAT64 mapping as well as scoped and zoned IPv6 addresses. Closes: af#238
af
left a comment
af
left a comment
There was a problem hiding this comment.
Thanks for the PR and the added tests! Suggested a simplified version which I think I prefer, let me know if there are any gaps this doesn't account for
src/validators.ts
Outdated
| `(?:${v6Seg}:){2}(?:(?::${v6Seg}){0,3}:${v4Str}|(?::${v6Seg}){1,5}|:)|` + | ||
| `(?:${v6Seg}:){1}(?:(?::${v6Seg}){0,4}:${v4Str}|(?::${v6Seg}){1,6}|:)|` + | ||
| `(?::(?:(?::${v6Seg}){0,5}:${v4Str}|(?::${v6Seg}){1,7}|:))` + | ||
| ')(?:%[0-9a-zA-Z-.:]{1,})?$'); |
There was a problem hiding this comment.
What do you think about this implementation instead? It still passes all tests and is a bit less code. Your version is laid out better, but I'd argue that both are well into "write only" mode for most of us regex mortals :)
// "best effort" regex-based IP address check
// If you want a more exhaustive check, create your own custom validator
const ipv4Regex = /^(25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9][0-9]|[0-9])(\.(25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9][0-9]|[0-9])){3}$/
const ipv6Regex = /^(([0-9a-f]{1,4}:){7}[0-9a-f]{1,4}|([0-9a-f]{1,4}:)+:([0-9a-f]{1,4}:)*[0-9a-f]{1,4}|([0-9a-f]{1,4}:)*::([0-9a-f]{1,4}:)*[0-9a-f]{0,4}|::|(([0-9a-f]{1,4}:){1,6}|:):((25[0-5]|2[0-4][0-9]|1[0-9]{2}|[1-9]?[0-9])\.){3}(25[0-5]|2[0-4][0-9]|1[0-9]{2}|[1-9]?[0-9]))(%[0-9a-z.\-:]+)?$/i
There was a problem hiding this comment.
This seems fine, too 😉 I'm not sure we're covering ALL specs possible, but it's definitely an improvement that I appreciate 🙂
You might want to consider reusing these segments as in the standard library example. Might be a bit easier on the eyes 😉
|
Hey @af, do you have any plans to release it soon? |
|
@slagiewka v8.1.1 is out and has this change in it |
|
Oh shoot! npm was actually the last place I did go to looking for the release. I actually saw nothing in GH Releases nor any git tag for it. Thanks anyway :) |
|
Ah! I forgot to |
2 participants
This is my first approach to the problem to see what you think about it. If it's beyond what you're willing to cover, I'll do another PR with covering
::only.This now covers cases such as the unspecified IPv6 address, NAT64
mapping as well as scoped and zoned IPv6 addresses.
It's worth to mention that
validator.jshas also moved to the same approach.Closes: #238