Code

Opened 3 years ago

Closed 2 years ago

#16395 closed Cleanup/optimization (fixed)

urlize works with malformed URLs

Reported by: BernhardEssl 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

Description

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://:/:/.foo.com")
Out[2]: u'<a href="http://:/:/.foo.com" rel="nofollow">http://:/:/.foo.com</a>'

I added a patch and a testcase for it.

Attachments (2)

urlize_malformed_urls.diff (2.3 KB) - added by BernhardEssl 3 years ago.
urlize_malformed_urls-2.diff (2.1 KB) - added by BernhardEssl 3 years ago.

Download all attachments as: .zip

Change History (7)

Changed 3 years ago by BernhardEssl

comment:1 Changed 3 years ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Design 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:////@foo.com will be misinterpreted as an email. (By the way, Trac happily turns that into an HTTP link).

comment:2 Changed 3 years ago by aaugustin

  • Type changed from Bug to Cleanup/optimization

Changed 3 years ago by BernhardEssl

comment:3 Changed 3 years ago by BernhardEssl

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

comment:4 Changed 3 years ago by jacob

  • Triage Stage changed from Design decision needed to Accepted

comment:5 Changed 2 years ago by aaugustin

  • Resolution set to fixed
  • Status changed from new to closed

In [17358]:

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.