urlize incorrectly includes quotation marks in links

[EmilStenstrom]

>>> urlize("some text here \"hi@example.com\" and some text afterwards")
u'some text here <a href="mailto:"hi@example.com"">"hi@example.com"</a> and some text afterwards'

Note the extra quotes inside the links.

Luckily, this is solved quite easily, just do this:

>>> from django.utils import html
>>> html.TRAILING_PUNCTUATION += ['"', '\'']
>>> html.WRAPPING_PUNCTUATION += [('"', '"'), ('\'', '\'')]
>>> urlize("some text here \"hi@example.com\" and some text afterwards")
u'some text here "<a href="mailto:hi@example.com">hi@example.com</a>" and some text afterwards'

I suggest quotation marks are included in the *_PUNCTUATION variables in django.utils.html.

[EmilStenstrom]

I don't know if this matters, but django-rest-framework has both of the changes above added in their version of urlize: ​https://github.com/glic3rinu/django-rest-framework/commit/2c0363ddaec22ac54385f7e0c2e1401ed3ff0879

[EmilStenstrom]

Similar ticket, that has to do with JSON somehow... https://code.djangoproject.com/ticket/17576

[wim@…]

Hi Emil,

Thanks for reporting! Clear that we get erroneous output here. You are welcome to write a patch, if you want to.

[Chris Piwoński <piwonski.kris@…>]

ticket patch

[EmilStenstrom]

@Chris Piwoński: Thanks for a patch!

Can i suggest you add a few more tests? The first only works with the TRAILING_PUNCTUATION fix. The second seems to work with the WRAPPING_PUNCTUATION fix.

>>> html.urlize("some text here hi@example.com\" and some text afterwards")
u'some text here <a href="mailto:hi@example.com"">hi@example.com"</a> and some text afterwards'
>>> html.urlize("some text here \"hi@example.com and some text afterwards")
u'some text here <a href="mailto:"hi@example.com">"hi@example.com</a> and some text afterwards'

And maybe some tests for the single quote case too?

[Chris Piwoński <piwonski.kris@…>]

Made tests for all six cases.

Changes are on github: ​https://github.com/kriestof/django/tree/ticket_20364

[Claude Paroz]

Sorry, but the RFC flag should never be set by the patch author.

[EmilStenstrom]

This looks great Chris. The new tests pass, and the code looks good.

[Aymeric Augustin]

If you read #17576, which is linked from above, it provides you with a failing test case: adding a comma after the quote.

For instance I'm almost sure this doesn't work with the current patch.

urlize('Email us at "hi@example.com", or phone us at +xx.yy').

The problem here is that urlize cannot deal with multiple punctuation, and quotes are often followed by other punctuation. I'd rather not merge a half-broken feature, and in my humble opinion this ticket should be closed wontfix like #17576.

That said, urlize seems to attract tons of contributions, so maybe it's a very important part of Django and I should stop wontfixing such tickets... I'll let another committer make the decision!

[Krzysztof Jurewicz]

In case this ticket isn’t marked as wontfix, shouldn’t we also support localized quotation marks like „” and «»? There is ​a list of them available on Wikipedia.

[Anssi Kääriäinen]

I am not that good with HTML stuff, so I am not going to make wontfix call... But in any case this isn't Ready for checkin...

[GilesGreenway]

aaugustin's example is in fact passed with flying colours after applying the patch:

>>> urlize('Email us at "hi@example.com", or phone us at +xx.yy')
u'Email us at "<a href="mailto:hi@example.com">hi@example.com</a>", or phone us at +xx.yy'
>>> 
>>> urlize('Visit us at "http://www.example.com", or phone us at +xx.yy')
u'Visit us at "<a href="http://www.example.com">http://www.example.com</a>", or phone us at +xx.yy'

I'll see if I can get a test written. (I'm a newbie contributor at the PyConUK Django sprint.)

[GilesGreenway]

Test pushed, pull request made:

​https://github.com/django/django/pull/1662

[Daniele Procida]

I've tested the patch at ​https://github.com/django/django/pull/1662 locally and it looks good to me. Commit/merge message needs to thank EmilStenstrom for the report and Chris Piwoński for the first patch, but other than that I think it's ready for checkin.

[GilesGreenway]

Done. 'Hope this is suitably congratulatory:

​https://github.com/django/django/pull/1662/commits

[Florian Apolloner <florian@…>]

In 6c06adad1dc45631c1c220d7f8fb531a9cf3ed55:

Fixed #20364 -- Changed urlize regexes to include quotation marks as punctation.

Thanks to EmilStenstrom for raising this, and to Chris Piwoński for all of the fixes and most of the tests.

[EmilStenstrom]

I think far to few Ticket threads end with the reporter wrapping up the thread, so here goes:

YOU ARE AWESOME :D