Navigation Menu

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

Constructor of ipaddress.IPv*Interface does not follow documentation #74076

Closed
Kentzo mannequin opened this issue Mar 23, 2017 · 11 comments
Closed

Constructor of ipaddress.IPv*Interface does not follow documentation #74076

Kentzo mannequin opened this issue Mar 23, 2017 · 11 comments
Labels
easy type-bug An unexpected behavior, bug, or error

Comments

@Kentzo
Copy link
Mannequin

Kentzo mannequin commented Mar 23, 2017

BPO 29890
Nosy @ericvsmith, @Kentzo, @zhangyangyu, @mlouielu, @humbdrag
PRs
  • bpo-29890: Fix IPv*Interface constructor when dealing with tuple #816
  • bpo-29890: Test IPv*Interface construction with tuple argument #30862
  • 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 = None
    created_at = <Date 2017-03-23.21:59:48.893>
    labels = ['easy', 'type-bug']
    title = 'Constructor of ipaddress.IPv*Interface does not follow documentation'
    updated_at = <Date 2022-01-24.21:29:56.167>
    user = 'https://github.com/Kentzo'

    bugs.python.org fields:

    activity = <Date 2022-01-24.21:29:56.167>
    actor = 'humbdrag'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = []
    creation = <Date 2017-03-23.21:59:48.893>
    creator = 'Ilya.Kulakov'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 29890
    keywords = ['patch', 'easy']
    message_count = 9.0
    messages = ['290062', '290069', '290462', '290469', '290472', '290473', '290479', '305134', '308855']
    nosy_count = 6.0
    nosy_names = ['eric.smith', 'pmoody', 'Ilya.Kulakov', 'xiang.zhang', 'louielu', 'humbdrag']
    pr_nums = ['816', '30862']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue29890'
    versions = ['Python 3.5', 'Python 3.6']

    @Kentzo
    Copy link
    Mannequin Author

    Kentzo mannequin commented Mar 23, 2017

    As per documentation, it should understand the same arguments as IPv*Network.

    Unfortunately it does not recognize netmask in string form. Hence the following code will fail:

        ipaddress.ip_interface(('192.168.1.10', '255.255.255.0'))
    
    while the following will work:
    ipaddress.ip_network(('192.168.1.10', '255.255.255.0'), strict=False)
    

    @Kentzo Kentzo mannequin added the type-bug An unexpected behavior, bug, or error label Mar 23, 2017
    @ericvsmith
    Copy link
    Member

    This should be easy enough to fix, at least in IPv4Interface.__init__. It needs to copy some of IPv4Network.__init__, dealing with address[1] and calling _make_netmask(). Currently, it just calls int(address[1]).

    I haven't looked at IPv6Interface.

    Tests are also needed, of course.

    @mlouielu
    Copy link
    Mannequin

    mlouielu mannequin commented Mar 25, 2017

    The document here says: https://docs.python.org/3/library/ipaddress.html#interface-objects

    ""IPv4Interface is a subclass of IPv4Address""

    trying with:

    >>> ipaddress.IPv4Address(('192.168.128.0', '255.255.255.0'))
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib/python3.6/ipaddress.py", line 1284, in __init__
        self._ip = self._ip_int_from_string(addr_str)
      File "/usr/lib/python3.6/ipaddress.py", line 1118, in _ip_int_from_string
        raise AddressValueError("Expected 4 octets in %r" % ip_str)
    ipaddress.AddressValueError: Expected 4 octets in "('192.168.128.0', '255.255.255.0')"

    So the behavior of IPv4Interface seem to be correct?

    @ericvsmith
    Copy link
    Member

    While an IPv4Interface may be a subclass of an IPv4Address, it definitely has more information attached to it: the netmask of the network it's on. So an interface (like a network) needs to allow additional parameters to specify the netmask.

    It should be true that IPv4Interface is like IPv4Network, but it will allow arbitrary host bits. A IPv4Network in strict mode will have all zeros for the host bits, while a IPv4Network in non-strict mode will allow ones in the host bits (see Ilya's original example).

    If you look at IPv4Interface.__init__, it actually does just that: creates an IPv4Network with strict=False, then extracts the network parts. I'm not exactly sure why it also has the int(address[1]) code there, too, since IPvv4Network deals with the address[1] part. I would think extracting _prefixlen from the network (as it does later in __init__ for the non-tuple case) would be good enough.

    @mlouielu
    Copy link
    Mannequin

    mlouielu mannequin commented Mar 25, 2017

    Eric: I made the patch, reference to which IPv*Network dealing with tuple. Should I also add the unittest for it?

    Also, can you help me code review this, thanks.

    @ericvsmith
    Copy link
    Member

    Thanks! Yes, we'll need tests.

    I'm out of town most of the weekend, but I'll look at this as soon as I can.

    @mlouielu
    Copy link
    Mannequin

    mlouielu mannequin commented Mar 25, 2017

    Add unittest. Since IPv6 do not support prefix netmask ('ffff:ff00::'), it have only test like this:

    >> a = ipaddress.ip_interface(('dead:beaf::', '32'))
    >> b = ipaddress.ip_interface('dead:beaf::/32')
    >> str(a) == str(b)

    @Kentzo
    Copy link
    Mannequin Author

    Kentzo mannequin commented Oct 27, 2017

    Can this be included into the next bugfix release?

    @Kentzo
    Copy link
    Mannequin Author

    Kentzo mannequin commented Dec 21, 2017

    Do you need any help with the change?

    @furkanonder
    Copy link
    Sponsor Contributor

    @JelleZijlstra The issue seems to be resolved in this PR.

    @JelleZijlstra
    Copy link
    Member

    Yes, thanks!

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    easy type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants