Skip to content

lib.net_pack stores wrong bit length #71

Description

@FirefighterBlu3

when packing an ip_address() for io, the bits value is wrongly stored as a /0 instead of /32 or /128

the following will fix this:

--- /usr/lib/python3.4/site-packages/postgresql/types/io/lib.py.orig    2014-06-02 12:42:50.783337348 +0000
+++ /usr/lib/python3.4/site-packages/postgresql/types/io/lib.py 2014-06-02 12:56:30.460003975 +0000
@@ -283,7 +283,9 @@
        Pack Postgres' inet/cidr data structure.
        """
        family, mask, data = triple
-       return bytes((fmap[family], mask or 0, 0 if mask is None else 1, len(data))) + data
+       mflag = mask and 1 or 0
+       mask = mask and mask or {4:32, 6:128}[family]
+       return bytes((fmap[family], mask, mflag, len(data))) + data

 def net_unpack(data,
        # Map IP version number to PGSQL src/include/utils/inet.h.

the original code stores "1.2.3.4" as "1.2.3.4/0" instead of "1.2.3.4/32". the above fix will store the correct bits value

Activity

  1. commonism commented on Jul 4, 2014

    @commonism

    same as #66

  2. commonism commented on Oct 9, 2014

    @commonism

    This patch does not work, as unpacking an ip address with bits set in the host mask like
    1.1.1.1/24
    is rejected by "ipaddress".

  3. FirefighterBlu3 commented on Jun 5, 2015

    @FirefighterBlu3
    Author

    @commonism correct. for type INET you cannot have a mask length, mask length is only valid for type CIDR. 1.1.1.1/24. unfortunately, this patch is still needed and it does work with ipaddress.

    although, my local definition for this function is slightly different, using .get() instead.

        family, bits, data = triple
        is_cidr = bits and 1 or 0
        bits    = bits and bits or {4:32, 6:128}.get(family, 32)
        return bytes((fmap[family], bits, is_cidr, len(data))) + data

    here's a sample program.

    import postgresql as pgsql
    
    pgdb = pgsql.open('pq://xx:xx@10.255.0.2/vse?[sslmode]=prefer')
    
    pgdb.execute('DROP TABLE IF EXISTS test_table_a')
    pgdb.execute('CREATE TABLE test_table_a (a INET, b CIDR)')
    
    s = pgdb.prepare('INSERT INTO test_table_a (a,b) VALUES ($1,$2)')
    
    s('1.1.1.1', '2.2.2.2')      # CIDR default address will yield  2.2.2.2/32
    
    try:
      s('1.1.1.1/24', '2.2.2.2')
    except:
      print('invalid notation for type INET')
    
    s('1.1.1.1', '2.2.0.0/16')   # 2.2.2.2/16 will by default raise: DETAIL: Value has bits set to right of mask
    
    q = pgdb.query('SELECT * FROM test_table_a')
    
    for _ in q:
      print(_)
    
    pgdb.close()

    this results in the following output:

    ~$ python /tmp/p.py
    invalid notation for type INET
    (IPv4Address('1.1.1.1'), IPv4Network('2.2.2.2/32'))
    (IPv4Address('1.1.1.1'), IPv4Network('2.2.0.0/16'))
    

    unmodified lib.py:

    vse=# select * from test_table_a;
        a    |     b
    ---------+------------
     1.1.1.1/0 | 2.2.2.2/32
     1.1.1.1/0 | 2.2.0.0/16
    (2 rows)

    with my patch:

    vse=# select * from test_table_a;
        a    |     b
    ---------+------------
     1.1.1.1 | 2.2.2.2/32
     1.1.1.1 | 2.2.0.0/16
    (2 rows)

    1.1.1.1/0 causes problems with other software and should be in INET format which has no mask length.

  4. commonism commented on Jun 8, 2015

    @commonism

    You are wrong with your assumption INET can not have a mask.

    http://www.postgresql.org/docs/9.4/static/datatype-net-types.html

    8.9.1. inet
    The inet type holds an IPv4 or IPv6 host address, and optionally its subnet, all in one field. The subnet is represented by the number of network address bits present in the host address (the "netmask"). If the netmask is 32 and the address is IPv4, then the value does not indicate a subnet, only a single host. In IPv6, the address length is 128 bits, so 128 bits specify a unique host address. Note that if you want to accept only networks, you should use the cidr type rather than inet.

    The input format for this type is address/y where address is an IPv4 or IPv6 address and y is the number of bits in the netmask. If the /y portion is missing, the netmask is 32 for IPv4 and 128 for IPv6, so the value represents just a single host. On display, the /y portion is suppressed if the netmask specifies a single host.

    INET allows a mask.

    8.9.3. inet vs. cidr
    The essential difference between inet and cidr data types is that inet accepts values with nonzero bits to the right of the netmask, whereas cidr does not.

    INET could be mapped to IPvXInterface.
    https://docs.python.org/3.5/library/ipaddress.html#ipaddress.IPv4Interface

  5. FirefighterBlu3 commented on Jun 9, 2015

    @FirefighterBlu3
    Author

    My presentation was unclear and my intent was to point out that the core module ipaddress was unable to handle INET with a netmask, I apologize. For PostgreSQL type INET, you cannot have netmask bits set when using ipaddress.ip_address(). ip_address() will reject the data if a bitmask is present, and ipaddress is the default translator for py-postgresql in Python3.

    >>> ipaddress.ip_address('1.1.1.1')
    IPv4Address('1.1.1.1')
    >>> ipaddress.ip_address('1.1.1.1/32')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib64/python3.4/ipaddress.py", line 54, in ip_address
        address)
    ValueError: '1.1.1.1/32' does not appear to be an IPv4 or IPv6 address
    >>> ipaddress.ip_network('1.1.1.1/32')
    IPv4Network('1.1.1.1/32')

    ipaddress.ip_network() can accept address data (and will translate) wherein the data appears to be a host but has a less than full width mask set.

    >>> ipaddress.ip_network('1.1.1.1/24')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib64/python3.4/ipaddress.py", line 74, in ip_network
        return IPv4Network(address, strict)
      File "/usr/lib64/python3.4/ipaddress.py", line 1490, in __init__
        raise ValueError('%s has host bits set' % self)
    ValueError: 1.1.1.1/24 has host bits set
    >>> ipaddress.ip_network('1.1.1.1/24', strict=False)
    IPv4Network('1.1.1.0/24')

    Previous to my patch, py-postgresql was storing INET addresses with a netmask indicated as /0. PostgreSQL is happy storing this data with a mask length, but querying it results in exceptions in the form of:

    >>> ipaddress.ip_address('1.1.1.1/0')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib64/python3.4/ipaddress.py", line 54, in ip_address
        address)
    ValueError: '1.1.1.1/0' does not appear to be an IPv4 or IPv6 address

    It's fine if other software is capable of using pgsql type INET with netmask bits. py-postgresql's data type translator uses ipaddress in Python3 and the basic INSERT/SELECT out of the box, will break.

    In the past I had a modified ipaddress module that handled the hosts with a netmask length but the notion of least surprise is that these functions should work out of the box without having to modify core modules.

  6. jwp commented on Jun 9, 2015

    @jwp
    Contributor

    I think I remember running into this. It was a little annoying having to strip out the /0 from ''::inet::text casts. Provide some tests with the patch and I'll apply it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions