Code

Opened 21 months ago

Closed 14 months ago

Last modified 14 months ago

#19070 closed Bug (fixed)

urlize template filter raises exception in some cases

Reported by: rivolaks@… Owned by: erikr
Component: Template system Version: master
Severity: Normal Keywords: dceu13
Cc: eromijn@… Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In some cases, urlize template filter fails because it's trying to parse ipv6 url and exception is raised:

>>> from django.utils.html import urlize
>>> urlize('www.ee]')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "django/utils/functional.py", line 193, in wrapper
    return func(*args, **kwargs)
  File "django/utils/html.py", line 213, in urlize
    url = smart_urlquote('http://%s' % middle)
  File "django/utils/html.py", line 152, in smart_urlquote
    scheme, netloc, path, query, fragment = urlsplit(url)
  File "/usr/lib/python2.7/urlparse.py", line 183, in urlsplit
    raise ValueError("Invalid IPv6 URL")
ValueError: Invalid IPv6 URL

'www.google.com]' also breaks, but 'google.com]' doesn't.

IMHO urlize filter should never raise exceptions. In my case it is used on plaintext user input which we have no control over, and I'm assuming that errors in Django's template rendering are always handled internally and silently. In this case I'd expect urlize() to catch the ValueError from urlsplit and handle it somehow, e.g. by skipping the url.

Happens in both 1.4 as well as master, 1.3 is not affected.

Attachments (3)

19070-1.diff (1.6 KB) - added by claudep 21 months ago.
Fixing the square bracket issue
19070-2.diff (1.9 KB) - added by tom@… 19 months ago.
test_urlize.diff (805 bytes) - added by andrea_crotti 14 months ago.
This adds a test that shows that the behaviour of urlize described here does not fail on 1.6.0

Download all attachments as: .zip

Change History (16)

Changed 21 months ago by claudep

Fixing the square bracket issue

comment:1 Changed 21 months ago by claudep

  • Component changed from Uncategorized to Template system
  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Bug

Attached a patch which fixes the specific square bracket issue (note that the issue is only accurate from Python 2.7).

The question remains whether we should swallow exceptions around smart_urlquote. Aymeric?

comment:2 Changed 21 months ago by Architekt

  • Triage Stage changed from Accepted to Ready for checkin

Patch reviewed and it's RFC.

Perhaps the question about swallowing exceptions shoud be discussed in django-developers or in separate ticket.

comment:3 Changed 21 months ago by andrewgodwin

I'm going to commit the patch since it's the precise fix for this bug and we're getting affected by this - if we want to discuss the general exceptions issue let's do it elsewhere.

comment:4 Changed 21 months ago by Andrew Godwin <andrew@…>

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

In 7f75460fd6befbef805fee3c91608efb0e9f444d:

Fixed #19070 -- urlize filter no longer raises exceptions on 2.7

Thanks to claudep for the patch.

comment:5 Changed 19 months ago by tom@…

  • Resolution fixed deleted
  • Status changed from closed to reopened

With the above patch applied / git HEAD:

>>> from django.utils.html import urlize
>>> urlize('www.ee]')
u'<a href="http://www.ee">www.ee</a>]'
>>> urlize('www[foo.com')
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/Users/tomi/Projects/feedify/venv/lib/python2.7/site-packages/django/utils/functional.py", line 176, in wrapper
    return func(*args, **kwargs)
  File "/Users/tomi/Projects/feedify/venv/lib/python2.7/site-packages/django/utils/html.py", line 168, in urlize
    url = smart_urlquote('http://%s' % middle)
  File "/Users/tomi/Projects/feedify/venv/lib/python2.7/site-packages/django/utils/html.py", line 107, in smart_urlquote
    scheme, netloc, path, query, fragment = urlparse.urlsplit(url)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/urlparse.py", line 182, in urlsplit
    raise ValueError("Invalid IPv6 URL")
ValueError: Invalid IPv6 URL
>>> 

pull request at https://github.com/django/django/pull/573 / patch attached.

Changed 19 months ago by tom@…

comment:6 Changed 19 months ago by Andrew Godwin <andrew@…>

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

In 501c7a221cfd022b6d0021fd9a54005c5ffc7a23:

Merge pull request #573 from tominsam/master

Fixed #19070: urlize template filter raises exception in some cases

comment:7 Changed 14 months ago by EmilStenstrom

  • Resolution fixed deleted
  • Status changed from closed to new

It still seems possible to break urlize in Django 1.5.1:

Testcase:

>>> import django
>>> django.VERSION
(1, 5, 1, 'final', 0)
>>> from django.template.defaultfilters import urlize
>>> urlize('[http://168.192.0.1](http://168.192.0.1)')
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/Users/EmilStenstrom/Envs/kundo/lib/python2.7/site-packages/django/template/defaultfilters.py", line 45, in _dec
    return func(*args, **kwargs)
  File "/Users/EmilStenstrom/Envs/kundo/lib/python2.7/site-packages/django/template/defaultfilters.py", line 336, in urlize
    return mark_safe(urlize_impl(value, nofollow=True, autoescape=autoescape))
  File "/Users/EmilStenstrom/Envs/kundo/lib/python2.7/site-packages/django/utils/functional.py", line 194, in wrapper
    return func(*args, **kwargs)
  File "/Users/EmilStenstrom/Envs/kundo/lib/python2.7/site-packages/django/utils/html.py", line 212, in urlize
    url = smart_urlquote(middle)
  File "/Users/EmilStenstrom/Envs/kundo/lib/python2.7/site-packages/django/utils/html.py", line 153, in smart_urlquote
    scheme, netloc, path, query, fragment = urlsplit(url)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/urlparse.py", line 182, in urlsplit
    raise ValueError("Invalid IPv6 URL")
ValueError: Invalid IPv6 URL

comment:8 Changed 14 months ago by timo

  • Has patch unset
  • Triage Stage changed from Ready for checkin to Unreviewed
  • Version changed from 1.4 to 1.5

@EmilStenstrom, I think opening a new ticket would have been the best course of action here since the commits for this ticket were already released, but moving this back to "Unreviewed" for the new issue.

comment:9 Changed 14 months ago by andrea_crotti

On django (1, 6, 0, 'alpha', 0)
I get a different output
u'[<a href="http://168.192.0.1](http://168.192.0.1)" rel="nofollow">http://168.192.0.1](http://168.192.0.1)</a>'
I'm not entirely sure that this result is correct either though..

Last edited 14 months ago by andrea_crotti (previous) (diff)

Changed 14 months ago by andrea_crotti

This adds a test that shows that the behaviour of urlize described here does not fail on 1.6.0

comment:10 Changed 14 months ago by erikr

  • Has patch set
  • Keywords dceu13 added
  • Owner changed from nobody to erikr
  • Patch needs improvement set
  • Status changed from new to assigned
  • Version changed from 1.5 to master

As this problem was reported, I think it would be good to add the test to the testsuite. It's a realistic use, and means we can be sure this does not break at any time. I'll clean the patch a little bit and make a PR.

comment:11 Changed 14 months ago by erikr

  • Cc eromijn@… added
  • Patch needs improvement unset
Last edited 14 months ago by erikr (previous) (diff)

comment:12 Changed 14 months ago by Erik Romijn <eromijn@…>

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

In 1927bf7c91761040a7e46197fc2d19b66f2ae332:

Fix #19070 -- Additional test for urlize and brackets

comment:13 Changed 14 months ago by Aymeric Augustin <aymeric.augustin@…>

In d34b1c29e294b19e51a47918125314a1540c01d4:

Merge pull request #1161 from erikr/urlize-additional-test

Fixed #19070 -- Additional test for urlize and brackets

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.