Title: ipaddress subnet slicing iterator malfunction
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.7, Python 3.6, Python 3.5
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: ebarry, era, kormat, ncoghlan, pitrou, pmoody, r.david.murray, serhiy.storchaka, tklausmann, xiang.zhang
Priority: high Keywords: 3.5regression, patch

Created on 2016-08-04 13:07 by tklausmann, last changed 2016-11-01 09:55 by era.

File name Uploaded Description Edit
ipaddress_iter_fix.patch ebarry, 2016-08-04 14:51 review
issue27683.patch xiang.zhang, 2016-08-23 07:32 review
Messages (13)
msg271972 - (view) Author: (tklausmann) Date: 2016-08-04 13:07
Between Python 3.4 (3.4.5 I've tested with) and Python 3.5 (3.5.2), the behavior of ipaddress's subnet() method changed.

This code:

$ cat
import ipaddress

p = ipaddress.IPv4Network("")
subnets = p.subnets(new_prefix=31)
n = next(subnets)


has two different outcomes on 3.4 vs. 3.5:

$ python3.4 
[IPv4Address(''), IPv4Address('')]
$ python3.5 

AIUI, the 3.4 behavior is the correct one (or the docs need to be fixed).
msg271974 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-08-04 13:21
If someone wants to bisect to find out which changeset introduced the behavior change that would be helpful.
msg271976 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-08-04 13:43
The change was introduced in issue21487.
msg271979 - (view) Author: (tklausmann) Date: 2016-08-04 14:22
I just bisected it and the specific commit is this one:
msg271981 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-08-04 14:47
Since this is a regression, I'm going to claim this has higher than normal priority, for whatever good that will do :)
msg271982 - (view) Author: Emanuel Barry (ebarry) * Date: 2016-08-04 14:51
Here's a patch that fixes this bug, test included. I made this against the 3.5 branch, but should apply cleanly on default.
msg271983 - (view) Author: Stephen Shirley (kormat) Date: 2016-08-04 14:53
The bug appears to be in the new form of the constructor. Here's a more minimal reproduction:

In python3.5:
>>> list(ipaddress.IPv4Network(("", 31)).hosts())

In python3.4
>>> list(ipaddress.IPv4Network("").hosts())
[IPv4Address(''), IPv4Address('')]
msg272018 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-08-05 06:32
As Stephen notes, the underlying problem appears to be a behavioural difference between two theoretically equivalent ways of defining a network:

>>> list(ipaddress.IPv4Network(('', 31)).hosts())
>>> list(ipaddress.IPv4Network(('')).hosts())
[IPv4Address(''), IPv4Address('')]

Now, the first case is the documented behaviour: hosts() is *supposed to* exclude the network and broadcast address, and those are the only addresses in a /31.

If you want to iterate over all the *addresses* in a network (including the network and broadcast addresses) then you need to iterate over the network object directly:

>>> list(ipaddress.IPv4Network(('', 31)))
[IPv4Address(''), IPv4Address('')]
>>> list(ipaddress.IPv4Network(('')))
[IPv4Address(''), IPv4Address('')]

However, as Emanuel found when writing his patch, there's currently an undocumented special case for /31 networks: the definition of "hosts" is *implicitly changed* for such instances to include the nominal network and broadcast address (by setting "self.hosts = self.__iter__"), presumably on the assumption that such networks represent a point-to-point link between two hosts, so the concepts of "network address" and "broadcast address" don't really apply.

That special case seems pragmatically useful, so I think the right fix would be to:

- document the special case that for /31 networks, hosts() includes the network and broadcast addresses (on the assumption the "network" is actually a point-to-point link between two hosts)
- refactor IPv4Network.__init__ to first map the supplied input to a "candidate_address" and "candidate_netmask" and then use *common* validation logic to determine the actual network address and netmask (this will also address the "# fixme" comment for the int/bytes case)
- check whether or not IPv6Network is affected by the same behavioural discrepancy
msg272031 - (view) Author: Emanuel Barry (ebarry) * Date: 2016-08-05 14:10
Ack, special cases! I can look into making a patch, but I'm not really acquainted with ipaddress or the relevant protocols, so it might not be optimal. I'll give it a shot next week if nobody else does.
msg273419 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-08-23 07:32
issue27683.patch tries to fix this. It alters the doc and refactors the constructor, so there is no difference between different arguments

As for IPv6Network, it has the same problem.
msg274756 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-09-07 04:30
ping :)
msg279609 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-10-28 15:35
Ping. ;-)
msg279866 - (view) Author: (era) Date: 2016-11-01 09:55
#28577 requests a similar special case for /32
Date User Action Args
2016-11-01 09:55:55erasetnosy: + era
messages: + msg279866
2016-10-28 15:35:27xiang.zhangsetnosy: + pmoody
messages: + msg279609
2016-09-27 12:09:16serhiy.storchakasetstage: needs patch -> patch review
versions: + Python 3.7
2016-09-07 04:30:11xiang.zhangsetmessages: + msg274756
2016-08-23 07:32:53xiang.zhangsetfiles: + issue27683.patch
nosy: + xiang.zhang
messages: + msg273419

2016-08-05 14:10:42ebarrysetmessages: + msg272031
stage: patch review -> needs patch
2016-08-05 06:32:20ncoghlansetmessages: + msg272018
2016-08-04 14:53:19kormatsetmessages: + msg271983
2016-08-04 14:51:23ebarrysetstage: patch review
2016-08-04 14:51:02ebarrysetfiles: + ipaddress_iter_fix.patch

nosy: + ebarry
messages: + msg271982

keywords: + patch
2016-08-04 14:47:46r.david.murraysetkeywords: + 3.5regression
priority: normal -> high
messages: + msg271981

versions: + Python 3.6
2016-08-04 14:22:48tklausmannsetmessages: + msg271979
2016-08-04 13:43:29serhiy.storchakasetnosy: + pitrou, serhiy.storchaka, ncoghlan
messages: + msg271976
2016-08-04 13:21:34r.david.murraysetnosy: + r.david.murray
messages: + msg271974
2016-08-04 13:08:05tklausmannsetnosy: + kormat
2016-08-04 13:07:37tklausmanncreate