#14976: Add is_html flag to contrib.messages
-----------------------------------+----------------------------------------
          Reporter:  tedtieken     |         Owner:  nobody        
            Status:  new           |     Milestone:                
         Component:  Contrib apps  |       Version:  1.2           
        Resolution:                |      Keywords:  safe, messages
             Stage:  Accepted      |     Has_patch:  0             
        Needs_docs:  0             |   Needs_tests:  0             
Needs_better_patch:  0             |  
-----------------------------------+----------------------------------------
Changes (by lukeplant):

  * summary:  Add is_safe flag to contrib.messages => Add is_html flag to
              contrib.messages
  * stage:  Unreviewed => Accepted

Old description:

> I would like to have add a message.is_safe flag to the Message model of
> the contrib.messages app.[[BR]]
> [[BR]]
> The flag would be set to False by default and could be explicitly
> overridden for trusted messages.  There are times when it would be
> helpful to the end user to include an html link in a message ("Welcome,
> click here to create a profile", "You've sent 25 points to user_b, click
> here to see your balance," etc.), and with the current message system
> there is not a good way to do this.[[BR]]
> [[BR]]
>
> Adding the is_safe flag would require a minor set of backward compatible
> changes:[[BR]]
>
> {{{
> def success(request, message, extra_tags='', fail_silently=False):
> to
> def success(request, message, extra_tags='', fail_silently=False,
> is_safe=False):
>
> def add_message(request, level, message, extra_tags='',
> fail_silently=False):
> to
> def add_message(request, level, message, extra_tags='',
> fail_silently=False, is_safe=False):
>
> def __init__(self, level, message, extra_tags=None):
> to
> def __init__(self, level, message, extra_tags=None, is_safe=False):
>
> #add to __init__
> self.is_safe = is_safe
> }}}
>
> Then in the template: {% if message.is_safe %} {{ message|safe }}{% else
> %}{{ message }}{% endif %}.[[BR]]
> [[BR]]
>

> Alternative ways to do this:[[BR]]
> 1. Run all messages through the safe filter[[BR]]
> This would require a code-wide policy of "make sure you escape anything
> in a message that might have user input" such as if my message is "your
> post %s is now published" % blog.post or "%s has sent you the message %s"
> %(user, message.content).   I would then have to worry about every
> variable I use in a message string, if it could contain script, and if it
> is already escaped (or escape everything again).  I would also have to
> worry if everyone else working on the codebase is doing this correctly.
> [[BR]]
>
> 2.Use a tag[[BR]]
> I could have a policy of adding "safe" to the tags I want to run through
> the safe filter, but this is also fraught with downsides.  Since all tags
> get output into html, the safe flag would end up output to the end user.
> The template logic is less clear and error prone {{ "test" in
> message.extra_tags }} would work, but would return a false positive if
> you tried to use "contest" as a tag.  Granted "contest" as a message tag
> is a rare case; it is just another layer of messiness of security code
> mashed in with markup.[[BR]]
> [[BR]]
> If this isn't violating a core django design precept, I'll get started on
> a patch in the next few days.

New description:

 I would like to have add a message.is_html flag to the Message model of
 the contrib.messages app.

 The flag would be set to False by default and could be explicitly
 overridden for messages that are HTML.  There are times when it would be
 helpful to the end user to include an html link in a message ("Welcome,
 click here to create a profile", "You've sent 25 points to user_b, click
 here to see your balance," etc.), and with the current message system
 there is not a good way to do this.

 Adding the is_html flag would require a minor set of backward compatible
 changes:

 {{{
 def success(request, message, extra_tags='', fail_silently=False):
 to
 def success(request, message, extra_tags='', fail_silently=False,
 is_html=False):

 def add_message(request, level, message, extra_tags='',
 fail_silently=False):
 to
 def add_message(request, level, message, extra_tags='',
 fail_silently=False, is_html=False):

 def __init__(self, level, message, extra_tags=None):
 to
 def __init__(self, level, message, extra_tags=None, is_html=False):

 #add to __init__
 self.is_html = is_html
 }}}

 Then in the template:
 {{{
 {% if message.is_html %}{{ message|safe }}{% else %}{{ message }}{% endif
 %}.
 }}}


 Alternative ways to do this:


  1. Run all messages through the safe filter[[BR]][[BR]]
  This would require a code-wide policy of "make sure you escape anything
 in a message that might have user input" such as if my message is "your
 post %s is now published" % blog.post or "%s has sent you the message %s"
 %(user, message.content).   I would then have to worry about every
 variable I use in a message string, if it could contain script, and if it
 is already escaped (or escape everything again).  I would also have to
 worry if everyone else working on the codebase is doing this correctly.

  2. Use a tag[[BR]][[BR]]
  I could have a policy of adding "html" to the tags I want to run through
 the safe filter, but this is also fraught with downsides.  Since all tags
 get output into html, the safe flag would end up output to the end user.
 The template logic is less clear and error prone.

 If this isn't violating a core django design precept, I'll get started on
 a patch in the next few days.

Comment:

 It isn't at all obvious what 'is_safe' refers to. I thought you were
 talking about trusted vs untrusted messages. 'is_html' would be '''much'''
 clearer - so I've changed that. I also fixed up some other things in the
 description where you seemed to switch from "safe" to "test" - it was a
 bit confusing.

 Other than that, I can see the case for this request. We need to think
 about XSS, but AFAICS there is no issue. The Cookie backend for Messages
 is potentially vulnerable, but 1) Cookies are a very poor vector for XSS,
 and 2) we are signing and checking all Messages using HMAC.

 With regards to compatibility, we would also need to ensure that messages
 pickled before the change can be unpickled after it.

 So I've accepted this ticket, assuming we can find a fully backwards
 compatible solution.

-- 
Ticket URL: <http://code.djangoproject.com/ticket/14976#comment:2>
Django <http://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Reply via email to