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

Handle substitution errors with no parameters to ValidationError #9313

Closed
wants to merge 8 commits into from
Closed
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: 9 additions & 2 deletions rest_framework/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ def __init__(self, detail=None, code=None, params=None):
# For validation failures, we may collect many errors together,
# so the details should always be coerced to a list if not already.
if isinstance(detail, str):
detail = [detail % params]
detail = [self.template_detail(detail, params)]
elif isinstance(detail, ValidationError):
detail = detail.detail
elif isinstance(detail, (list, tuple)):
Expand All @@ -166,13 +166,20 @@ def __init__(self, detail=None, code=None, params=None):
if isinstance(detail_item, ValidationError):
final_detail += detail_item.detail
else:
final_detail += [detail_item % params if isinstance(detail_item, str) else detail_item]
final_detail += [self.template_detail(detail_item, params) if isinstance(detail_item, str) else detail_item]
detail = final_detail
elif not isinstance(detail, dict) and not isinstance(detail, list):
detail = [detail]

self.detail = _get_error_details(detail, code)

def template_detail(self, detail, params):
"""Handle error messages with templates and placeholders."""
try:
return detail % params
except (KeyError, ValueError, TypeError, IndexError, AttributeError):
return detail


class ParseError(APIException):
status_code = status.HTTP_400_BAD_REQUEST
Expand Down
90 changes: 90 additions & 0 deletions tests/test_validation_error.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import pytest
from django.test import TestCase

from rest_framework import serializers, status
Expand Down Expand Up @@ -195,3 +196,92 @@ def test_validation_error_details_validation_errors_nested_list(self):
assert str(error.detail[1]) == 'Invalid value: 43'
assert str(error.detail[2]) == 'Invalid value: 44'
assert str(error.detail[3]) == 'Invalid value: 45'

def test_validation_error_without_params_string_templating(self):
"""Ensure that substitutable errors can be emitted without params."""

# mimic the logic in fields.Field.run_validators by saving the exception
# detail into a list which will then be the detail for a new ValidationError.
# this should not throw a KeyError or a TypeError even though
# the string has a substitutable substring ...
errors = []
try:
raise ValidationError(detail='%(user)s')
except ValidationError as exc:
errors.extend(exc.detail)

# ensure it raises the correct exception type as an input to a new ValidationError
with pytest.raises(ValidationError):
raise ValidationError(errors)

def test_validation_error_without_params_digit(self):
"""Ensure that substitutable errors can be emitted with a digit placeholder."""

# mimic the logic in fields.Field.run_validators by saving the exception
# detail into a list which will then be the detail for a new ValidationError.
# this should not throw a TypeError on the date format placeholders ...
errors = []
try:
raise ValidationError(detail='%d')
except ValidationError as exc:
errors.extend(exc.detail)

# ensure it raises the correct exception type as an input to a new ValidationError
with pytest.raises(ValidationError):
raise ValidationError(errors)

def test_validation_error_without_params_date_formatters(self):
"""Ensure that substitutable errors can be emitted with invalid template placeholders."""

# mimic the logic in fields.Field.run_validators by saving the exception
# detail into a list which will then be the detail for a new ValidationError.
# this should not throw a ValueError on the date format placeholders ...
errors = []
try:
raise ValidationError(detail='Expects format %Y-%m-%d %H:%M:%S')
except ValidationError as exc:
errors.extend(exc.detail)

# ensure it raises the correct exception type as an input to a new ValidationError
with pytest.raises(ValidationError):
raise ValidationError(errors)

def test_validation_error_with_param_that_has_attribute_error(self):
"""Ensure that substitutable errors can be emitted with a bad string repr."""

class FooBar:
def __str__(self):
raise AttributeError("i was poorly coded")

# mimic the logic in fields.Field.run_validators by saving the exception
# detail into a list which will then be the detail for a new ValidationError.
# this should not throw a ValueError on the date format placeholders ...
errors = []
try:
raise ValidationError(detail='%s', params=FooBar())
except ValidationError as exc:
errors.extend(exc.detail)

# ensure it raises the correct exception type as an input to a new ValidationError
with pytest.raises(ValidationError):
raise ValidationError(errors)

def test_validation_error_with_param_that_has_index_error(self):
"""Ensure that substitutable errors can be emitted with a bad string repr."""

class FooBar:
def __str__(self):
raise IndexError("i was poorly coded")

# mimic the logic in fields.Field.run_validators by saving the exception
# detail into a list which will then be the detail for a new ValidationError.
# this should not throw a ValueError on the date format placeholders ...
errors = []
try:
raise ValidationError(detail='%s', params=FooBar())
except ValidationError as exc:
errors.extend(exc.detail)

# ensure it raises the correct exception type as an input to a new ValidationError
with pytest.raises(ValidationError):
raise ValidationError(errors)
Loading