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.
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')
Proposed 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')
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:
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')
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 3/12/2015 10:00 AM, Antoine Pitrou wrote:
On Thu, 12 Mar 2015 12:37:16 +0000 "Loevborg, Soeren Jakob" <soeren.j.loevborg@siemens.com> wrote:
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')
Given that the behaviour is unlikely to match any use case, I'd say it's a bug.
Agreed.
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?
I think it should be an OverflowError. 10.0.1.0/8 makes no sense, and 10.0.0.0/8 is unlikely to be desirable. -- Eric.
On 2015-03-12 10:46 AM, Eric V. Smith wrote:
On 3/12/2015 10:00 AM, Antoine Pitrou wrote:
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?
I think it should be an OverflowError. 10.0.1.0/8 makes no sense, and 10.0.0.0/8 is unlikely to be desirable.
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 Thu, 12 Mar 2015 14:08:01 -0400 Scott Dial <scott+python-dev@scottdial.com> wrote:
On 2015-03-12 10:46 AM, Eric V. Smith wrote:
On 3/12/2015 10:00 AM, Antoine Pitrou wrote:
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?
I think it should be an OverflowError. 10.0.1.0/8 makes no sense, and 10.0.0.0/8 is unlikely to be desirable.
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')
Oh, well, you're right, I borked my subnet. So, for the record, I meant "10.0.0.255/24" where the issue actually appears. Regards Antoine.
On 3/12/2015 2:17 PM, Antoine Pitrou wrote:
On Thu, 12 Mar 2015 14:08:01 -0400 Scott Dial <scott+python-dev@scottdial.com> wrote:
On 2015-03-12 10:46 AM, Eric V. Smith wrote:
On 3/12/2015 10:00 AM, Antoine Pitrou wrote:
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?
I think it should be an OverflowError. 10.0.1.0/8 makes no sense, and 10.0.0.0/8 is unlikely to be desirable.
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')
Oh, well, you're right, I borked my subnet. So, for the record, I meant "10.0.0.255/24" where the issue actually appears.
I, too, was thinking /24. I think that overflowing the host portion should raise OverflowError. -- Eric.
On 3/14/2015 7:04 AM, francis wrote:
Hi,
I, too, was thinking /24. I think that overflowing the host portion should raise OverflowError.
Just curiosity, why not a modulo calculation on the subnet instead of raising the error?
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.
On 3/14/2015 4:52 PM, Eric V. Smith wrote:
On 3/14/2015 7:04 AM, francis wrote:
Hi,
I, too, was thinking /24. I think that overflowing the host portion should raise OverflowError.
Just curiosity, why not a modulo calculation on the subnet instead of raising the error?
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
That should be "I can't say I've ever needed addition at all...".
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:
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.
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