Opened 8 years ago

Closed 4 years ago

#6474 closed Cleanup/optimization (invalid)

conditional_escape: Try to convert to a string first, then check if SafeData

Reported by: guettli Owned by: nobody
Component: Uncategorized Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no



if you try to conditional_escape() an object, which unicode
method returns a SafeString, the SafeStrinng gets escaped (one time to much).

I attached a patch with unittest.

All other tests pass.

Attachments (1)

utils_conditional_escape.diff (1.4 KB) - added by guettli 8 years ago.

Download all attachments as: .zip

Change History (7)

Changed 8 years ago by guettli

comment:1 Changed 8 years ago by mtredinnick

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

It's fairly dangerous (and I'm not sure it's entirely legal) to have the __unicode__ method return something that isn't simply a Unicode object.

More research required here to determine if this is actually a safe Python practice. If not, the whole issue is moot.

comment:2 Changed 8 years ago by guettli

This is derived from newforms.utils.ErrorList: There __unicode__()
returns mark_safe(...), too. There, I discoverd the bug. I just
wanted to keep the unittest self-contained.

# newforms.utils.ErrorList:
class ErrorList(list, StrAndUnicode):
    A collection of errors that knows how to display itself in various formats.
    def __unicode__(self):
        return self.as_ul()

    def as_ul(self):
        if not self: return u''
        return mark_safe(u'<ul class="errorlist">%s</ul>'
                % ''.join([u'<li>%s</li>' % force_unicode(e) for e in self]))

comment:3 Changed 8 years ago by guettli

  • Cc hv@… added

comment:4 Changed 5 years ago by patchhammer

  • Easy pickings unset
  • Patch needs improvement set
  • Severity set to Normal
  • Type set to Uncategorized

utils_conditional_escape.diff fails to apply cleanly on to trunk

comment:5 Changed 5 years ago by julien

  • Type changed from Uncategorized to Cleanup/optimization

comment:6 Changed 4 years ago by guettli

  • Cc hv@… removed
  • Resolution set to invalid
  • Status changed from new to closed
  • UI/UX unset

The unittest of the patch still fails, but I (original reporter) close it.

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