Opened 9 years ago

Closed 3 years ago

#25456 closed Cleanup/optimization (fixed)

Make GenericIPAddressField normalize IPv4 addresses

Reported by: Frank Owned by: nobody
Component: Database layer (models, ORM) Version: 1.8
Severity: Normal Keywords: ipaddress, ipv4, database, normalization
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

IPv4 addresses do not get normalized before storing them in the database. They are stored as Char fields in the database which can lead to both 127.0.0.010 and 127.0.0.10 being stored in the database, although the value should be unique.

Change History (9)

comment:1 by Tim Graham, 9 years ago

Does the behavior change justify breaking backwards compatibility (silently changing the input value)? Perhaps so, considering that PostgreSQL (and its native IP address data type) won't let you store duplicate values. It reports "Key (ip)=(192.168.1.10) already exists." for the two addresses you proposed -- so is the proper normalization to remove the leading zero?

in reply to:  1 comment:2 by Frank, 9 years ago

Replying to timgraham:

Does the behavior change justify breaking backwards compatibility (silently changing the input value)? Perhaps so, considering that PostgreSQL (and its native IP address data type) won't let you store duplicate values. It reports "Key (ip)=(192.168.1.10) already exists." for the two addresses you proposed -- so is the proper normalization to remove the leading zero?

To start off, I do not really if you're advocating pro or con :-) But, I think the current situation is pretty bad. I mean, people can create models with addresses which they think have unique values. And in a sense this also goes against what you expect. You expect an generic ip address object to be evaluated as such, as an ip address, and not as a mere string value because it is stored as one. As far as I know, there is also not mention of this in the current documentation.

So, I also don't think that preserving backwards compatibility preservation is doing anyone a service in this matter. This might leave people having unreliable data.

As to what the proper normalization might be, I think removing the leading zeroes would be enough.

Last edited 9 years ago by Frank (previous) (diff)

comment:3 by Tim Graham, 9 years ago

Summary: GenericIPAddressField does not normalize IPv4 addressesMake GenericIPAddressField normalize IPv4 addresses
Triage Stage: UnreviewedAccepted

I was mostly thinking out loud.

comment:4 by KwonHan Bae, 9 years ago

I think is it possible to add django.utils.ipv4 ?
for ipv6 normalize, django have django.utils.ipv6.clean_ipv6_address.
but I think removing leading zero took about 5~6 lines function.
which namespace is proper to add this function? maybe name will be normalize_ipv4_address

comment:5 by KwonHan Bae, 9 years ago

Ok I sent PR. https://github.com/django/django/pull/5424
please check it

comment:6 by Tim Graham, 9 years ago

Has patch: set

comment:7 by Tim Graham, 9 years ago

Patch needs improvement: set

comment:8 by Sasha Gaevsky, 9 years ago

Has patch: unset
Patch needs improvement: unset

Updated ticket since related PR was closed.

comment:9 by Mariusz Felisiak, 3 years ago

Resolution: fixed
Status: newclosed

Leading zeros are not allowed anymore in IPv4 addresses. Fixed in e1d787f1b36d13b95187f8f425425ae1b98da188.

Note: See TracTickets for help on using tickets.
Back to Top