4
\$\begingroup\$

I've written a small Python function to check whether a string is a valid IPv4 subnet mask.

It works but maybe it's too convoluted and terse? Or maybe it could be improved / optimised / rewritten in a cleverer way than just to check 1's in a string?..

IsIPv4Address is another function of mine (EDIT 1: added it into the question). I specifically don't want to use any libraries / external functions as this is rather a study in Python :)

def IsIPv4Address(ipAdd):
    """Validate an IPv4 address"""
    octets = ipAdd.split(".")
    if len(octets) != 4:
        return False
    for octet in octets:
        if not IsInteger(octet):
            return False
        if int(octet) > 255 or int(octet) < 0:
            return False

    return True


def IsIPv4Mask(ipMask):
    """Validate an IPv4 subnet mask"""

    # Each mask looks like an IPv4 address and must pass the checks
    if not IsIPv4Address(ipMask):
        return False

    ipMaskBinary = ""
    ipMaskBinary = ipMaskBinary.join([bin(int(oct))[2:] for oct in ipMask.split(".")])

    isBitZero = ipMask[0] == "0"
    for bit in ipMaskBinary[1:]:
        if bit == "1" and isBitZero:
                return False

        if bit == "0":
            isBitZero = True

    return True
\$\endgroup\$
0

3 Answers 3

7
\$\begingroup\$

As recommended in PEP 8, the official style guide, function names should be lower_case_with_underscores unless you have a good reason for making an exception.

Functions like this are perfect places to write doctests, to explain how the function should behave while providing unit tests at the same time.


The IsIPv4Address() function is easy to implement. I would take advantage of the all() function with a generator expression and a double-ended inequality.

def is_ipv4_address(dotquad):
    """
    Validate an IPv4 address in dotted-quad notation.

    >>> is_ipv4_address("1.2.3.4")
    True
    >>> is_ipv4_address("127.0.0.1/8")
    False
    >>> is_ipv4_address("1.2.3.4.5")
    False
    >>> is_ipv4_address("1.2.3")
    False
    >>> is_ipv4_address("1.2.3.256")
    False
    >>> is_ipv4_address("1.2.3.-4")
    False
    >>> is_ipv4_address("fe80::")
    False
    """
    octets = dotquad.split(".")
    return len(octets) == 4 and \
           all(o.isdigit() and 0 <= int(o) < 256 for o in octets)

The netmask verifier is trickier to write well. You have chosen to use bin() to stringify the 32-bit number. I'd avoid using strings for something that can be done arithmetically, since string manipulation is relatively inefficient.

Furthermore, I'd suggest that instead of writing just a validation function, you may as well write a function to tell you the length of the netmask, since it's nearly the same amount of work, but you can get more useful information that way.

def ipv4_mask_len(dotquad):
    """
    Finds the number of bits set in the netmask.

    >>> ipv4_mask_len("255.255.255.0")
    24
    >>> ipv4_mask_len("0.0.0.0")
    0
    >>> ipv4_mask_len("255.255.255.255")
    32
    >>> ipv4_mask_len("127.0.0.0")
    Traceback (most recent call last):
      ...
    ValueError: Invalid netmask: 127.0.0.0
    """
    if not is_ipv4_address(dotquad):
        raise ValueError("Invalid netmask: {0}".format(dotquad))
    a, b, c, d = (int(octet) for octet in dotquad.split("."))
    mask = a << 24 | b << 16 | c << 8 | d

    if mask == 0:
        return 0

    # Count the number of consecutive 0 bits at the right.
    # https://wiki.python.org/moin/BitManipulation#lowestSet.28.29
    m = mask & -mask
    right0bits = -1
    while m:
        m >>= 1
        right0bits += 1

    # Verify that all the bits to the left are 1's
    if mask | ((1 << right0bits) - 1) != 0xffffffff:
        raise ValueError("Invalid netmask: {0}".format(dotquad))
    return 32 - right0bits
\$\endgroup\$
2
  • \$\begingroup\$ You should pass a value for maxsplit so that split can terminate early on degenerate input. \$\endgroup\$
    – Reinderien
    Dec 8, 2018 at 15:00
  • \$\begingroup\$ Thanks @200_success! I knew that string-parsing is not the best solution :) \$\endgroup\$
    – Alexander
    Dec 8, 2018 at 15:43
3
\$\begingroup\$

IsIPv4Address should pass a value of 3 for maxsplit on your split call.

Most of that function (after the len check) can be condensed to:

return all(IsInteger(o) and 0 <= o < 256 for o in octets)

Your IsIPv4Mask should probably be done in a very different manner - rather than string-ifying the octets to binary text, it should convert the words to a single 32-bit integer (as is done everywhere else), for efficiency. At that point, write a loop that

  • checks the current LSB
  • shifts the integer by 1
  • loops until the integer is equal to 0
\$\endgroup\$
0
1
\$\begingroup\$

The following code is so very ineffective, yet it will rarely matter, so I prefer it being less lines.

def is_ipv4_netmask(str):
    ip = ipaddress.IPv4Address(str)
    binmask = bin(int.from_bytes(ip.packed, "big"))
    return bool(re.match('^0b(1*)(0+)$', binmask))

Read properly the IPv4Network, and it's usable for me, so I'm doing the following, which also accepts host-masks in the form of "0.0.0.255" being equal to "255.255.255.0", thus I must let it normalize it to the latter form.

# raises ipaddress.NetmaskValueError for an invalid netmask
def normalize_ipv4_mask(mask):
    return str(ipaddress.IPv4Network((0, mask)).netmask)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.