Automatically remove invalid attributes for iframes

CHANGES.rst

@@ -6,7 +6,8 @@ There's a frood who really knows where his towel is.
 1.1b2 (unreleased)
 ^^^^^^^^^^^^^^^^^^
 
-- Nothing changed yet.
+- Automatically remove invalid attributes for iframes (closes `#44`_).
+  [rodfersou]
 
 
 1.1b1 (2016-03-24)
@@ -136,3 +137,4 @@ There's a frood who really knows where his towel is.
 .. _`#32`: https://github.com/simplesconsultoria/sc.embedder/issues/32
 .. _`#39`: https://github.com/simplesconsultoria/sc.embedder/issues/39
 .. _`#42`: https://github.com/simplesconsultoria/sc.embedder/issues/42
+.. _`#44`: https://github.com/simplesconsultoria/sc.embedder/issues/44

CONTRIBUTORS.rst

@@ -8,6 +8,7 @@ This package would not have been possible without the contribution of the follow
 - Héctor Velarde
 - João S. O. Bueno
 - Juan Pablo Giménez
+- Rodrigo Ferreira de Souza
 - `et alii`_
 
 Development sponsored by `Simples Consultoria`_.

src/sc/embedder/content/embedder.py

@@ -40,6 +40,15 @@
 import urlparse
 
 
+# https://html.spec.whatwg.org/multipage/embedded-content.html#the-iframe-element
+# https://html.spec.whatwg.org/multipage/dom.html#global-attributes
+ALLOWED_IFRAME_ATTRIBUTES = [
+    'accesskey', 'allowfullscreen', 'class', 'contenteditable', 'contextmenu', 'dir',
+    'draggable', 'dropzone', 'height', 'hidden', 'id', 'is', 'itemid', 'itemprop',
+    'itemref', 'itemscope', 'itemtype', 'lang', 'name', 'sandbox', 'spellcheck',
+    'src', 'srcdoc', 'style', 'tabindex', 'title', 'translate', 'width']
+
+
 grok.templatedir('templates')
 
 
@@ -277,6 +286,23 @@ def _validate_url(self, url):
             return False
         return True
 
+    def _remove_iframe_invalid_attributes(self, iframe):
+        """Remove invalid attributes for iframe.
+
+        :param iframe: [required] lxml element with iframe.
+        :type url: lxml element
+        :returns: fixed lxml element with iframe
+        :rtype: lxml element
+        """
+        items = iframe.attrib.items()
+        for k, v in items:
+            if k.lower() not in ALLOWED_IFRAME_ATTRIBUTES:
+                del(iframe.attrib[k])
+            if not k.islower():
+                iframe.attrib[k.lower()] = v
+                del(iframe.attrib[k])
+        return iframe
+
     def load_oembed(self, action):
         url = self.widgets['url'].value
 
@@ -292,6 +318,15 @@ def load_oembed(self, action):
             json_data = self.get_fallback(url)
             if json_data is None:
                 return
+        # html parameter not always required:
+        # https://github.com/abarmat/python-oembed/blob/master/oembed/__init__.py#L157-L167
+        # https://github.com/abarmat/python-oembed/blob/master/oembed/__init__.py#L181-L187
+        if 'html' in json_data:
+            el = html.fromstring(json_data['html'])
+            # Somethimes the embedded data will not be an iframe
+            if el.tag == 'iframe':
+                el = self._remove_iframe_invalid_attributes(el)
+                json_data['html'] = html.tostring(el)
         for k, v in self.tr_fields.iteritems():
             if json_data.get(k):
                 self.widgets[v].value = unicode(json_data[k])

src/sc/embedder/tests/test_content.py

@@ -1,6 +1,8 @@
 # -*- coding: utf-8 -*-
 from httmock import all_requests
 from httmock import HTTMock
+from lxml import html
+from lxml.html.builder import IFRAME
 from plone.app.testing import setRoles
 from plone.app.testing import TEST_USER_ID
 from plone.namedfile.file import NamedBlobImage
@@ -66,6 +68,28 @@ def test_interface(self):
         self.assertTrue(IEmbedder.providedBy(self.multimedia))
         self.assertTrue(verifyObject(IEmbedder, self.multimedia))
 
+    def test_remove_iframe_invalid_attributes(self):
+        add_view = self.folder.unrestrictedTraverse('++add++sc.embedder')
+        add_form = add_view.form_instance
+
+        # iframe without attributes
+        iframe = IFRAME()
+        self.assertEqual('<iframe></iframe>', html.tostring(iframe))
+        add_form._remove_iframe_invalid_attributes(iframe)
+        self.assertEqual('<iframe></iframe>', html.tostring(iframe))
+
+        # iframe with allowed attribute
+        iframe.attrib['src'] = 'http://www.plone.org'
+        self.assertEqual('<iframe src="http://www.plone.org"></iframe>', html.tostring(iframe))
+        add_form._remove_iframe_invalid_attributes(iframe)
+        self.assertEqual('<iframe src="http://www.plone.org"></iframe>', html.tostring(iframe))
+
+        # iframe with not allowed attribute
+        iframe.attrib['frameborder'] = '0'
+        self.assertEqual('<iframe src="http://www.plone.org" frameborder="0"></iframe>', html.tostring(iframe))
+        add_form._remove_iframe_invalid_attributes(iframe)
+        self.assertEqual('<iframe src="http://www.plone.org"></iframe>', html.tostring(iframe))
+
     def test_custom_player_size_addform(self):
         """ Check if the custom size applies to the embed code in the
             add form.
@@ -279,7 +303,10 @@ def test_videojs(self):
 
         # We trigger the action of load
         add_form.handleLoad(add_form, action)
-        iframe = u'\n<iframe src="http://nohost/plone/test-folder/@@embedder_videojs?src=http%3A%2F%2Fvjs.zencdn.net%2Fv%2Foceans.webm&type=video%2Fwebm"\n        class="vjs-iframe"\n        allowfullscreen="1" mozallowfullscreen="1" webkitallowfullscreen="1"\n        frameborder="0">\n</iframe>\n'
+        iframe = (
+            u'<iframe src="http://nohost/plone/test-folder/@@embedder_videojs?src='
+            u'http%3A%2F%2Fvjs.zencdn.net%2Fv%2Foceans.webm&amp;type=video%2Fwebm" '
+            u'class="vjs-iframe" allowfullscreen="1">\n</iframe>')
         self.assertEqual(
             u'', add_form.widgets['IDublinCore.title'].value)
         self.assertEqual(
@@ -297,12 +324,21 @@ def test_videojs(self):
         video.height = '264'
         video.embed_html = iframe
 
-        self.assertItemsEqual({u'thumb_html': u'<iframe src="http://nohost/plone/test-folder/@@embedder_videojs?src=http%3A%2F%2Fvideo-js.zencoder.com%2Foceans-clip.webm&amp;type=video%2Fwebm" class="vjs-iframe" allowfullscreen="1" mozallowfullscreen="1" webkitallowfullscreen="1" frameborder="0" width="188" height="141">\n</iframe>',
-                               u'embed_html': u'\n<iframe src="http://nohost/plone/test-folder/@@embedder_videojs?src=http%3A%2F%2Fvideo-js.zencoder.com%2Foceans-clip.webm&type=video%2Fwebm"\n        class="vjs-iframe"\n        allowfullscreen="1" mozallowfullscreen="1" webkitallowfullscreen="1"\n        frameborder="0">\n</iframe>\n',
-                               u'description': u'',
-                               u'title': u'Oceans clip'
-                               },
-                              json.loads(video.unrestrictedTraverse('@@tinymce-jsondetails')()))
+        self.assertEqual({
+            u'thumb_html': (
+                u'%3Ciframe%20src%3D%22http%3A//nohost/plone/test-folder/%40%40'
+                u'embedder_videojs%3Fsrc%3Dhttp%253A%252F%252Fvjs.zencdn.net%252Fv%252F'
+                u'oceans.webm%26amp%3Btype%3Dvideo%252Fwebm%22%20class%3D%22vjs-iframe'
+                u'%22%20allowfullscreen%3D%221%22%20width%3D%22188%22%20height%3D%2214'
+                u'1%22%3E%0A%3C/iframe%3E'),
+            u'embed_html': (
+                u'%3Ciframe%20src%3D%22http%3A//nohost/plone/test-folder/%40%40'
+                u'embedder_videojs%3Fsrc%3Dhttp%253A%252F%252Fvjs.zencdn.net%252Fv%252F'
+                u'oceans.webm%26amp%3Btype%3Dvideo%252Fwebm%22%20class%3D%22vjs-iframe'
+                u'%22%20allowfullscreen%3D%221%22%3E%0A%3C/iframe%3E'),
+            u'description': u'',
+            u'title': u'Oceans clip'},
+            json.loads(video.unrestrictedTraverse('@@tinymce-jsondetails')()))
 
     def test_facebook_manual(self):
         add_view = self.folder.unrestrictedTraverse('++add++sc.embedder')