Markdown safe mode fails to return result if there's link in content

[simonas@…]

I have been using

{{ comment.comment|markdown:"safe,nl2br"|nofollow }}

to render my comments and after moving to 1.4 I noticed that some comments fail to render.

After some testing I noticed that:

1. Only comments with links fail to render.

2. Safe mode is to blame (specifically this line). Disabling it (or setting enable_attributes to True) fixes the problem.

[Preston Holmes]

The markdown library has an interesting use of the term "safe mode" as it also enables the markdown syntax to inject JS attributes. see #17837

as such, the safe mode feature and markdown's "enable_attributes" were coupled in Django 1.4 : ​https://docs.djangoproject.com/en/dev/releases/1.4/#attributes-disabled-in-markdown-when-safe-mode-set

So if you want to enable markdown features that are not safe, including links and attributes, the safe parameter must now be dropped.

A customized tag could always be used if you want to override Django's behavior, but these were tightly coupled as a security measure.

[Carl Meyer]

I would have closed this needsinfo, not invalid. From the provided information, it is not clear whether the OP is doing something unsafe, or there is a bug. Markdown links are not unsafe, and need to work for markdown to be useful. The ticket asserts that links are not working in safe mode; if that is true, it's a serious problem.

Also, we need a clearer definition of "fail to render". Turning off enable_attributes should not cause markdown to fail entirely, it should just cause the attribute extension constructs to be passed through as-is to the output instead of turning into HTML attributes.

simonas, could you provide the full markdown text of a comment that "fails to render," and clarify exactly what "fails to render" means?

[simonas@…]

Result when safe mode is disabled (expected)

[simonas@…]

Result when safe mode is enabled (unexpected)

[simonas@…]

Sorry for lack of information.

First of all two comments:

[Google](http://google.com) is search engine.

Google is search engine

Without safe mode enabled result looks like expected:

However when safe mode is enabled comment with link fails to render completely:

Some more examples of failing links:

[Hash link](#comment-xxx)

[Relative link](/great-article/)

By the way markdown.version = '2.1.0'

[Preston Holmes]

Sorry for the premature closing of the ticket.

Unfortunately this looks like a bug in python-markdown

>>> markdown.markdown('[Google](http://google.com) is search engine.',safe=True)
u'<p><a href="http://google.com">Google</a> is search engine.</p>'
>>> markdown.markdown('[Google](http://google.com) is search engine.',safe=True, enable_attributes=False)
u'<p></p>'

This seems to be a flaw in the parsing that effects more than just links:

​https://github.com/waylan/Python-Markdown/issues/87

Not clear how to best triage this from the Django point of view. Django appears to be doing the right thing.

Certainly the markdown tag could use an upgrade that would potentially expose the newer options to the safe param in markdown (other than just safe=True). From the point of view of Django - the safe_mode and enable_attributes had to be coupled, because the option to the tag is only "safe", so it needs to be the safest it can be.

It also looks like the markdown issue referred to in #17837 has recently been reopened by the maintainer:

​https://github.com/waylan/Python-Markdown/issues/82

[Carl Meyer]

Ugh. So pending fixes in the upstream Markdown library, we have the choice of insecure or broken.

Or we could switch to ​https://github.com/trentm/python-markdown2 python-markdown2 - that's what I've done for my own projects, and have had no issues so far. This would have to be done over a deprecation path, or we could add support for markdown2 but maintain support for python-markdown; one would have to be a fallback for the other.

Marking release blocker as this is a serious regression that makes the "markdown" filter (practically speaking) unusable.

[Preston Holmes]

Because Django only provides a simple on/off 'safe' option to the markup tag, the output needs to be the most secure - which includes setting enable_attributes false. Hopefully python-markdown can get this bug fixed, if you are impacted by this bug you might:

• consider contributing a fix to python-markdown
• create your own tag using an alternate markdown library (ie markdown2 ​https://github.com/trentm/python-markdown2)
• turning off safe mode, and running the output through a sanitizing filter like 'bleach' ​http://pypi.python.org/pypi/bleach