Skip to content

ipaddress should accept bytearray in addition to bytes #78646

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

Open
joernheissler mannequin opened this issue Aug 22, 2018 · 10 comments
Open

ipaddress should accept bytearray in addition to bytes #78646

joernheissler mannequin opened this issue Aug 22, 2018 · 10 comments
Labels
3.8 (EOL) end of life stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@joernheissler
Copy link
Mannequin

joernheissler mannequin commented Aug 22, 2018

BPO 34465
Nosy @gpshead, @ncoghlan, @serhiy-storchaka, @zhangyangyu, @joernheissler, @prudvinit
PRs
  • gh-78646: Added support for bytearrray type , to create IPv4 addresss #8908
  • 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 2018-08-22.19:48:00.413>
    labels = ['3.8', 'type-feature', 'library']
    title = 'ipaddress should accept bytearray in addition to bytes'
    updated_at = <Date 2018-09-11.16:35:00.331>
    user = 'https://github.com/joernheissler'

    bugs.python.org fields:

    activity = <Date 2018-09-11.16:35:00.331>
    actor = 'joernheissler'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2018-08-22.19:48:00.413>
    creator = 'joernheissler'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 34465
    keywords = ['patch']
    message_count = 10.0
    messages = ['323906', '323982', '324047', '324050', '324051', '324052', '324437', '324969', '325017', '325024']
    nosy_count = 7.0
    nosy_names = ['gregory.p.smith', 'ncoghlan', 'pmoody', 'serhiy.storchaka', 'xiang.zhang', 'joernheissler', 'prudvinit']
    pr_nums = ['8908']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue34465'
    versions = ['Python 3.8']

    @joernheissler
    Copy link
    Mannequin Author

    joernheissler mannequin commented Aug 22, 2018

    Hi,

    the ipaddress module accepts bytes' objects in the constructors. bytearray' however is not supported, see paste below.

    Should this be supported too?

    >> import ipaddress

    >>> ipaddress.IPv4Address(bytes([127, 0, 0, 1]))
    IPv4Address('127.0.0.1')
    
    >>> ipaddress.IPv4Address(bytearray([127, 0, 0, 1]))
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib/python3.7/ipaddress.py", line 1301, in __init__
        self._ip = self._ip_int_from_string(addr_str)
      File "/usr/lib/python3.7/ipaddress.py", line 1135, in _ip_int_from_string
        raise AddressValueError("Expected 4 octets in %r" % ip_str)
    ipaddress.AddressValueError: Expected 4 octets in "bytearray(b'\\x7f\\x00\\x00\\x01')"

    @joernheissler joernheissler mannequin added 3.7 (EOL) end of life 3.8 (EOL) end of life stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Aug 22, 2018
    @prudvinit
    Copy link
    Mannequin

    prudvinit mannequin commented Aug 24, 2018

    I think it should be supported too. How about a list of bytes? eg : [127,0,0,1].. Should that be supported as well?

    @serhiy-storchaka
    Copy link
    Member

    What is your use case?

    New features can be added only in the developed version.

    @serhiy-storchaka serhiy-storchaka removed the 3.7 (EOL) end of life label Aug 25, 2018
    @joernheissler
    Copy link
    Mannequin Author

    joernheissler mannequin commented Aug 25, 2018

    My use case is parsing binary data. For that I use a bytearray as a buffer. buf[:4] gets me another bytearray which I'd want to convert to an ipaddress.

    I can't think of a usecase for list-of-int.

    @zhangyangyu
    Copy link
    Member

    Why not just bytes(buf[:4]) before passing?

    @joernheissler
    Copy link
    Mannequin Author

    joernheissler mannequin commented Aug 25, 2018

    That's what I'm doing now.
    But it would be more convenient if I could pass a bytearray.

    @zhangyangyu
    Copy link
    Member

    I'm -1 on this change. I think the workaround is easy and direct.

    @gpshead
    Copy link
    Member

    gpshead commented Sep 11, 2018

    This isn't limited to just IPv4Address, the Network classes and the IPv6 classes that accept bytes should also be updated for consistency if we're going to do this.

    (bytes, bytearray, memoryview) make sense to support, and explicitly test in unittests.

    Until then, the workaround is to call bytes on the relevant slice of those.

    @serhiy-storchaka
    Copy link
    Member

    See bpo-27572 for moving in opposite direction. Supporting the buffer protocol rather of just bytes and bytearray complicates the code, and can be considered as a feature creep, especially if an input is a text encoded as bytes.

    But in this case the bytes argument of IPv4Address() is not a text representation like b'127.0.0.1', but a packed array of bytes like bytes([127, 0, 0, 1]). Accepting bytearray and memoryview makes more sense for it. Yet I'm not sure that supporting them is worth adding an overhead. This will affect every call of the IPv4Address constructor, and I think that this is not the most common case.

    Maybe add a special purposed named constructor IPv4Address.from_bytes() that will accept any objects supporting the buffer protocol?

    @joernheissler
    Copy link
    Mannequin Author

    joernheissler mannequin commented Sep 11, 2018

    Maybe add a special purposed named constructor IPv4Address.from_bytes() that will accept any objects supporting the buffer protocol?

    That would work for me.
    I wonder if there should be something like ipaddress.ip_address_from_bytes too that can construct IPv4Adress or IPv6Address.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 (EOL) end of life stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants