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

lib.net_pack stores wrong bit length #71

Open
FirefighterBlu3 opened this issue Jun 2, 2014 · 6 comments
Open

lib.net_pack stores wrong bit length #71

FirefighterBlu3 opened this issue Jun 2, 2014 · 6 comments

Comments

@FirefighterBlu3
Copy link

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

@commonism
Copy link

same as #66

@commonism
Copy link

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".

@FirefighterBlu3
Copy link
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:[email protected]/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.

@commonism
Copy link

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

@FirefighterBlu3
Copy link
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.

@jwp
Copy link
Contributor

jwp commented Jun 9, 2015

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
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants