Opened 2 years ago

Closed 2 years ago

#20650 closed Bug (fixed)

Filter tag accepts escape filter as a parameter

Reported by: grzesiof@… Owned by: nobody
Component: Template system Version: 1.5
Severity: Normal Keywords:
Cc: bmispelon@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

The docs state that the filter tag doesn't work with safe or escape (https://docs.djangoproject.com/en/dev/ref/templates/builtins/#filter).
But because of a bug in the code it does. This piece of the filter implementation is supposed to handle it:

if getattr(func, '_decorated_function', func).__name__ in ('escape', 'safe'):
            raise TemplateSyntaxError

(https://github.com/django/django/blob/master/django/template/defaulttags.py#L668)

But because of the way safe and escape are defined, the decorated name is matching the filter name only for safe:

@register.filter("escape", is_safe=True)
def escape_filter(value):

@register.filter(is_safe=True)
def safe(value):

https://github.com/django/django/blob/master/django/template/defaultfilters.py#L442
https://github.com/django/django/blob/master/django/template/defaultfilters.py#L400

Perhaps the easiest solution would be to alter the escape_filter definition.

Attachments (1)

tests-ticket-20650.diff (898 bytes) - added by bmispelon 2 years ago.

Download all attachments as: .zip

Change History (6)

Changed 2 years ago by bmispelon

comment:1 Changed 2 years ago by bmispelon

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 2 years ago by bmispelon

Turns out it's an (old) regression introduced by commit 69cfee2f167f64c15de6bfcd6b9779534132db2f.

Thanks for the detailed report by the way.

comment:3 Changed 2 years ago by bmispelon

  • Cc bmispelon@… added

Not sure what's the best way to go about fixing this is.
I see three options:

1) Replace "escape" with "escape_filter" (the actual name of the function).

While this would work, it'd be pretty fragile since it would break (again) if someone was to rename the function.

2) Add an attribute on the filter function to store the corresponding filter's name.

This can be done in template.Library.filter() quite easily and it seems to work. It should be fully backwards-compatible too.

3) Add an boolean attribute on the filter function that indicates whether this filter can be used as an argument to {% filter %}.

This would have the added benefit of allowing third-party filters to declare that they shouldn't be used with {% filter %}.

I think option 1) is not advisable, but I'm not sure from 2) or 3) which one is better.

comment:4 Changed 2 years ago by bmispelon

  • Has patch set

I've looked at it a bit more and decided to go with 2).

The problem with 3) is that it'd be tricky to keep the same helpful error message without complicating the code too much (right now, since we now which filters are forbidden, we can point to the correct usage in the error message).

I also decided to make this new _filter_name attribute "private" and undocumented so that we can improve on it if needed.

The complete patch ends up being quite simple and can be found there: https://github.com/django/django/pull/1304

comment:5 Changed 2 years ago by Baptiste Mispelon <bmispelon@…>

  • Resolution set to fixed
  • Status changed from new to closed

In ec371ace004203100d24a74edafc16534dd5d5a9:

Fixed #20650 -- Fixed {% filter %} incorrectly accepting 'escape' as argument

Thanks to grzesiof for the report and to loic84 and Alex Gaynor
for the review.

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