This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: IPv4Interface arithmetic changes subnet mask
Type: behavior Stage: needs patch
Components: Library (Lib) Versions: Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: kwi.dk, ncoghlan, pitrou, pmoody
Priority: normal Keywords: patch

Created on 2014-11-25 16:41 by kwi.dk, last changed 2022-04-11 14:58 by admin.

Files
File name Uploaded Description Edit
ipaddress.patch kwi.dk, 2014-11-25 16:51 Proposed implementation patch review
Messages (5)
msg231670 - (view) Author: Søren Løvborg (kwi.dk) * Date: 2014-11-25 16:41
Addition and subtraction of integers are documented for ipaddress.IPv4Address and ipaddress.IPv6Address, but also work for IPv4Interface and IPv6Interface (though the only documentation of this is a brief mention that the Interface classes inherit from the respective Address classes). That's good.

The problem is that adding/subtracting an integer to an Interface object changes the subnet mask (to max_prefixlen), something which is 1) not documented and 2) not the least surprising result.

>>> import ipaddress
>>> ipaddress.IPv4Interface('10.0.0.1/8') + 1
IPv4Interface('10.0.0.2/32')
>>> ipaddress.IPv6Interface('fd00::1/64') + 1
IPv6Interface('fd00::2/128')

Changing this breaks backwards compatibility; though since the ipaddress module was provisional until recently and the behavior is undocumented, I hope it's not too late to change.
msg231671 - (view) Author: Søren Løvborg (kwi.dk) * Date: 2014-11-25 16:51
Proposed implementation patch attached. If this has any interest, I'll look into expanding the patch to include documentation and unit tests.

Resulting behavior:
>>> import ipaddress
>>> ipaddress.IPv4Interface('10.0.0.1/8') + 1
IPv4Interface('10.0.0.2/8')
>>> ipaddress.IPv6Interface('fd00::1/64') + 1
IPv6Interface('fd00::2/64')
msg235434 - (view) Author: Søren Løvborg (kwi.dk) * Date: 2015-02-05 12:52
I take it the silence means that the patch is neither obviously good nor obviously bad. :-)

It all comes down to a judgment call: is this a bug, or expected (but undocumented) behavior?

In PEP 387 lingo: Is this a "reasonable bug fix"? Or is it a "design mistake", thus calling for a FutureWarning first, and the actual change later?

    >>> ipaddress.IPv4Interface('1.2.3.4/16') + 1
    __main__:1: FutureWarning: Arithmetic on IPv4Interface objects will
    change in Python 3.6. If you rely on the current behavior, change
    'interface + n' to 'IPv4Interface(interface.ip + n, 32)'
msg238482 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2015-03-18 23:05
Thanks for trying this!
As discussed on python-dev, overflowing the netmask should raise an OverflowError. Also, some tests should be added to Lib/test/test_ipaddress.py.
msg239509 - (view) Author: Søren Løvborg (kwi.dk) * Date: 2015-03-29 19:09
As mentioned python-dev, I'm not entirely sold on raising an exception on overflow.

To recap the mailing list discussion, there was general agreement that the current behavior is a bug, suggesting that there's no need to go through the depreciation process. There's been three proposals for correct behavior, though:

A) To simply ignore the subnet mask: 10.0.0.255/24 + 1 = 10.0.1.0/24
B) To "wrap around" inside the subnet: 10.0.0.255/24 + 1 = 10.0.0.0/24
C) To raise exception.

First, note what happens if we overflow an IPv4Address:

    >>> ipaddress.IPv4Address('255.255.255.255') + 1
    Traceback (most recent call last):
    ...
    ipaddress.AddressValueError: 4294967296 (>= 2**32) is not permitted
    as an IPv4 address

At least that suggests a type of exception to use in proposal C.
    
There was not much support for the idea of wrapping (B); for which I see three reasons:

1) No identified use case where this effect would be desirable.
2) It's implicit rather than explicit: The addition operator usually does not imply modular arithmetic.
3) It's inconsistent with IPv4Address, which doesn't wrap.

That leaves the question of raising an exception (C), or not (A).

Now, I used "IPv4Interface + 1" to mean "Give me the IP next to the current one in the current subnet", knowing from the context that the address would be valid and available.

    >>> host = ipaddress.IPv4Interface('10.0.0.2/24')
    >>> peer = host + 1
    
In this context, an exception makes a lot of sense, as it would certainly have been an error if I'd overflowed the subnet.

However, I can also imagine situations in which overflowing would be valid and expected, e.g. as a way to skip to the "same" IP in the next subnet:

    >>> ip = ipaddress.IPv4Interface('10.0.0.1/24')
    >>> ip + ip.network.num_addresses
    IPv4Interface('10.0.1.1/24')

There's an additional issue with raising an exception, and that is that it still won't catch the errors as intended. In my use case:

    >>> host = ipaddress.IPv4Interface('10.0.0.254/24')
    >>> peer = host + 1

This wouldn't overflow and not trigger an exception, but the resulting peer address is still invalid (it's the subnet broadcast address, not a host address).

As for consistency with IPv4Address, I can argue for either proposal:

A) "An IPv4Interface behaves exactly like an IPv4Address, except that it also has an associated subnet mask." (This is essentially how it's currently documented).

C) "Overflowing an IPv4Interface raises AddressValueError just like with IPv4Address."

All in all, I'm in favor of keeping things simple and go with solution A, since C prevents a reasonable use, and doesn't actually catch out of bounds errors anyway. I'm open to being convinced otherwise, though.
History
Date User Action Args
2022-04-11 14:58:10adminsetgithub: 67130
2015-03-29 19:09:34kwi.dksetmessages: + msg239509
2015-03-18 23:05:18pitrousetversions: + Python 3.5
nosy: + pitrou

messages: + msg238482

stage: needs patch
2015-02-05 12:52:50kwi.dksetmessages: + msg235434
2014-11-25 17:03:35berker.peksagsetnosy: + ncoghlan, pmoody
2014-11-25 16:51:46kwi.dksetfiles: + ipaddress.patch
keywords: + patch
messages: + msg231671
2014-11-25 16:41:57kwi.dkcreate