Skip to content

Commit 7a0939a

Browse files
committed
issue2551098: markdown links missing rel="noreferer nofollow"
Links generated by all markdown backends are missing the noopener and nofollow relation that roundup's normal text -> html core adds to prevent security issues and link spam. Now rel="nofollow" is added to links generated by markdown2 backends and rel="nofollow noopener" for mistune and markdown backends. Markdown2 isn't as programable as the other two backends so I used the built-in nofollow support. This means that a user that generates a link that opens in a new window can manpulate the parent window.
1 parent 5156341 commit 7a0939a

File tree

2 files changed

+114
-8
lines changed

2 files changed

+114
-8
lines changed

roundup/cgi/templating.py

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ class Markdown(markdown2.Markdown):
7070
_safe_protocols = re.compile('(?!' + ':|'.join([re.escape(s) for s in _disable_url_schemes]) + ':)', re.IGNORECASE)
7171

7272
def _extras(config):
73-
extras = { 'fenced-code-blocks' : {} }
73+
extras = { 'fenced-code-blocks' : {}, 'nofollow': None }
7474
if config['MARKDOWN_BREAK_ON_NEWLINE']:
7575
extras['break-on-newline'] = True
7676
return extras
@@ -96,7 +96,21 @@ def run(self, root):
9696
if url.startswith(s + ':'):
9797
el.attrib['href'] = '#'
9898

99+
class LinkRendererWithRel(Treeprocessor):
100+
''' Rendering class that sets the rel="nofollow noreferer"
101+
for links. '''
102+
rel_value = "nofollow noopener"
103+
104+
def run(self, root):
105+
for el in root.iter('a'):
106+
if 'href' in el.attrib:
107+
url = el.get('href').lstrip(' \r\n\t\x1a\0').lower()
108+
if not url.startswith('http'): # only add rel for absolute http url's
109+
continue
110+
el.set('rel', self.rel_value)
111+
99112
# make sure any HTML tags get escaped and some links restricted
113+
# and rel="nofollow noopener" are added to links
100114
class SafeHtml(MarkdownExtension):
101115
def extendMarkdown(self, md, md_globals=None):
102116
if hasattr(md.preprocessors, 'deregister'):
@@ -112,7 +126,11 @@ def extendMarkdown(self, md, md_globals=None):
112126
md.treeprocessors.register(RestrictLinksProcessor(), 'restrict_links', 0)
113127
else:
114128
md.treeprocessors['restrict_links'] = RestrictLinksProcessor()
115-
129+
if hasattr(md.preprocessors, 'register'):
130+
md.treeprocessors.register(LinkRendererWithRel(), 'add_link_rel', 0)
131+
else:
132+
md.treeprocessors['add_link_rel'] = LinkRendererWithRel()
133+
116134
def _extensions(config):
117135
extensions = [SafeHtml(), 'fenced_code']
118136
if config['MARKDOWN_BREAK_ON_NEWLINE']:
@@ -128,10 +146,44 @@ def _extensions(config):
128146
def _import_mistune():
129147
try:
130148
import mistune
149+
from mistune import Renderer, escape_link, escape
150+
131151
mistune._scheme_blacklist = [ s + ':' for s in _disable_url_schemes ]
132152

153+
class LinkRendererWithRel(Renderer):
154+
''' Rendering class that sets the rel="nofollow noreferer"
155+
for links. '''
156+
157+
rel_value = "nofollow noopener"
158+
159+
def autolink(self, link, is_email=False):
160+
''' handle <url or email> style explicit links '''
161+
text = link = escape_link(link)
162+
if is_email:
163+
link = 'mailto:%s' % link
164+
return '<a href="%(href)s">%(text)s</a>' % { 'href': link, 'text': text }
165+
return '<a href="%(href)s" rel="%(rel)s">%(href)s</a>' % {
166+
'rel': self.rel_value, 'href': escape_link(link)}
167+
168+
def link(self, link, title, content):
169+
''' handle [text](url "title") style links and Reference
170+
links '''
171+
172+
values = {
173+
'content': escape(content),
174+
'href': escape_link(link),
175+
'rel': self.rel_value,
176+
'title': escape(title) if title else '',
177+
}
178+
179+
if title:
180+
return '<a href="%(href)s" rel="%(rel)s" ' \
181+
'title="%(title)s">%(content)s</a>' % values
182+
183+
return '<a href="%(href)s" rel="%(rel)s">%(content)s</a>' % values
184+
133185
def _options(config):
134-
options = {}
186+
options = {'renderer': LinkRendererWithRel(escape = True)}
135187
if config['MARKDOWN_BREAK_ON_NEWLINE']:
136188
options['hard_wrap'] = True
137189
return options

test/test_templating.py

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,45 @@ def test_input_xhtml(self):
420420

421421
# common markdown test cases
422422
class MarkdownTests:
423+
def mangleMarkdown2(self, s):
424+
''' markdown2's rel=nofollow support on 'a' tags isn't programmable.
425+
So we are using it's builtin nofollow support. Mangle the string
426+
so that it matches the test case.
427+
428+
turn: <a rel="nofollow" href="foo"> into
429+
<a href="foo" rel="nofollow noopener">
430+
431+
Also if it is a mailto url, we don't expect rel="nofollow",
432+
so delete it.
433+
434+
turn: <a rel="nofollow" href="mailto:foo"> into
435+
<a href="mailto:foo">
436+
437+
Also when a title is present it is put in a different place
438+
from markdown, so fix it to normalize.
439+
440+
turn:
441+
442+
<a rel="nofollow" href="http://example.com/" title="a title">
443+
into
444+
<a href="http://example.com/" rel="nofollow noopener" title="a title">
445+
'''
446+
if type(self) == Markdown2TestCase and s.find('a rel="nofollow"') != -1:
447+
if s.find('href="mailto:') == -1:
448+
# not a mailto url
449+
if 'rel="nofollow"' in s:
450+
if 'title="' in s:
451+
s = s.replace(' rel="nofollow" ', ' ').replace(' title=', ' rel="nofollow noopener" title=')
452+
else:
453+
s = s.replace(' rel="nofollow" ', ' ').replace('">', '" rel="nofollow noopener">')
454+
455+
return s
456+
else:
457+
# a mailto url
458+
return s.replace(' rel="nofollow" ', ' ')
459+
return s
460+
461+
423462
def test_string_markdown(self):
424463
p = StringHTMLProperty(self.client, 'test', '1', None, 'test', u2s(u'A string with <br> *embedded* \u00df'))
425464
self.assertEqual(p.markdown().strip(), u2s(u'<p>A string with &lt;br&gt; <em>embedded</em> \u00df</p>'))
@@ -437,7 +476,10 @@ def test_string_markdown_link(self):
437476
html_unescape = HTMLParser().unescape
438477

439478
p = StringHTMLProperty(self.client, 'test', '1', None, 'test', u2s(u'A link <[email protected]>'))
440-
self.assertEqual(html_unescape(p.markdown().strip()), u2s(u'<p>A link <a href="mailto:[email protected]">[email protected]</a></p>'))
479+
m = html_unescape(p.markdown().strip())
480+
m = self.mangleMarkdown2(m)
481+
482+
self.assertEqual(m, u2s(u'<p>A link <a href="mailto:[email protected]">[email protected]</a></p>'))
441483

442484
def test_string_markdown_javascript_link(self):
443485
# make sure we don't get a "javascript:" link
@@ -457,6 +499,7 @@ def test_string_markdown_forced_line_break(self):
457499
# Rather than using a different result for each
458500
# renderer, look for '<br' and require three of them.
459501
m = p.markdown()
502+
print(m)
460503
self.assertEqual(3, m.count('<br'))
461504

462505
def test_string_markdown_code_block(self):
@@ -498,14 +541,23 @@ def test_markdown_hyperlinked_url(self):
498541
# so rstrip \n.
499542
p = StringHTMLProperty(self.client, 'test', '1', None, 'test', u2s(u'http://example.com/'))
500543
m = p.markdown(hyperlink=1)
501-
self.assertEqual(m.rstrip('\n'), '<p><a href="http://example.com/">http://example.com/</a></p>')
544+
m = self.mangleMarkdown2(m)
545+
print(m)
546+
self.assertEqual(m.rstrip('\n'), '<p><a href="http://example.com/" rel="nofollow noopener">http://example.com/</a></p>')
502547

503548
p = StringHTMLProperty(self.client, 'test', '1', None, 'test', u2s(u'<http://example.com/>'))
504549
m = p.markdown(hyperlink=1)
505-
self.assertEqual(m.rstrip('\n'), '<p><a href="http://example.com/">http://example.com/</a></p>')
550+
m = self.mangleMarkdown2(m)
551+
self.assertEqual(m.rstrip('\n'), '<p><a href="http://example.com/" rel="nofollow noopener">http://example.com/</a></p>')
552+
553+
p = StringHTMLProperty(self.client, 'test', '1', None, 'test', u2s(u'[label](http://example.com/ "a title")'))
554+
m = p.markdown(hyperlink=1)
555+
m = self.mangleMarkdown2(m)
556+
self.assertEqual(m.rstrip('\n'), '<p><a href="http://example.com/" rel="nofollow noopener" title="a title">label</a></p>')
506557

507558
p = StringHTMLProperty(self.client, 'test', '1', None, 'test', u2s(u'![](http://example.com/)'))
508559
m = p.markdown(hyperlink=1)
560+
m = self.mangleMarkdown2(m)
509561
self.assertIn(m, [
510562
'<p><img src="http://example.com/" alt=""/></p>\n',
511563
'<p><img src="http://example.com/" alt="" /></p>\n',
@@ -515,11 +567,13 @@ def test_markdown_hyperlinked_url(self):
515567

516568
p = StringHTMLProperty(self.client, 'test', '1', None, 'test', u2s(u'An URL http://example.com/ with text'))
517569
m = p.markdown(hyperlink=1)
518-
self.assertEqual(m.rstrip('\n'), '<p>An URL <a href="http://example.com/">http://example.com/</a> with text</p>')
570+
m = self.mangleMarkdown2(m)
571+
self.assertEqual(m.rstrip('\n'), '<p>An URL <a href="http://example.com/" rel="nofollow noopener">http://example.com/</a> with text</p>')
519572

520573
p = StringHTMLProperty(self.client, 'test', '1', None, 'test', u2s(u'An URL https://example.com/path with text'))
521574
m = p.markdown(hyperlink=1)
522-
self.assertEqual(m.rstrip('\n'), '<p>An URL <a href="https://example.com/path">https://example.com/path</a> with text</p>')
575+
m = self.mangleMarkdown2(m)
576+
self.assertEqual(m.rstrip('\n'), '<p>An URL <a href="https://example.com/path" rel="nofollow noopener">https://example.com/path</a> with text</p>')
523577

524578
@skip_mistune
525579
class MistuneTestCase(TemplatingTestCase, MarkdownTests) :

0 commit comments

Comments
 (0)