classification
Title: ipaddress subnet slicing iterator malfunction
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.8, Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ebarry, era, kormat, miss-islington, 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 2018-03-21 01:58 by xiang.zhang. This issue is now closed.

Files
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
Pull Requests
URL Status Linked Edit
PR 6016 merged xiang.zhang, 2018-03-07 14:35
PR 6168 merged miss-islington, 2018-03-21 00:26
PR 6169 merged miss-islington, 2018-03-21 00:27
Messages (16)
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 iat.py
import ipaddress

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

print(list(n.hosts()))

has two different outcomes on 3.4 vs. 3.5:

$ python3.4 iat.py 
[IPv4Address('172.0.0.4'), IPv4Address('172.0.0.5')]
$ python3.5 iat.py 
[]

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:

https://hg.python.org/cpython/rev/2711677cf874
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) * (Python triager) 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(("127.0.0.4", 31)).hosts())
[]

In python3.4
>>> list(ipaddress.IPv4Network("127.0.0.4/31").hosts())
[IPv4Address('127.0.0.4'), IPv4Address('127.0.0.5')]
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(('127.0.0.4', 31)).hosts())
[]
>>> list(ipaddress.IPv4Network(('127.0.0.4/31')).hosts())
[IPv4Address('127.0.0.4'), IPv4Address('127.0.0.5')]

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(('127.0.0.4', 31)))
[IPv4Address('127.0.0.4'), IPv4Address('127.0.0.5')]
>>> list(ipaddress.IPv4Network(('127.0.0.4/31')))
[IPv4Address('127.0.0.4'), IPv4Address('127.0.0.5')]

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) * (Python triager) 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
msg314175 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2018-03-21 00:25
New changeset 10b134a07c898c2fbc5fd3582503680a54ed80a2 by Xiang Zhang in branch 'master':
bpo-27683: Fix a regression for host() of ipaddress network objects (GH-6016)
https://github.com/python/cpython/commit/10b134a07c898c2fbc5fd3582503680a54ed80a2
msg314178 - (view) Author: miss-islington (miss-islington) Date: 2018-03-21 01:22
New changeset 3326c9267f9df15fa77094b8a740be4eaa4b9374 by Miss Islington (bot) in branch '3.7':
bpo-27683: Fix a regression for host() of ipaddress network objects (GH-6016)
https://github.com/python/cpython/commit/3326c9267f9df15fa77094b8a740be4eaa4b9374
msg314181 - (view) Author: miss-islington (miss-islington) Date: 2018-03-21 01:49
New changeset ae2feb32e7eff328199ce7d527593b3c2aa1fcab by Miss Islington (bot) in branch '3.6':
bpo-27683: Fix a regression for host() of ipaddress network objects (GH-6016)
https://github.com/python/cpython/commit/ae2feb32e7eff328199ce7d527593b3c2aa1fcab
History
Date User Action Args
2018-03-21 01:58:25xiang.zhangsetstatus: open -> closed
stage: patch review -> resolved
resolution: fixed
versions: + Python 3.8, - Python 3.5
2018-03-21 01:49:44miss-islingtonsetmessages: + msg314181
2018-03-21 01:22:27miss-islingtonsetnosy: + miss-islington
messages: + msg314178
2018-03-21 00:27:22miss-islingtonsetpull_requests: + pull_request5924
2018-03-21 00:26:23miss-islingtonsetpull_requests: + pull_request5923
2018-03-21 00:25:15xiang.zhangsetmessages: + msg314175
2018-03-07 14:35:54xiang.zhangsetpull_requests: + pull_request5783
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