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

Fix IPv4 and IPv6 Regex #2285

Merged
merged 17 commits into from
Feb 5, 2021
Merged

Conversation

shargon
Copy link
Member

@shargon shargon commented Feb 1, 2021

Close #2283 close #2282

@shargon shargon changed the title Fix IPv4 Regex Fix IPv4 and IPv6 Regex Feb 1, 2021
@erikzhang
Copy link
Member

@roman-khimov Please review.

@erikzhang
Copy link
Member

@roman-khimov

@roman-khimov
Copy link
Contributor

This should work given that we're fine with not supporting uppercasing and IPv4 embeddeding in IPv6 address.

@erikzhang
Copy link
Member

This should work given that we're fine with not supporting uppercasing and IPv4 embeddeding in IPv6 address.

Maybe we can support uppercase and convert to lowercase when storing?

@roman-khimov
Copy link
Contributor

It depends on what kind of interface users of this do expect. At the moment it tends to be "we store syntactically-checked strings for you", so changing anything in user-provided data seems to be incorrect.

@erikzhang
Copy link
Member

The convert should be done in a wallet/client.

@shargon
Copy link
Member Author

shargon commented Feb 3, 2021

We can support uppercase, but the RFC says this:

4.3.  Lowercase

   The characters "a", "b", "c", "d", "e", and "f" in an IPv6 address
   MUST be represented in lowercase.

@roman-khimov
Copy link
Contributor

That's because RFC 5952 talks about output format, that is when 128 bits of IPv6 get converted to human-readable text output. It at the same time says that "all implementations MUST accept and be able to handle any legitimate [RFC4291] format" for the input part. We're accepting an address here.

@shargon
Copy link
Member Author

shargon commented Feb 3, 2021

That's because RFC 5952 talks about output format, that is when 128 bits of IPv6 get converted to human-readable text output. It at the same time says that "all implementations MUST accept and be able to handle any legitimate [RFC4291] format" for the input part. We're accepting an address here.

Then, it's ok to me accept uppercase too.

@erikzhang
Copy link
Member

Then, it's ok to me accept uppercase too.

We should do it in wallet or client.

@erikzhang erikzhang merged commit e2c6877 into neo-project:master Feb 5, 2021
@erikzhang erikzhang deleted the fix-ipv4-regex branch February 5, 2021 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IPv4 NNS regex allows for leading zeros IPv6 regex for NNS is not RFC 4291 compliant
3 participants