Opened 9 years ago

Closed 8 years ago

#16395 closed Cleanup/optimization (fixed)

urlize works with malformed URLs

Reported by: Bernhard Essl Owned by: nobody
Component: Template system 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


I think the template filter urlize should not return broken URLs as clickable links.

In [1]: from django.template.defaultfilters import urlize

In [2]: urlize("http://:/:/")
Out[2]: u'<a href="http://:/:/" rel="nofollow">http://:/:/</a>'

I added a patch and a testcase for it.

Attachments (2)

urlize_malformed_urls.diff (2.3 KB) - added by Bernhard Essl 9 years ago.
urlize_malformed_urls-2.diff (2.1 KB) - added by Bernhard Essl 9 years ago.

Download all attachments as: .zip

Change History (7)

Changed 9 years ago by Bernhard Essl

Attachment: urlize_malformed_urls.diff added

comment:1 Changed 9 years ago by Aymeric Augustin

Patch needs improvement: set
Triage Stage: UnreviewedDesign decision needed

Your patch is basically checking that http:// is followed by a word character; could you explain why this is the right thing to do? IMO, validation must be implemented correctly or not at all.

Regarding the patch, I think it would be much clearer to structure the code like this:

if middle.startswith('http://') or middle.startswith('https://'):
    # do additional checks, and set url only if the checks pass
elif ... # unchanged

It's more readable than adding not middle.startswith('http') to every subsequent condition — since you only added it to the first one, I think http://// will be misinterpreted as an email. (By the way, Trac happily turns that into an HTTP link).

comment:2 Changed 9 years ago by Aymeric Augustin

Type: BugCleanup/optimization

Changed 9 years ago by Bernhard Essl

comment:3 Changed 9 years ago by Bernhard Essl

Thanks aaugustin for the input. I updated the patch. hihi @trac

comment:4 Changed 9 years ago by Jacob

Triage Stage: Design decision neededAccepted

comment:5 Changed 8 years ago by Aymeric Augustin

Resolution: fixed
Status: newclosed

In [17358]:

Fixed #16395 -- Prevented urlize from highlighting some malformed URLs. Thanks BernhardEssl for the report and initial patch.

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