Issue 3402 - Properly handle nested inline tags

cms/converters.py

 # coding: utf-8
 
 # This file is part of the Adblock Plus web scripts,
-# Copyright (C) 2006-2015 Eyeo GmbH
+# Copyright (C) 2006-2016 Eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 #
@@ skipping 159 matching lines @@
 
     # Insert attributes
     result = escape(result)
     def stringify_attribute((name, value)):
       return '%s="%s"' % (
         escape(name),
         escape(self.insert_localized_strings(value, {}))
       )
 
     for tag in self.whitelist:
-      allowed_tags = [t % re.escape(tag_name)
-                      for t in ("\<%s[^<>]*?\>", "\<\/%s\>")

[Sebastian Noack, 2016/01/21 13:55:56]

Nit: None of your escaping applies to regexp as you don't use double backslashes
or  r"..". However, that shows that escaping isn't even necessary here.

Nit: You could simplify the surrounding Python code if you use a single regexp:
"<(?:%s[^<>]*?|/%s)>"

[kzar, 2016/02/19 14:32:28]

Done.

-                      for tag_name in self.whitelist.difference({tag})]

[Sebastian Noack, 2016/01/21 13:55:56]

Nit: self.whitelist - {tag}

Besides being more concise, it's probably also easier to understand as it's
clear that arithmetic semantics apply and you don't wonder whether
Set.difference() is some symmetric operation.

[kzar, 2016/02/19 14:32:28]

Done.

-      allowed_contents = "(?:%s)" % "|".join(["[^<>]"] + allowed_tags)
+      allowed_contents = "(?:[^<>]|%s)" % "|".join((
+        "<(?:%s[^<>]*?|/%s)>" % (t, t)
+         for t in map(re.escape, self.whitelist - {tag})
+      ))
       saved = saved_attributes.get(tag, [])
       for attrs in saved:
         attrs = map(stringify_attribute, attrs)
         result = re.sub(
           r"%s(%s*?)%s" % (re_escape("<%s>" % tag), allowed_contents,

[Sebastian Noack, 2016/01/21 13:55:56]

I wonder whether you should parse the content recursively to make sure that tags
are closed correctly.

[kzar, 2016/02/19 14:32:28]

I guess so but probably overkill, we didn't check that before.

                            re_escape("</%s>" % tag)),
           lambda match: r'<%s%s>%s</%s>' % (
             tag,
             " " + " ".join(attrs) if attrs else "",
             match.group(1),
             tag
           ),
           result, 1, flags=re.S
         )
       result = re.sub(
@@ skipping 240 matching lines @@
         stack.pop()
       stack[-1]["subitems"].append(item)
       stack.append(item)
     return structured
 
 converters = {
   "html": RawConverter,
   "md": MarkdownConverter,
   "tmpl": TemplateConverter,
 }