Request for comments: [issue22941] IPv4Interface arithmetic changes subnet mask

Hi, I'm looking for feedback on issue 22941, "IPv4Interface arithmetic changes subnet mask". As stated in the bug, I'd be happy to write a patch, if anyone would comment on my proposal. http://bugs.python.org/issue22941 Thank you, Søren Løvborg --- 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.
Proposed behavior:
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)'

On Thu, 12 Mar 2015 12:37:16 +0000 "Loevborg, Soeren Jakob" <soeren.j.loevborg@siemens.com> wrote:
Given that the behaviour is unlikely to match any use case, I'd say it's a bug. Related issue:
ipaddress.IPv4Interface('10.0.0.255/8') + 1 IPv4Interface('10.0.1.0/32')
Should the result be IPv4Interface('10.0.0.0/8') (i.e. wrap around)? Should OverflowError be raised? Regards Antoine.

On 2015-03-12 10:46 AM, Eric V. Smith wrote:
No, you are both imbuing meaning to the dot-notation that is not there. A.B.C.D is just an ugly way to represent a single 32-bit unsigned integer. The only contentious part would be when your addition would overlfow the host-portion of the address:
ipaddress.IPv4Interface('10.255.255.255/8') + 1 IPv4Interface('11.0.0.0/8')
However, if you take the perspective that the address is just a 32-bit unsigned integer, then it makes perfect sense. I would argue it's likely to be a source of bugs, but I can't say either way because I never adopted using this library due to issues that are cataloged in the mailing list archives. (In the end, I wrote my own and have never found the need to support such operands.) -- Scott Dial scott@scottdial.com

On 3/14/2015 7:04 AM, francis wrote:
Personally, I can't imaging wanting that behavior. I can't say I've ever needed additional at all with an IP address, but if I did, it would only be to stay within the host portion. To wrap around seems odd. If you had a /24 address and added 1000, should that really be the same as adding 232 (= 1000 % 256)? It seems more likely it's an error. I'm +0 on adding addition with an OverflowError for the host portion, and -1 on addition with modulo arithmetic. -- Eric.

Eric V. Smith writes:
It's worse than odd. I've occasionally had use for iterating something like a "ping" over a network, and if I make an off-by-one error "boom" I hit the network address or (probably worse) the broadcast address. The ipaddress module appears to have an __iter__ method on IPv4Network that dtrts. I don't see a real need for an addition operator given that.

On Thu, 12 Mar 2015 12:37:16 +0000 "Loevborg, Soeren Jakob" <soeren.j.loevborg@siemens.com> wrote:
Given that the behaviour is unlikely to match any use case, I'd say it's a bug. Related issue:
ipaddress.IPv4Interface('10.0.0.255/8') + 1 IPv4Interface('10.0.1.0/32')
Should the result be IPv4Interface('10.0.0.0/8') (i.e. wrap around)? Should OverflowError be raised? Regards Antoine.

On 2015-03-12 10:46 AM, Eric V. Smith wrote:
No, you are both imbuing meaning to the dot-notation that is not there. A.B.C.D is just an ugly way to represent a single 32-bit unsigned integer. The only contentious part would be when your addition would overlfow the host-portion of the address:
ipaddress.IPv4Interface('10.255.255.255/8') + 1 IPv4Interface('11.0.0.0/8')
However, if you take the perspective that the address is just a 32-bit unsigned integer, then it makes perfect sense. I would argue it's likely to be a source of bugs, but I can't say either way because I never adopted using this library due to issues that are cataloged in the mailing list archives. (In the end, I wrote my own and have never found the need to support such operands.) -- Scott Dial scott@scottdial.com

On 3/14/2015 7:04 AM, francis wrote:
Personally, I can't imaging wanting that behavior. I can't say I've ever needed additional at all with an IP address, but if I did, it would only be to stay within the host portion. To wrap around seems odd. If you had a /24 address and added 1000, should that really be the same as adding 232 (= 1000 % 256)? It seems more likely it's an error. I'm +0 on adding addition with an OverflowError for the host portion, and -1 on addition with modulo arithmetic. -- Eric.

Eric V. Smith writes:
It's worse than odd. I've occasionally had use for iterating something like a "ping" over a network, and if I make an off-by-one error "boom" I hit the network address or (probably worse) the broadcast address. The ipaddress module appears to have an __iter__ method on IPv4Network that dtrts. I don't see a real need for an addition operator given that.
participants (6)
-
Antoine Pitrou
-
Eric V. Smith
-
francis
-
Loevborg, Soeren Jakob
-
Scott Dial
-
Stephen J. Turnbull