Add proper validation for updating relational fields with incorrect types. Fixes #446.

docs/topics/release-notes.md

@@ -16,6 +16,10 @@ Major version numbers (x.0.0) are reserved for project milestones.  No major poi
 
 ## 2.1.x series
 
+### Master
+
+* Bugfix: Validation errors instead of exceptions when related fields receive incorrect types.
+
 ### 2.1.15
 
 **Date**: 3rd Jan 2013
@@ -36,9 +40,9 @@ Major version numbers (x.0.0) are reserved for project milestones.  No major poi
 * Bugfix: Model fields with `blank=True` are now `required=False` by default.
 * Bugfix: Nested serializers now support nullable relationships.
 
-**Note**: From 2.1.14 onwards, relational fields move out of the `fields.py` module and into the new `relations.py` module, in order to seperate them from regular data type fields, such as `CharField` and `IntegerField`.
+**Note**: From 2.1.14 onwards, relational fields move out of the `fields.py` module and into the new `relations.py` module, in order to separate them from regular data type fields, such as `CharField` and `IntegerField`.
 
-This change will not affect user code, so long as it's following the recommended import style of `from rest_framework import serializers` and refering to fields using the style `serializers.PrimaryKeyRelatedField`.
+This change will not affect user code, so long as it's following the recommended import style of `from rest_framework import serializers` and referring to fields using the style `serializers.PrimaryKeyRelatedField`.
 
 
 ### 2.1.13

rest_framework/relations.py

@@ -276,6 +276,11 @@ class SlugRelatedField(RelatedField):
     default_read_only = False
     form_field_class = forms.ChoiceField
 
+    default_error_messages = {
+        'does_not_exist': _("Object with %s=%s does not exist."),
+        'invalid': _('Invalid value.'),
+    }
+
     def __init__(self, *args, **kwargs):
         self.slug_field = kwargs.pop('slug_field', None)
         assert self.slug_field, 'slug_field is required'
@@ -291,8 +296,11 @@ class SlugRelatedField(RelatedField):
         try:
             return self.queryset.get(**{self.slug_field: data})
         except ObjectDoesNotExist:
-            raise ValidationError('Object with %s=%s does not exist.' %
+            raise ValidationError(self.error_messages['does_not_exist'] %
                                   (self.slug_field, unicode(data)))
+        except (TypeError, ValueError):
+            msg = self.error_messages['invalid']
+            raise ValidationError(msg)
 
 
 class ManySlugRelatedField(ManyRelatedMixin, SlugRelatedField):
@@ -311,6 +319,14 @@ class HyperlinkedRelatedField(RelatedField):
     default_read_only = False
     form_field_class = forms.ChoiceField
 
+    default_error_messages = {
+        'no_match': _('Invalid hyperlink - No URL match'),
+        'incorrect_match': _('Invalid hyperlink - Incorrect URL match'),
+        'configuration_error': _('Invalid hyperlink due to configuration error'),
+        'does_not_exist': _("Invalid hyperlink - object does not exist."),
+        'invalid': _('Invalid value.'),
+    }
+
     def __init__(self, *args, **kwargs):
         try:
             self.view_name = kwargs.pop('view_name')
@@ -347,7 +363,7 @@ class HyperlinkedRelatedField(RelatedField):
         slug = getattr(obj, self.slug_field, None)
 
         if not slug:
-            raise ValidationError('Could not resolve URL for field using view name "%s"' % view_name)
+            raise Exception('Could not resolve URL for field using view name "%s"' % view_name)
 
         kwargs = {self.slug_url_kwarg: slug}
         try:
@@ -361,7 +377,7 @@ class HyperlinkedRelatedField(RelatedField):
         except:
             pass
 
-        raise ValidationError('Could not resolve URL for field using view name "%s"' % view_name)
+        raise Exception('Could not resolve URL for field using view name "%s"' % view_name)
 
     def from_native(self, value):
         # Convert URL -> model instance pk
@@ -369,7 +385,13 @@ class HyperlinkedRelatedField(RelatedField):
         if self.queryset is None:
             raise Exception('Writable related fields must include a `queryset` argument')
 
-        if value.startswith('http:') or value.startswith('https:'):
+        try:
+            http_prefix = value.startswith('http:') or value.startswith('https:')
+        except AttributeError:
+            msg = self.error_messages['invalid']
+            raise ValidationError(msg)
+
+        if http_prefix:
             # If needed convert absolute URLs to relative path
             value = urlparse(value).path
             prefix = get_script_prefix()
@@ -379,10 +401,10 @@ class HyperlinkedRelatedField(RelatedField):
         try:
             match = resolve(value)
         except:
-            raise ValidationError('Invalid hyperlink - No URL match')
+            raise ValidationError(self.error_messages['no_match'])
 
         if match.url_name != self.view_name:
-            raise ValidationError('Invalid hyperlink - Incorrect URL match')
+            raise ValidationError(self.error_messages['incorrect_match'])
 
         pk = match.kwargs.get(self.pk_url_kwarg, None)
         slug = match.kwargs.get(self.slug_url_kwarg, None)
@@ -394,14 +416,18 @@ class HyperlinkedRelatedField(RelatedField):
         elif slug is not None:
             slug_field = self.get_slug_field()
             queryset = self.queryset.filter(**{slug_field: slug})
-        # If none of those are defined, it's an error.
+        # If none of those are defined, it's probably a configuation error.
         else:
-            raise ValidationError('Invalid hyperlink')
+            raise ValidationError(self.error_messages['configuration_error'])
 
         try:
             obj = queryset.get()
         except ObjectDoesNotExist:
-            raise ValidationError('Invalid hyperlink - object does not exist.')
+            raise ValidationError(self.error_messages['does_not_exist'])
+        except (TypeError, ValueError):
+            msg = self.error_messages['invalid']
+            raise ValidationError(msg)
+
         return obj
 
 
@@ -460,7 +486,7 @@ class HyperlinkedIdentityField(Field):
         slug = getattr(obj, self.slug_field, None)
 
         if not slug:
-            raise ValidationError('Could not resolve URL for field using view name "%s"' % view_name)
+            raise Exception('Could not resolve URL for field using view name "%s"' % view_name)
 
         kwargs = {self.slug_url_kwarg: slug}
         try:
@@ -474,4 +500,4 @@ class HyperlinkedIdentityField(Field):
         except:
             pass
 
-        raise ValidationError('Could not resolve URL for field using view name "%s"' % view_name)
+        raise Exception('Could not resolve URL for field using view name "%s"' % view_name)

rest_framework/tests/relations.py

@@ -19,4 +19,15 @@ class FieldTests(TestCase):
         https://github.com/tomchristie/django-rest-framework/issues/446
         """
         field = serializers.PrimaryKeyRelatedField(queryset=NullModel.objects.all())
-        self.assertRaises(serializers.ValidationError, field.from_native, ('',))
+        self.assertRaises(serializers.ValidationError, field.from_native, '')
+        self.assertRaises(serializers.ValidationError, field.from_native, [])
+
+    def test_hyperlinked_related_field_with_empty_string(self):
+        field = serializers.HyperlinkedRelatedField(queryset=NullModel.objects.all(), view_name='')
+        self.assertRaises(serializers.ValidationError, field.from_native, '')
+        self.assertRaises(serializers.ValidationError, field.from_native, [])
+
+    def test_slug_related_field_with_empty_string(self):
+        field = serializers.SlugRelatedField(queryset=NullModel.objects.all(), slug_field='pk')
+        self.assertRaises(serializers.ValidationError, field.from_native, '')
+        self.assertRaises(serializers.ValidationError, field.from_native, [])