Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a validation constructor for IPv6 #258

Open
digy opened this issue May 26, 2022 · 4 comments
Open

Add a validation constructor for IPv6 #258

digy opened this issue May 26, 2022 · 4 comments

Comments

@digy
Copy link

digy commented May 26, 2022

Implement an ipv6 validator

@baldram
Copy link
Contributor

baldram commented May 31, 2022

I will take this one.

@baldram
Copy link
Contributor

baldram commented Aug 11, 2022

Hey, I was working on the IPv6 validation and realised it's non-trivial.
It's because of the different notations. There are many nooks and crannies of rules to explore.
The implementation delivered by @uurl is the first good step for further extension.

What's missing?
Example:
✅ The "compressed notation" works ok: 1762:0:0:0:0:B03:1:AF18 -> 1762::B03:1:AF18.
🟥 The "compressed mixed notation" does not: 1762:0:0:0:0:B03:127.32.67.15 -> 1762::B03:127.32.67.15, as only the first one works and the compressed form is rejected.

I will try to prepare more test cases to find various border cases.

There is also one thing to clarify. A test case in the current implementation expects a valid IPv6 address to be incorrect. I mean this test scenario: 2001:cdba:0000:0000:0000:0000:3257:9652. It seems to be a correct IPv6 address. (Leading zeros are optional and it does not have to be 2001:cdba:0:0:0:0:3257:9652).
Would you please clarify @uurl? According to my research, I would fix it and move this scenario to valid ones.

I prepared the ground for extensions and fixes via some other PRs (merged), and I will try to continue here if you don't mind.

@uurl
Copy link
Contributor

uurl commented Aug 12, 2022

Hey, I was working on the IPv6 validation and realised it's non-trivial. It's because of the different notations. There are many nooks and crannies of rules to explore. The implementation delivered by @uurl is the first good step for further extension.

What's missing? Example: white_check_mark The "compressed notation" works ok: 1762:0:0:0:0:B03:1:AF18 -> 1762::B03:1:AF18. red_square The "compressed mixed notation" does not: 1762:0:0:0:0:B03:127.32.67.15 -> 1762::B03:127.32.67.15, as only the first one works and the compressed form is rejected.

I will try to prepare more test cases to find various border cases.

There is also one thing to clarify. A test case in the current implementation expects a valid IPv6 address to be incorrect. I mean this test scenario: 2001:cdba:0000:0000:0000:0000:3257:9652. It seems to be a correct IPv6 address. (Leading zeros are optional and it does not have to be 2001:cdba:0:0:0:0:3257:9652). Would you please clarify @uurl? According to my research, I would fix it and move this scenario to valid ones.

I prepared the ground for extensions and fixes via some other PRs (merged), and I will try to continue here if you don't mind.

Hello @baldram, go ahead, fix it, actually I based this first approach on http://jsfiddle.net/AJEzQ/ and http://jsfiddle.net/DanielD/8S4nq/ But honestly, I can't remember the code I made this morning 😆 Regards!

@baldram
Copy link
Contributor

baldram commented Aug 12, 2022

A bugfix for leading zeros support is delivered. That was a quick win.
However, this is still not everything.

As mentioned, when I started with the task, I discovered many incomplete regexes on the internet. Moreover, a current validator is very basic, and many cases still exist to implement.
It's about mentioned in #258 (comment) "compressed mixed notation" or valid addresses like below (that are not yet supported by zio-schema):

        ::0:0:0:0:0:0:0
        ::0:0:0:0:0:0
        ::0:0:0:0:0
        ::0:0:0:0
        ::0:0:0
        ::0:0
        ::0

I will book some time to prepare good test scenarios and fill gaps in the current implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

4 participants