Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#8966 closed (fixed)

{% if "x"|length_is:n %} always evaluates True

Reported by: Thomas Steinacher <tom@…> Owned by: carljm
Component: Template system Version: master
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

{% if "x"|capfirst|length_is:n %} is always true. Example:

In [1]: from django.template import Template, Context

In [2]: Template('{% if "x"|capfirst|length_is:0 %}This is wrong{% endif %}').render(Context({}))
Out[2]: u'This is wrong'

Using latest Django SVN.

Attachments (3)

8966_test.diff (866 bytes) - added by carljm 7 years ago.
failing test
8966_naive_fix.diff (471 bytes) - added by carljm 7 years ago.
simple fix (remove is_safe from length_is filter)
8966_with_docs.diff (1.3 KB) - added by carljm 7 years ago.
same patch, with warning note in docs

Download all attachments as: .zip

Change History (9)

comment:1 Changed 7 years ago by carljm

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Summary changed from {% if "x"|capfirst|length_is:n %} always evaluates True to {% if "x"|length_is:n %} always evaluates True
  • Version changed from 1.0 to SVN

capfirst is irrelevant. This also fails:

>>> Template('{% if "x"|length_is:0 %}Wrong{% endif %}').render(Context({}))
u'Wrong'

Changed summary to reflect this. Also changed version to SVN, as this problem is still present in trunk.

The problem here is that the is_safe attribute is set on the length_is tag, which causes mark_safe to be called on the return value, and mark_safe(False) returns the string 'False'. Not sure where in that chain is the proper place for the fix. It's possible that mark_safe needs to be smart about not always coercing its argument to a string, or FilterExpression.resolve needs to check if the return value is a string before calling mark_safe on it. But the naive fix is just to remove the is_safe attribute from length_is, since auto-escaping will never harm its return value.

Changed 7 years ago by carljm

failing test

Changed 7 years ago by carljm

simple fix (remove is_safe from length_is filter)

comment:2 Changed 7 years ago by kratorius

  • Component changed from Uncategorized to Template system

comment:3 Changed 7 years ago by SmileyChris

  • Triage Stage changed from Unreviewed to Ready for checkin

Removal of is_safe is the right thing to do. It's a gotcha though -- a note explaining that is_safe shouldn't be used if you're relying on a non-string return object in docs/howto/custom-template-tags.txt would be useful.

comment:4 Changed 7 years ago by carljm

  • Owner changed from nobody to carljm

Changed 7 years ago by carljm

same patch, with warning note in docs

comment:5 Changed 7 years ago by kmtracey

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

(In [9291]) Fixed #8966 -- Changed is_safe for length_is filter to False, since its return value is a boolean, not a string.

Thanks Thomas Steinacher and carljm.

comment:6 Changed 7 years ago by kmtracey

(In [9292]) [1.0.X] Fixed #8966 -- Changed is_safe for length_is filter to False, since its return value is a boolean, not a string.

Thanks Thomas Steinacher, carljm, and SmileyChris.

Backport of r9291 from trunk.

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