plone / Products.CMFPlone

The core of the Plone content management system
https://plone.org
GNU General Public License v2.0
246 stars 186 forks source link

add inline svg transform #3302

Open iham opened 3 years ago

iham commented 3 years ago

PLIP (Plone Improvement Proposal)

Responsible Persons

Proposer: @iham

Seconder: @agitator

Abstract

Render uploaded (Plone Content Type) images inline if they are svg‘s.

Motivation

Assumptions

Should/Could be in core as it deals with how svgs images are rendered using a transform. This is not theme specific. (I wrote the issue in plonetheme.barceloneta in the first place, which was wrong.)

Maybe adding a control panel (image handling?) checkbox to de/activate that transform can be an option, to keep the choice at user-level.

Proposal & Implementation

my approach was to write an adapter (python2 code!):

configure.zcml

  <adapter
      name="plone.inline-svg"
      for="zope.interface.Interface <a theme/addon Layer/Plone default layer>"
      factory=".transform.InlineSVGTransform"
      />

and the transform itself: transform.py

# -*- coding: utf-8 -*-
"""Transforming svg images to inline."""
from lxml import etree
from lxml.etree import Element
from lxml.html import fromstring
from repoze.xmliter.serializer import XMLSerializer

from zope.component import getUtility
from zope.interface import implementer

from plone import api
from plone.transformchain.interfaces import ITransform
from plone.uuid.interfaces import IUUIDGenerator

@implementer(ITransform)
class InlineSVGTransform(object):
    """Transforming SVG images to inline svg &
        adding attributes given from the image.
        See http://web-accessibility.carnegiemuseums.org/code/svg/ for details.
    """

    # right before diazo
    order = 8840
    xpath_expr = '//body[not(contains(@class, "zmi"))]//img[not(contains(@src, "++")) and substring(@src,string-length(@src) -string-length(".svg") +1) = ".svg"]'  # noqa E501

    def __init__(self, published, request):
        self.published = published
        self.request = request

    def transformBytes(self, result, encoding):  # noqa N802
        return self.transformIterable([result], encoding)

    def transformUnicode(self, result, encoding):  # noqa N802
        return self.transformIterable([result], encoding)

    def transformIterable(self, result, encoding):  # noqa N802
        content_type = self.request.response.getHeader('Content-Type')
        if content_type and content_type.startswith('text/html') and result:
            tree = None
            if isinstance(result, XMLSerializer):
                tree = result.tree.getroot()
            else:
                if result[0]:
                    tree = fromstring(unicode(result[0].decode('utf-8')))

            if tree is not None:
                portal = api.portal.get()
                changed = False
                for node in tree.xpath(self.xpath_expr):
                    src = node.attrib['src']
                    if len(src) > 0:
                        rel_path = src.replace(
                            self.request.SERVER_URL,
                            '',
                        ).split('/@@images/')[0]

                        img = portal.restrictedTraverse(rel_path)
                        if img:
                            svg = fromstring(
                                unicode(img.image.data.decode('utf-8')),
                            )
                            self._attributes_manipulation(img, node, svg)
                            node.getparent().replace(node, svg)
                            changed = True
                if changed:
                    if not isinstance(result, XMLSerializer):
                        result[0] = etree.tostring(tree, encoding=encoding)

        return result

    def _attributes_manipulation(self, img, node, svg):
        """setting class, title, alt."""
        # classes
        classes = []
        classes.append(node.attrib['class'])
        classes.append(svg.attrib.get('class', ''))
        svg.set('class', ' '.join(classes))

        # alt
        desc = Element('desc')
        desc.text = node.attrib['alt']
        desc.set('id', 'desc')
        svg.insert(0, desc)

        # title
        title = Element('title')
        title.text = node.attrib['title']
        title.set('id', 'title')
        svg.insert(0, title)

        # inline svg id's still must be unique including the svg id itself
        self._unify_ids(svg.getparent())

        # set aria attributes
        svg.set('role', 'img')
        # and point to the corrected title and desc id's
        svg.set(
            'aria-labelledby',
            '{title} {desc}'.format(
                title=svg.xpath('title/@id')[0],
                desc=svg.xpath('desc/@id')[0],
            ),
        )

    def _unify_ids(self, svg):
        """As the ids of inline svgs are not allowed to occur multiple times,
           we need to unify them."""
        uid = getUtility(IUUIDGenerator)()
        for node in svg.xpath('//*[@id and string-length(@id)!=0]'):
            node.set(
                'id',
                '{id}-{uid}'.format(
                    id=node.attrib['id'],
                    uid=uid,
                ),
            )

Deliverables

Code this as a core-thing.

Risks

Participants

I tested the approach above on a project and it worked fine so far. If somebody shouts a go, i implement it, wherever you want it to be placed.

mauritsvanrees commented 3 years ago

I wouldn't recommend this. This opens up a security hole that we patched in the May 2021 hotfix. See the XSS in file upload vulnerability page for a brief description.

Having said that, the approach is possible, but you should clean the svg to at least not contain script tags, on:click attributes, processing instructions, stuff like that. But I would only do that on a site that is only available in an internal network, and only has users that you trust, and you trust that your users won't get hacked. In other words: let's not add this in the core.

For small svg images from a filesystem theme, this could be fine: having the images inline means less requests are needed, although I guess this is less of a problem these days when you have http/2 configured in your webserver.

iham commented 3 years ago

@mauritsvanrees thanks for pointing out that risk.

i guess this will be a collective thing then. still a transform taking care of script tags and such.

being able to style svg via css is my main focus here

MrTango commented 3 years ago

But if you want to style it with CSS, i gues it will not be random images from users right? So if you have SVGs for special usecases, i gues a dedicated CT might be a better solution. This CT would be restricted to trusted users.