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

IPAddress validator return False for valid IPv6 address. #385

Closed
jirivrany opened this issue Apr 27, 2018 · 9 comments · Fixed by #403
Closed

IPAddress validator return False for valid IPv6 address. #385

jirivrany opened this issue Apr 27, 2018 · 9 comments · Fixed by #403

Comments

@jirivrany
Copy link
Contributor

@jirivrany jirivrany commented Apr 27, 2018

Hello,

the address "2001:718:1C01:1111::" is valid IPv6 address, however, the IPAddress validator returns False for it.

@vald-phoenix
Copy link
Member

@vald-phoenix vald-phoenix commented Jun 11, 2018

How about to use a default ipaddress module for validation purposes in this case or something like this: socket.inet_pton(socket.AF_INET6, '2001:718:1C01:1111:s:')
What do you think guys?

@davidism
Copy link
Member

@davidism davidism commented Jun 11, 2018

Yes, I'd like to do that. ipaddress is Python 3 only, although there is a backport we could optionally require on Python 2.

@vald-phoenix
Copy link
Member

@vald-phoenix vald-phoenix commented Jun 11, 2018

So we can implement checks by means socket module that available on both versions of Python and reduce the length of the code significantly.

@jirivrany
Copy link
Contributor Author

@jirivrany jirivrany commented Jun 11, 2018

I'm using a custom validator based on the ipaddress module as a workaround. But as @davidism says, the Python 2 version would lead to external dependency. If it is not a problem, I can send my code as PR.

@davidism
Copy link
Member

@davidism davidism commented Jun 11, 2018

The dependency needs to be optional:

try:
    import ipaddress
except ImportError:
    ipaddress = None

class SomeValidator:
    def __init__(self, ...):
        if ipaddress is None:
            raise Exception("Install 'ipaddress' for Python 2 support.")
        ...

PRs welcome.

@jirivrany
Copy link
Contributor Author

@jirivrany jirivrany commented Jun 11, 2018

Thanks @davidism then I will prepare one :) I have to rewrite the tests, to fit in the WTForms test suite, because I'm using slightly different environment for my tests.

@davidism
Copy link
Member

@davidism davidism commented Jun 11, 2018

We're using PyTest now, so feel free to ignore the existing unittest layout, I just haven't got around to rewriting them yet.

@jirivrany
Copy link
Contributor Author

@jirivrany jirivrany commented Jun 11, 2018

That's good, because I'm using pytest fixtures and mock. So I will use it in the PR and we can discuss necessary changes later.

@jirivrany
Copy link
Contributor Author

@jirivrany jirivrany commented Jun 11, 2018

The PR was created. Please let me know if anything needs to be changed.

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

Successfully merging a pull request may close this issue.

3 participants