Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ipaddress.IPv6Network.hosts function omits network and broadcast addresses #63356

Closed
m01 mannequin opened this issue Oct 3, 2013 · 10 comments
Closed

ipaddress.IPv6Network.hosts function omits network and broadcast addresses #63356

m01 mannequin opened this issue Oct 3, 2013 · 10 comments
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@m01
Copy link
Mannequin

m01 mannequin commented Oct 3, 2013

BPO 19157
Nosy @ncoghlan
Files
  • ipaddress_ipv6_hosts.diff: naive implementation of proposed fix Support "bpo-" in Misc/NEWS #1
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2014-03-11.16:57:50.121>
    created_at = <Date 2013-10-03.22:51:31.222>
    labels = ['type-bug', 'library']
    title = 'ipaddress.IPv6Network.hosts function omits network and broadcast addresses'
    updated_at = <Date 2014-03-11.22:09:43.447>
    user = 'https://bugs.python.org/m01'

    bugs.python.org fields:

    activity = <Date 2014-03-11.22:09:43.447>
    actor = 'ncoghlan'
    assignee = 'pmoody'
    closed = True
    closed_date = <Date 2014-03-11.16:57:50.121>
    closer = 'pmoody'
    components = ['Library (Lib)']
    creation = <Date 2013-10-03.22:51:31.222>
    creator = 'm01'
    dependencies = []
    files = ['31956']
    hgrepos = []
    issue_num = 19157
    keywords = ['patch']
    message_count = 10.0
    messages = ['198927', '209927', '209929', '210667', '210668', '210836', '213150', '213175', '213177', '213179']
    nosy_count = 4.0
    nosy_names = ['ncoghlan', 'pmoody', 'python-dev', 'm01']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = None
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue19157'
    versions = ['Python 3.5']

    @m01
    Copy link
    Mannequin Author

    m01 mannequin commented Oct 3, 2013

    (See also: http://stackoverflow.com/q/19159168/1298153. This is my first bug submission)
    Contrary to IPv4, IPv6 does not have a concept of network and broadcast addresses. It looks like the same code is used to generate the hosts for both IPv4 and IPv6, resulting in the network and broadcast addresses being omitted in IPv6, even though these are valid IPv6 addresses, albeit they may have a special meaning.

    Repro:

    $ python3
    Python 3.3.2 (default, Sep  6 2013, 09:30:10) 
    [GCC 4.8.1 20130725 (prerelease)] on linux
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import ipaddress
    >>> print("\n".join([str(x) for x in ipaddress.ip_network("2001:0db8::/120").hosts()]))
    2001:db8::1
    2001:db8::2
    ...
    2001:db8::fe
    >>> 
    >>> hex(int(ipaddress.ip_address('2001:db8::fe')))
    '0x20010db80000000000000000000000fe'

    The v4 docs state, that the hosts() function...
    "Returns an iterator over the usable hosts in the network. The usable hosts are all the IP addresses that belong to the network, except the network address itself and the network broadcast address."

    Assuming hosts excludes routers, this should probably return a generator/an iterator covering all addresses in the network, apart from the very first one (all 0's after the prefix), since this is typically the subnet-router anycast address (http://tools.ietf.org/html/rfc4291#section-2.6.1).

    While the top range (including the currently omitted prefix + "all 1's" address) contains reserved anycast addresses (http://tools.ietf.org/html/rfc2526), I'm not sure whether excluding them is a great idea, as in the future some of these may become valid (anycast) host addresses.

    Backwards compatibility considerations:
    The proposed fix would add one extra address to the ones already returned. I think this is less likely to break code, than, say, removing all the reserved anycast addresses.

    Implementation options:

    1. override _BaseNetwork hosts() implementation in IPv6Network
    2. add an is_host_address(ip_address) method, or something along those lines, to IPv4 and IPv6Address classes, and change the hosts() function to only return addresses for which is_host_address is True. This function might be generally useful, but the implementation is likely to be slower than 1.
    3. use address_exclude perhaps..

    I'm sure there are many other ways to implement the fix, but I've attached a "naive" diff. I'm not sure if there's a better way to implement it that doesn't involve copying code from hosts()/iter and slightly tweaking it.

    With 1:
    >>> print("\n".join([str(x) for x in ipaddress.ip_network("2001:0db8::/124").hosts()]))
    2001:db8::1
    2001:db8::2
    2001:db8::3
    2001:db8::4
    2001:db8::5
    2001:db8::6
    2001:db8::7
    2001:db8::8
    2001:db8::9
    2001:db8::a
    2001:db8::b
    2001:db8::c
    2001:db8::d
    2001:db8::e
    2001:db8::f

    @m01 m01 mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Oct 3, 2013
    @ncoghlan
    Copy link
    Contributor

    ncoghlan commented Feb 1, 2014

    Peter, could you take a look at this one? The status quo seems reasonable to me (and I assume to you since the stdlib ipaddress matches the way ipaddr handles this case), but there are details to Michiel's proposal that I'm not able to adequately assess.

    However, also changing the target version to 3.5 - even if this behaviour was tweaked, it's unlikely to be something we would adjust in a maintenance release.

    @pmoody
    Copy link
    Mannequin

    pmoody mannequin commented Feb 2, 2014

    Ack. My first impression is that #1 is probably the right way to do this. I'm arguing with hg about the right way to stash a change, but I'll get this fixed.

    @pmoody pmoody mannequin self-assigned this Feb 2, 2014
    @pmoody
    Copy link
    Mannequin

    pmoody mannequin commented Feb 8, 2014

    Since there's no broadcast address for ipv6, should calling .broadcast* on an ipv6 address raise an exception?

    @pmoody
    Copy link
    Mannequin

    pmoody mannequin commented Feb 8, 2014

    Hey Michiel, the patch looks good to me. Have you signed the contributor license agreement?

    http://www.python.org/psf/contrib/contrib-form/

    Nick, is there anyway for me to check if this has been signed?

    @m01
    Copy link
    Mannequin Author

    m01 mannequin commented Feb 10, 2014

    Hey Peter,

    Cool, thanks for the feedback!
    Looks like my email replies didn't get attached to the bug tracker..

    I've just signed the form.

    Regarding the .broadcast* question, I think that's probably best handled in a separate issue, but I think there are two other options in addition to the one you suggested: a) ensuring the .broadcast_address() function isn't actually defined for IPv6Networks (this would also trigger an exception if someone tried to call it, like you suggested) or b) making it return the link-local all-nodes multicast address. Personally speaking I'd probably favour option a, but I'm still new to Python standard library development ;-)

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 11, 2014

    New changeset b6271cbcc762 by Peter Moody in branch 'default':
    Issue bpo-19157: Include the broadcast address in the usuable hosts for IPv6
    http://hg.python.org/cpython/rev/b6271cbcc762

    @pmoody pmoody mannequin closed this as completed Mar 11, 2014
    @ncoghlan
    Copy link
    Contributor

    Peter, just confirming: you consider this a bug fix rather than a new
    feature? I ask as default is currently still 3.4.1, which I find slightly
    confusing myself (it will switch to 3.5 after the 3.4.0 final release this
    weekend)

    @pmoody
    Copy link
    Mannequin

    pmoody mannequin commented Mar 11, 2014

    Yes, I think omitting the broadcast address in the usable hosts was a bug.

    @ncoghlan
    Copy link
    Contributor

    Cool, thanks for clarifying.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant