Changes between Initial Version and Version 2 of Ticket #14976


Ignore:
Timestamp:
Dec 27, 2010, 9:02:01 PM (13 years ago)
Author:
Luke Plant
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.

Legend:

Unmodified
Added
Removed
Modified

  • Ticket #14976

    • Property Triage Stage Unreviewed → Accepted
    • Property Summary Add is_safe flag to contrib.messages → Add is_html flag to contrib.messages

  • Ticket #14976 – Description

    initial v2  
    1 I would like to have add a message.is_safe flag to the Message model of the contrib.messages app.[[BR]]
    2 [[BR]]
    3 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]]
    4 [[BR]]
     1I would like to have add a message.is_html flag to the Message model of the contrib.messages app.
    52
    6 Adding the is_safe flag would require a minor set of backward compatible changes:[[BR]]
     3The 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.
     4
     5Adding the is_html flag would require a minor set of backward compatible changes:
    76
    87{{{
    98def success(request, message, extra_tags='', fail_silently=False):
    109to
    11 def success(request, message, extra_tags='', fail_silently=False, is_safe=False):
     10def success(request, message, extra_tags='', fail_silently=False, is_html=False):
    1211
    1312def add_message(request, level, message, extra_tags='', fail_silently=False):
    1413to
    15 def add_message(request, level, message, extra_tags='', fail_silently=False, is_safe=False):
     14def add_message(request, level, message, extra_tags='', fail_silently=False, is_html=False):
    1615
    1716def __init__(self, level, message, extra_tags=None):
    1817to
    19 def __init__(self, level, message, extra_tags=None, is_safe=False):
     18def __init__(self, level, message, extra_tags=None, is_html=False):
    2019
    2120#add to __init__
    22 self.is_safe = is_safe
     21self.is_html = is_html
    2322}}}
    2423
    25 Then in the template: {% if message.is_safe %} {{ message|safe }}{% else %}{{ message }}{% endif %}.[[BR]]
    26 [[BR]]
     24Then in the template:
     25{{{
     26{% if message.is_html %}{{ message|safe }}{% else %}{{ message }}{% endif %}.
     27}}}
    2728 
    2829
    29 Alternative ways to do this:[[BR]]
    30 1. Run all messages through the safe filter[[BR]]
    31 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]]
     30Alternative ways to do this:
    3231
    33 2.Use a tag[[BR]]
    34 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]]
    35 [[BR]]
     32
     33 1. Run all messages through the safe filter[[BR]][[BR]]
     34 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.
     35
     36 2. Use a tag[[BR]][[BR]]
     37 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.
     38
    3639If this isn't violating a core django design precept, I'll get started on a patch in the next few days.
Back to Top