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

get_error_detail should not format without error.params #6622

Closed
6 tasks done
WolfgangFellger opened this issue Apr 30, 2019 · 1 comment
Closed
6 tasks done

get_error_detail should not format without error.params #6622

WolfgangFellger opened this issue Apr 30, 2019 · 1 comment

Comments

@WolfgangFellger
Copy link

WolfgangFellger commented Apr 30, 2019

Checklist

  • I have verified that that issue exists against the master branch of Django REST framework.
  • I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
  • This is not a usage question. (Those should be directed to the discussion group instead.)
  • This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
  • I have reduced the issue to the simplest possible case.
  • I @michael-k has included a failing test as a pull request. (Thanks!)

get_error_detail should not attempt to format the message in DjangoValidationErrors if params aren't used. This is also in line with how Django behaves.

If f-strings are used to generate the error message, this means duplicate formatting, and fails if a % occurs in the message.

Steps to reproduce

Most basic example I could muster (this is actually supposed to be ModelValidator with the validator on a Django model)

from rest_framework import serializers
from django.core.exceptions import ValidationError


def validate(value):
    raise ValidationError(f'{value} is invalid')

class MySerializer(serializers.Serializer):
    field = serializers.CharField(validators=[validate])


s = MySerializer(data={'field': 'f%'})  
if not s.is_valid():  # throws TypeError: not enough arguments for format string
    print(s.errors)

Expected behavior

Should return the error message "f% is not valid"

Actual behavior

Throws a TypeError in

ErrorDetail(error.message % (error.params or ()),

@michael-k
Copy link
Contributor

michael-k commented Apr 30, 2019

FYI: I'm working on a PR to solve this. Including a failing test. Done

Btw., this is the PR that introduced the formatting: #5785 (released with v3.9.0)

michael-k added a commit to michael-k/django-rest-framework that referenced this issue Apr 30, 2019
This prevents exceptions when the error message contains `%`, but is
not intended for formatting.  Django itself does the same:
https://github.com/django/django/blob/6866c91b638de5368c18713fa851bfe56253ea55/django/core/exceptions.py#L168-L169

Fixes encode#6622
pchiquet pushed a commit to pchiquet/django-rest-framework that referenced this issue Nov 17, 2020
This prevents exceptions when the error message contains `%`, but is
not intended for formatting.  Django itself does the same:
https://github.com/django/django/blob/6866c91b638de5368c18713fa851bfe56253ea55/django/core/exceptions.py#L168-L169

Fixes encode#6622
sigvef pushed a commit to sigvef/django-rest-framework that referenced this issue Dec 3, 2022
This prevents exceptions when the error message contains `%`, but is
not intended for formatting.  Django itself does the same:
https://github.com/django/django/blob/6866c91b638de5368c18713fa851bfe56253ea55/django/core/exceptions.py#L168-L169

Fixes encode#6622
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

2 participants