Opened 3 years ago

Closed 3 years ago

#24639 closed Bug (wontfix)

Interaction between ValidationError and mark_safe

Reported by: jambonrose Owned by: jambonrose
Component: Forms Version: 1.8
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Succinctly: the ValidationError message only respects mark_safe if the params argument is not used.

Current behavior:

message params output
text none text
safe none safe
text text text
safe text text
text safe text
safe safe text

Given that, according to the documentation, the best way to use ValidationError is with the params argument, it seems desirable to enable using mark_safe when using params, as this would enable us to code:

# method for a Form subclass
def clean_slug(self):
    new_slug = self.cleaned_data['slug'].lower()
    if new_slug == 'invalid_value':
        raise ValidationError(
            # _ is ugettext
            mark_safe(_('SlugField may not be '
                        '"%(slug_value)s" '
                        'for URL reasons.')),
            params={
                'slug_value':
                    mark_safe('<code>invalid_value</code>')})
    return new_slug

I think desired behavior ought to be:

message params output
text none text
safe none safe
text text text
safe text text
text safe text
safe safe safe


For more information about the issue, please see my message on django-developers.

Change History (4)

comment:1 Changed 3 years ago by jambonrose

I've just issued a PR.

I have a few concerns:

  1. Am I accidentally introducing a security issue?
  2. Is the behavior still too simple (should the text/safe or safe/text combo do something more complicated)?
  3. Are the tests in the current pull requests thorough enough?

I would appreciate any feedback.

comment:2 Changed 3 years ago by Harry Percival

Triage Stage: UnreviewedAccepted

comment:3 Changed 3 years ago by Tim Graham

Has patch: set

comment:4 Changed 3 years ago by Tim Graham

Resolution: wontfix
Status: newclosed

From Loic on the pull request:

IMO ValidationError should be HTML agnostic since it's the responsibility of ErrorList/Dict to deal with the interaction with HTML. This also seems pretty fragile, potentially unused params dictate the final outcome and so do safe params such as int and float. HTML in Python code is not something we want to promote anyway (HTML belongs in templates). The logic behind using params is to make things as atomic as possible to give more flexibility to the user of the form. If you are going to couple two different technologies, you might as well interpolate the final message yourself.

If we really wanted to deal with interpolation/formatting of SafeData it should be addressed using __format__ and __mod__ on the SafeData object itself.

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