Django

Code

Ticket #393 (closed: fixed)

Opened 3 years ago

Last modified 2 years ago

[patch] Filters don't take the str() value of a var

Reported by: Boffbowsh Assigned to: adrian
Milestone: Component: Core framework
Version: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

If I have a category model and do {{category|upper}} it fails, even though i have a repr(). Added str() to a number of filters to fix this behaviour. Maybe would be better to add to the template definition what sort of var it should accept? If it only accepts strings, str() everything first within django.core.template
Also added basic tests to string filters that don't accept args

Index: /usr/local/django_src/django/core/defaultfilters.py
===================================================================
--- /usr/local/django_src/django/core/defaultfilters.py	(revision 544)
+++ /usr/local/django_src/django/core/defaultfilters.py	(working copy)
@@ -9,7 +9,7 @@
 
 def addslashes(value, _):
     "Adds slashes - useful for passing strings to JavaScript, for example."
-    return value.replace('"', '\\"').replace("'", "\\'")
+    return str(value).replace('"', '\\"').replace("'", "\\'")
 
 def capfirst(value, _):
     "Capitalizes the first character of the value"
@@ -19,7 +19,7 @@
 def fix_ampersands(value, _):
     "Replaces ampersands with ``&`` entities"
     from django.utils.html import fix_ampersands
-    return fix_ampersands(value)
+    return fix_ampersands(str(value))
 
 def floatformat(text, _):
     """
@@ -35,7 +35,7 @@
 def linenumbers(value, _):
     "Displays text with line numbers"
     from django.utils.html import escape
-    lines = value.split('\n')
+    lines = str(value).split('\n')
     # Find the maximum width of the line count, for use with zero padding string format command
     width = str(len(str(len(lines))))
     for i, line in enumerate(lines):
@@ -44,7 +44,7 @@
 
 def lower(value, _):
     "Converts a string into all lowercase"
-    return value.lower()
+    return str(value).lower()
 
 def make_list(value, _):
     """
@@ -55,7 +55,7 @@
 
 def slugify(value, _):
     "Converts to lowercase, removes non-alpha chars and converts spaces to hyphens"
-    value = re.sub('[^\w\s-]', '', value).strip().lower()
+    value = re.sub('[^\w\s-]', '', str(value)).strip().lower()
     return re.sub('\s+', '-', value)
 
 def stringformat(value, arg):
@@ -74,7 +74,7 @@
 
 def title(value, _):
     "Converts a string into titlecase"
-    return re.sub("([a-z])'([A-Z])", lambda m: m.group(0).lower(), value.title())
+    return re.sub("([a-z])'([A-Z])", lambda m: m.group(0).lower(), str(value).title())
 
 def truncatewords(value, arg):
     """
@@ -93,17 +93,17 @@
 
 def upper(value, _):
     "Converts a string into all uppercase"
-    return value.upper()
+    return str(value).upper()
 
 def urlencode(value, _):
     "Escapes a value for use in a URL"
     import urllib
-    return urllib.quote(value)
+    return urllib.quote(str(value))
 
 def urlize(value, _):
     "Converts URLs in plain text into clickable links"
     from django.utils.html import urlize
-    return urlize(value, nofollow=True)
+    return urlize(str(value), nofollow=True)
 
 def urlizetrunc(value, limit):
     """
@@ -116,7 +116,7 @@
 
 def wordcount(value, _):
     "Returns the number of words"
-    return len(value.split())
+    return len(str(value).split())
 
 def wordwrap(value, arg):
     """
@@ -163,11 +163,11 @@
 def linebreaks(value, _):
     "Converts newlines into <p> and <br />s"
     from django.utils.html import linebreaks
-    return linebreaks(value)
+    return linebreaks(str(value))
 
 def linebreaksbr(value, _):
     "Converts newlines into <br />s"
-    return value.replace('\n', '<br />')
+    return str(value).replace('\n', '<br />')
 
 def removetags(value, tags):
     "Removes a space separated list of [X]HTML tags from the output"
@@ -410,7 +410,7 @@
 def phone2numeric(value, _):
     "Takes a phone number and converts it in to its numerical equivalent"
     from django.utils.text import phone2numeric
-    return phone2numeric(value)
+    return phone2numeric(str(value))
 
 def pprint(value, _):
     "A wrapper around pprint.pprint -- for debugging, really"
Index: /usr/local/django_src/tests/othertests/templates.py
===================================================================
--- /usr/local/django_src/tests/othertests/templates.py	(revision 544)
+++ /usr/local/django_src/tests/othertests/templates.py	(working copy)
@@ -1,4 +1,4 @@
-from django.core import template, template_loader
+from django.core import template, template_loader, defaultfilters
 
 # Helper objects for template tests
 class SomeClass:
@@ -10,6 +10,9 @@
         
     def method2(self, o):
         return o
+	
+    def __repr__(self):
+	return 'DjAnGo'
 
 class OtherClass:
     def method(self):
@@ -166,6 +169,25 @@
     'exception04': ("{% extends 'inheritance17' %}{% block first %}{% echo 400 %}5678{% endblock %}", {}, template.TemplateSyntaxError),
 }
 
+STRING_FILTERS = (
+	'linebreaksbr',
+	'striptags',
+	'escape',
+	'linebreaks',
+	'urlize',
+	'fix_ampersands',
+	'title',
+	'capfirst',
+	'wordcount',
+	'linenumbers',
+	'urlencode',
+	'lower',
+	'upper',
+	'phone2numeric',
+	'addslashes',
+	'slugify'
+	)
+
 # This replaces the standard template_loader.
 def test_template_loader(template_name, template_dirs=None):
     try:
@@ -176,6 +198,8 @@
 def run_tests(verbosity=0, standalone=False):
     template_loader.load_template_source, old_template_loader = test_template_loader, template_loader.load_template_source
     failed_tests = []
+    for stringfilter in STRING_FILTERS:
+	    TEMPLATE_TESTS['filter_' + stringfilter] = ( '{{ var|%s }}' % stringfilter, {"var": SomeClass()}, str(defaultfilters.template.registered_filters[stringfilter][0](str( 'DjAnGo' ), ())) )
     tests = TEMPLATE_TESTS.items()
     tests.sort()
     for name, vals in tests:

Attachments

filter-object.diff (7.4 kB) - added by Boffbowsh on 09/01/05 10:05:27.
str_filters.patch (10.1 kB) - added by SmileyChris on 01/23/07 16:50:37.
Shiny new patch, with tests and docs

Change History

08/22/05 11:05:24 changed by anonymous

What about Unicode strings?

08/22/05 11:12:46 changed by Boffbowsh

Would repr() be better then? On unicode strings, this performs the equivalent of: r = u"%s" % repr(u.encode('unicode-escape')) which is the same format as u"string" literals

08/22/05 13:35:34 changed by eugene@lazutkin.com

Did you try something like this?

{{category.__repr__|upper}}

08/22/05 13:52:45 changed by jacob

That won't work -- templates don't allow access to variables starting with underscores. The initial fix is correct: filters that take strings should explicitly str()-ify the variables.

08/22/05 14:32:08 changed by Boffbowsh

Need someone with more python unicode experience (and test data) to comment on this

09/01/05 10:05:02 changed by Boffbowsh

Ok, real patch now that takes into account unicode too. Added an extra optional arg to register_filter called string_only, signifies that the filter only works on strings and that resolve_variable_with_filter should convert the input first. Below is a test script:

from django.core import template

class TestClass:
	def __repr__(self):
		return('dJaNgO')

r = TestClass()

s = '''String: {{s|capfirst}}
Unicode: {{u|capfirst}}
Class: {{c|capfirst}}'''
t = template.Template(s)
c = template.Context({'u': u'la Pe\xf1a', 's': 'pyTHON', 'c': r})
print t.render(c)

and the output:

$ python unicodefiltertest.py
String: PyTHON
Unicode: La Peña
Class: DJaNgO

09/01/05 10:05:27 changed by Boffbowsh

  • attachment filter-object.diff added.

09/23/05 05:25:55 changed by Boffbowsh

*bump*

09/23/05 07:37:04 changed by jacob

Please, don't bump tickets; that's not polite.

09/23/05 08:22:43 changed by Boffbowsh

Sorry, but the patch has been sitting there for ages now, any idea of when you plan to include it?

11/26/05 12:42:00 changed by Maniac <Maniac@SoftwareManiacs.Org>

I've filed a ticket 924 with a patch that converts all str's on input to unicode. This will probably breal your patch, sorry!

However I beleive this approach with total unicodisation is cleaner since it isolates type checking in only one place. May be you could update your patch to assume that all strings you get are unicode's. Thoughts?

07/03/06 02:11:52 changed by anonymous

  • priority changed from normal to low.
  • type changed from defect to enhancement.
  • component changed from Template system to Core framework.
  • severity changed from normal to critical.

10/08/06 07:05:40 changed by Boffbowsh

Removing self from CC

10/08/06 07:05:59 changed by Boffbowsh

  • cc deleted.

Removing self from CC

01/23/07 09:59:54 changed by Marc Fargas <telenieko@telenieko.com>

  • needs_better_patch set to 1.

This bug has been arond without activity for 5 months now, and the original report is one year old. Can someone confirm it's still valid? Marking "Patch needs improvement" as the patch references no longer existing files.

01/23/07 16:37:41 changed by Ivan Sagalaev <Maniac@SoftwareManiacs.Org>

May I suggest looking at #924 that kinda superseeds this one and is safer. With non-ascii data simple str()ing is a recipe for breakage since it will use us-ascii codec by default.

01/23/07 16:40:32 changed by Ivan Sagalaev <Maniac@SoftwareManiacs.Org>

And by "looking" I really mean closing it also in favor of unicodification **if** that will happen before 1.0

01/23/07 16:50:37 changed by SmileyChris

  • attachment str_filters.patch added.

Shiny new patch, with tests and docs

01/23/07 16:59:43 changed by SmileyChris

  • needs_better_patch deleted.
  • stage changed from Unreviewed to Ready for checkin.

My patch sticks with str for now. I don't want to get into the whole unicode debate, just fix the bug. My patch will make it much easier to upgrade to that future step however.

01/23/07 17:09:34 changed by Marc Fargas <telenieko@telenieko.com>

I would rather set "Design decission needed" and let some core developer decide if the smart_string() function on the patch should support unicode before checkin (in favour of #2489) or if it's ok for now to be checked in.

But also, the bug should be fixed, so "Ready for chekin" is ok! But can I suggest opening a new ticket about adding unicode support to smart_string() when this one gets closed?

01/23/07 18:57:20 changed by SmileyChris

Will do Marc. Like you say, the bug should be fixed and if it's been sitting around this long, I'd prefer not to complicate the issue for now. I just wanted to also show some forward thinking :)

02/23/07 14:52:04 changed by jacob

  • status changed from new to closed.
  • resolution set to fixed.

Fixed in [4558].


Add/Change #393 ([patch] Filters don't take the str() value of a var)




Change Properties
Action