Skip to content

IPAddressField improvements #2747

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

Merged
merged 8 commits into from
Jun 4, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions docs/api-guide/fields.md
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,17 @@ A field that ensures the input is a valid UUID string. The `to_internal_value` m

"de305d54-75b4-431b-adb2-eb6b9e546013"

## IPAddressField

A field that ensures the input is a valid IPv4 or IPv6 string.

Corresponds to `django.forms.fields.IPAddressField` and `django.forms.fields.GenericIPAddressField`.

**Signature**: `IPAddressField(protocol='both', unpack_ipv4=False, **options)`

- `protocol` Limits valid inputs to the specified protocol. Accepted values are 'both' (default), 'IPv4' or 'IPv6'. Matching is case insensitive.
- `unpack_ipv4` Unpacks IPv4 mapped addresses like ::ffff:192.0.2.1. If this option is enabled that address would be unpacked to 192.0.2.1. Default is disabled. Can only be used when protocol is set to 'both'.

---

# Numeric fields
Expand Down
28 changes: 27 additions & 1 deletion rest_framework/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@
from django.conf import settings
from django.core.exceptions import ObjectDoesNotExist
from django.core.exceptions import ValidationError as DjangoValidationError
from django.core.validators import RegexValidator
from django.core.validators import RegexValidator, ip_address_validators
from django.forms import ImageField as DjangoImageField
from django.utils import six, timezone
from django.utils.dateparse import parse_date, parse_datetime, parse_time
from django.utils.encoding import is_protected_type, smart_text
from django.utils.translation import ugettext_lazy as _
from django.utils.ipv6 import clean_ipv6_address
from rest_framework import ISO_8601
from rest_framework.compat import (
EmailValidator, MinValueValidator, MaxValueValidator,
Expand Down Expand Up @@ -653,6 +654,31 @@ def to_representation(self, value):
return str(value)


class IPAddressField(CharField):
"""Support both IPAddressField and GenericIPAddressField"""

default_error_messages = {
'invalid': _('Enter a valid IPv4 or IPv6 address.'),
}

def __init__(self, protocol='both', **kwargs):
self.protocol = protocol.lower()
self.unpack_ipv4 = (self.protocol == 'both')
super(IPAddressField, self).__init__(**kwargs)
validators, error_message = ip_address_validators(protocol, self.unpack_ipv4)
self.validators.extend(validators)

def to_internal_value(self, data):
if data and ':' in data:
try:
if self.protocol in ('both', 'ipv6'):
return clean_ipv6_address(data, self.unpack_ipv4)
except DjangoValidationError:
self.fail('invalid', value=data)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. I think we arrived at a slight tweak to this, that involved us getting rid of the unpack_ipv4 flag and just always having that as True for 'both'. Something like this?...

if self.protocol in ('both', 'ipv6'):
    unpack_ipv4 = (self.protocol == 'both')
    return clean_ipv6_address(data)
return data

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Tom,

I'm away from my computer for few days so I will be able to reply this more properly at the end of the week. Basically I noticed that ipv4_unpack was needed anyway by the method that returns validator, that's why I left it. I need to check if tests still pass always defaulting it to True when is both. It doesn't have to be always True, nor always false.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm now back from holidays. I've given a look to the methods. The problem we may have forcing the unpack_ipv4 in case of 'both' is the following:

  • the user choose to use 'both' because he/she needs to validate both ipv4 and ipv6.
  • the use passes '192.168.0.1' as value, and the value is automatically 'unpacked'.
  • the output of 192.168.0.1 is: 2002:c0a8:0001::c0a8:0001

are you sure this is what we want?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I got this wrong again (oh gosh... I really suck with this!). unpack_ipv4 does exactly the opposite: given an ip that is an ipv6 representation of an ipv4, unpack_ipv4 transforms back the ipv6 to ipv4. This is proven by these tests https://github.com/django/django/blob/0ed7d155635da9f79d4dd67e4889087d3673c6da/tests/utils_tests/test_ipv6.py#L53


return super(IPAddressField, self).to_internal_value(data)


# Number types...

class IntegerField(Field):
Expand Down
5 changes: 5 additions & 0 deletions rest_framework/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,7 @@ class ModelSerializer(Serializer):
models.TextField: CharField,
models.TimeField: TimeField,
models.URLField: URLField,
models.GenericIPAddressField: IPAddressField,
}
serializer_related_field = PrimaryKeyRelatedField
serializer_url_field = HyperlinkedIdentityField
Expand Down Expand Up @@ -1347,6 +1348,10 @@ def get_unique_for_date_validators(self):
if hasattr(models, 'UUIDField'):
ModelSerializer.serializer_field_mapping[models.UUIDField] = UUIDField

# IPAddressField is deprecated in Django
if hasattr(models, 'IPAddressField'):
ModelSerializer.serializer_field_mapping[models.IPAddressField] = IPAddressField

if postgres_fields:
class CharMappingField(DictField):
child = CharField()
Expand Down
54 changes: 54 additions & 0 deletions tests/test_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,60 @@ class TestUUIDField(FieldValues):
field = serializers.UUIDField()


class TestIPAddressField(FieldValues):
"""
Valid and invalid values for `IPAddressField`
"""
valid_inputs = {
'127.0.0.1': '127.0.0.1',
'192.168.33.255': '192.168.33.255',
'2001:0db8:85a3:0042:1000:8a2e:0370:7334': '2001:db8:85a3:42:1000:8a2e:370:7334',
'2001:cdba:0:0:0:0:3257:9652': '2001:cdba::3257:9652',
'2001:cdba::3257:9652': '2001:cdba::3257:9652'
}
invalid_inputs = {
'127001': ['Enter a valid IPv4 or IPv6 address.'],
'127.122.111.2231': ['Enter a valid IPv4 or IPv6 address.'],
'2001:::9652': ['Enter a valid IPv4 or IPv6 address.'],
'2001:0db8:85a3:0042:1000:8a2e:0370:73341': ['Enter a valid IPv4 or IPv6 address.'],
}
outputs = {}
field = serializers.IPAddressField()


class TestIPv4AddressField(FieldValues):
"""
Valid and invalid values for `IPAddressField`
"""
valid_inputs = {
'127.0.0.1': '127.0.0.1',
'192.168.33.255': '192.168.33.255',
}
invalid_inputs = {
'127001': ['Enter a valid IPv4 address.'],
'127.122.111.2231': ['Enter a valid IPv4 address.'],
}
outputs = {}
field = serializers.IPAddressField(protocol='IPv4')


class TestIPv6AddressField(FieldValues):
"""
Valid and invalid values for `IPAddressField`
"""
valid_inputs = {
'2001:0db8:85a3:0042:1000:8a2e:0370:7334': '2001:db8:85a3:42:1000:8a2e:370:7334',
'2001:cdba:0:0:0:0:3257:9652': '2001:cdba::3257:9652',
'2001:cdba::3257:9652': '2001:cdba::3257:9652'
}
invalid_inputs = {
'2001:::9652': ['Enter a valid IPv4 or IPv6 address.'],
'2001:0db8:85a3:0042:1000:8a2e:0370:73341': ['Enter a valid IPv4 or IPv6 address.'],
}
outputs = {}
field = serializers.IPAddressField(protocol='IPv6')


# Number types...

class TestIntegerField(FieldValues):
Expand Down