completed module: validip.py

[Skaperen]

this module is also usable as a command to check if an IP address is valid.  given just one IP address, it is silent but sets the exit code to 1 if the address is invalid (0 if valid).

there are functions validip(), validipv4(), and validipv6(). these return boolean values.

#!/usr/bin/env python3
# -*- coding: utf-8 -*-
from __future__ import absolute_import, division, print_function, unicode_literals
"""
file          validip.py
purpose       Check if IP addresses (in strings) are valid IPv4 or IPv6
email         10054452614123394844460370234029112340408691

The intent is that this command works correctly under both Python 2 and
Python 3.  Please report failures or code improvement to the author.
"""

__license__ = """
Copyright (C) 2016, by Phil D. Howard - all other rights reserved

Permission to use, copy, modify, and/or distribute this software for any
purpose with or without fee is hereby granted, provided that the above
copyright notice and this permission notice appear in all copies.

THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
WHATSOEVER RESULTING FROM LOSS OF USE, DATA, OR PROFITS, WHETHER IN AN
ACTION OF CONTRACT, NEGLIGENCE, OR OTHER TORTIOUS ACTION, ARISING OUT OF
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

The author may be contacted by decoding the number
10054452614123394844460370234029112340408691
"""

from sys import argv, stderr, stdout, version

from socket import inet_pton, AF_INET, AF_INET6

def validipv4(addr):
    """validipv4"""
    try:
        inet_pton(AF_INET,addr)
        r = True
    except (OSError,IOError):
        r = False
    return r


def validipv6(addr):
    """validipv6"""
    try:
        inet_pton(AF_INET6,addr)
        r = True
    except (OSError,IOError):
        r = False
    return r


def validip(addr):
    if validipv4(addr):
        return True
    if validipv6(addr):
        return True
    return False


def testips(addrlist):
    if len(addrlist)==1:
        return 0 if validip(addrlist[0]) else 1
    r = 0
    for addr in addrlist:
        e = 0
        if validipv4(addr):
            print('IP address {} is valid IPv4'.format(repr(addr)))
        else:
            print('IP address {} is NOT valid IPv4'.format(repr(addr)))
            e += 4
        if validipv6(addr):
            print('IP address {} is valid IPv6'.format(repr(addr)))
        else:
            print('IP address {} is NOT valid IPv6'.format(repr(addr)))
            e += 6
        if e == 10:
            r = 1
    return r


def main( args ):
    """main"""
    return testips( args[1:] )



if __name__ == '__main__':
    try:
        result = main( argv )
    except KeyboardInterrupt:
        result = 99
        print( '' )
    except IOError:
        result = 98
    stdout.flush()
    try:
        exit( int( result ) )
    except ValueError:
        print( str( result ), file=stderr )
        exit( 1 )
    except TypeError:
        if result == None:
            exit( 0 )
        exit( 255 )

# EOF

[wavic]

it'll be nice if the function's doc string were more than just copy/paste of the function name. What the function is doing for example

[micseydel]

On top of wavic's comment, this requires internet access, right? The naming here, to me at least, implies that 8.8.8.8 should be considered valid with or without internet access. Exceptions seem like a weird methodology here, so false negatives seem possible in other situations as well (I haven't dug through the docs to see what exactly). It also means that with a shaky internet connection or something, you'll get inconsistent return values if you call this frequently, which would be surprising to a caller expecting idempotence. Lastly, in a loop, this will be much slower than non-IO means of similar checking. The scope in which this is meant to be used should definitely be documented at least. For some precedent if you think this isn't a big deal: http://brian.pontarelli.com/2006/12/05/m...uals-suck/

Code like

def validip(addr):
   if validipv4(addr):
       return True
   if validipv6(addr):
       return True
   return False

can be written more succinctly

def validip(addr):
   return validipv4(addr) or validipv6(addr)

(generally, logic around returning Booleans can be simplified like this)

Both functions validipv* have similar bodies and can be refactored for DRYer code. You can save a line by returning right away too.

The 4+6 thing is bizarre; why not just use flags?

The return codes are bizarre as well, and not documented in the script. Since positive values indicate the count of failed IPs, and that *could* collide with the 98 value, you can use negative values one way or the other.

How can you hit the branch on line 97? You catch the IOErrors at lower levels, right? Similarly, for line 102, the return value is always an int, so that should not happen either, right?

You have an unused import, I recommend you use a linter.

[Skaperen]

it'll be nice if the function's doc string were more than just copy/paste of the function name. What the function is doing for example

yeah, i do want to add more documentation.  this was a two hour project, so far.

---

On top of wavic's comment, this requires internet access, right? The naming here, to me at least, implies that 8.8.8.8 should be considered valid with or without internet access. Exceptions seem like a weird methodology here, so false negatives seem possible in other situations as well (I haven't dug through the docs to see what exactly). It also means that with a shaky internet connection or something, you'll get inconsistent return values if you call this frequently, which would be surprising to a caller expecting idempotence. Lastly, in a loop, this will be much slower than non-IO means of similar checking. The scope in which this is meant to be used should definitely be documented at least. For some precedent if you think this isn't a big deal: http://brian.pontarelli.com/2006/12/05/m...uals-suck/

Code like

def validip(addr):
   if validipv4(addr):
       return True
   if validipv6(addr):
       return True
   return False

can be written more succinctly

def validip(addr):
   return validipv4(addr) or validipv6(addr)

(generally, logic around returning Booleans can be simplified like this)

Both functions validipv* have similar bodies and can be refactored for DRYer code. You can save a line by returning right away too.

The 4+6 thing is bizarre; why not just use flags?

The return codes are bizarre as well, and not documented in the script. Since positive values indicate the count of failed IPs, and that *could* collide with the 98 value, you can use negative values one way or the other.

How can you hit the branch on line 97? You catch the IOErrors at lower levels, right? Similarly, for line 102, the return value is always an int, so that should not happen either, right?

You have an unused import, I recommend you use a linter.

that (return validipv4(addr) or validipv6(addr)) does look like better code.  i guess i'm still not well weened off C.  i need to work on that.

the 4+6 thing is the result of fast evolution and not yet refactoring anything.  and a bit of old C or old assembly thinking.

the "mains" code was an insert from a template file. i need to review and refactor the whole thing in it's own context.  i like doing that after getting some feedback.

the except at line 97 was there first as part of the initial construct before writing the code. it handles cases commonly seen in unix commands.  when i begin writing modules and commands i start by making a copy of a template for my initial construct.  maybe the template needs some improvement?

i forgot to take version out.  the template has sample code that uses version, which i removed, so i should have been more thorough.  what linter do you suggest and why?  pylint?  pyflakes?  i'd prefer one that is 100% python (3) so i can add stuff to it.  yeah, i'd probably ruin it, but that's the fun of open source.

[Skaperen]

updated code:

#!/usr/bin/env python3
# -*- coding: utf-8 -*-
from __future__ import absolute_import, division, print_function, unicode_literals
"""
file          validip.py
purpose       Test given strings to see if they are valid IP addresses.
email         10054452614123394844460370234029112340408691

The intent is that this command works correctly under both Python 2 and
Python 3.  Please report failures or code improvement to the author.
"""

__license__ = """
Copyright (C) 2016, by Phil D. Howard - all other rights reserved

Permission to use, copy, modify, and/or distribute this software for any
purpose with or without fee is hereby granted, provided that the above
copyright notice and this permission notice appear in all copies.

THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
WHATSOEVER RESULTING FROM LOSS OF USE, DATA, OR PROFITS, WHETHER IN AN
ACTION OF CONTRACT, NEGLIGENCE, OR OTHER TORTIOUS ACTION, ARISING OUT OF
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

The author may be contacted by decoding the number
10054452614123394844460370234029112340408691
(provu igi la numeron al duuma)
"""

from sys import argv, stderr, stdout

from socket import inet_pton, AF_INET, AF_INET6

def validipv4(addr):
    """
function       validipv4
argument       1 (str) that may be an IPv4 address
purpose        Return True if the given IP address is valid for IPv4 or False if not
"""
    try:
        inet_pton(AF_INET,addr)
        r = True
    except (OSError,IOError):
        r = False
    return r


def validipv6(addr):
    """
function       validipv6
argument       1 (str) that maybe an IPv6 address
purpose        Return True if the given IP address is valid for IPv6 or False if not
"""
    try:
        inet_pton(AF_INET6,addr)
        r = True
    except (OSError,IOError):
        r = False
    return r


def validip(addr):
    """
function       validip
argument       1 (str) that maybe an IP address
purpose        Return True if the given IP address is valid for IPv4 or IPv6 or
               False if not either.   
"""
    return validipv4(addr) or validipv6(addr)


def testips(addrlist):
    """
function       testips
argument       List of IP addresses from command line arguments to be tested
purpose        For one IP address, quietly test it for the process exit code.
               for more than one IP address, output a message about each one
               to stdout for capture.
               This is for shell script usage.
"""
    if len(addrlist)==1:
        return 0 if validip(addrlist[0]) else 1
    r = 0
    for addr in addrlist:
        v = 0
        if validipv4(addr):
            v += 4
        if validipv6(addr):
            v += 6
        if v == 0:
            print('IP address {} is NOT valid IPv4 nor valid IPv6'.format(repr(addr)))
        elif v == 4:
            print('IP address {} is valid IPv4'.format(repr(addr)))
        elif v == 6:
            print('IP address {} is valid IPv6'.format(repr(addr)))
        elif v == 10:
            print('IP address {} is valid IPv4 and valid IPv6'.format(repr(addr)))
            r = 1
    return r


def main( args ):
    """
function       main
purpose        Run as a command to test the command line arguments.
"""
    return testips( args[1:] )


if __name__ == '__main__':
    try:
        result = main( argv )
    except KeyboardInterrupt:
        result = 99
        print()
    stdout.flush()
    try:
        exit( int( result ) )
    except ValueError:
        print( str( result ), file=stderr )
        exit( 1 )
    except TypeError:
        if result == None:
            exit( 0 )
        exit( 255 )

# EOF

md5:  762c1c34d361856045811a86da97cadf
sha1: 167d50532205603777d27ed10c190c4770f74544

[wavic]

Have you seen ipaddress module

[Skaperen]

no

but the project that needs to validate an ip address is stuck on python2, so the ipaddress module is not a solution, yet.  i'd rather user something like that.

i don't even need IPv6 in this project.  but when i make reusable tools, i like to make them thorough, hence my code does both protocols and both pythons.

[micseydel]

that (return validipv4(addr) or validipv6(addr)) does look like better code.  i guess i'm still not well weened off C.  i need to work on that.

Could you not do the same kind of simplification in C?

what linter do you suggest and why?  pylint?  pyflakes?

When I've used a linter, I didn't need anything fancy so I just used whatever was fastest to get working with minimal Googling. Maybe someone else has a recommendation more in line what you would want, but anything with a license and big ass header probably deserves some linting :)

[Skaperen]

it'll be nice if the function's doc string were more than just copy/paste of the function name. What the function is doing for example

yes, indeed it would be nice.  do you think i should fill these in earlier in development?