Allow type-specific image manpulation handlers, build one for SVG

[sminnee]

By default, SVG files should be given a specialised and much simpler handler. SVG are at least as much a part of the internet as ReactJS and if SS4 was impossible to use to build ReactJS-based sites we'd think it's a bit broken. ;)

First up, this requires that image manipulation handlers can be plugged in for specific types, probably, with a fail-over to a default handler. Ideally, this could mean that other file types could have variant processors included (e.g. you might have a PDF handler that produces plaintext) but that's not a requirement for this use-case.

As for the SVG handler, we can keep things simple:

• For any of the resize operations, return the same SVG file but just return different values width & height.
• For any other operation (e.g. greyscale) have it either do a no-op, or throw an error, depending on a configuration setting, something like "error_on_supported_operation = true/false"

My hope/expectation is that this level of functionality would be possible without a SVG parser.

[sminnee]

A couple of approaches for type-specific image backends:

• Create a dispatcher implementation of Image_Backend, maybe call it ImageBackendDispatcher
• In ImageManipulation::getImageBackend(), use Image_Backend.extension as the service name, which means that you can override it for a single file extension. Simpler, but it becomes a bit more awkward to have a single backend that applies to multiple file extensions, or to dispatch on some other criteria.

[tractorcow]

Instead of ImageBackendDispatcher we put the logic into the existing ImageBackendFactory? It already supports conditional creation as well as caching based on SHA / filename.

[tractorcow]

Replace:

if ($store->exists() && $store->getIsImage()) {
    $backend = $this->creator->create($service, $params);
}

With

if ($store->exists() && $store->getIsImage()) {
    $service = $this->backendFor(File::get_extension($store->getFilename()));
    $backend = $this->creator->create($service, $params);
}

[tractorcow]

YML config:

---
Name: assetsimage
---
SilverStripe\Core\Injector\Injector:
  SilverStripe\Assets\ImageBackendFactory:
    constructor:
      creator: '%$SilverStripe\Core\Injector\InjectionCreator'
  SilverStripe\Assets\Image_Backend:
    class: SilverStripe\Assets\InterventionBackend
    factory: '%$SilverStripe\Assets\ImageBackendFactory'
  SilverStripe\Assets\Image_Backend.svg:
    class: SilverStripe\Assets\SvgBackend

All the logic would be in ImageBackendFactory, which could conditionally delegate back to a more precise service via injector.

[tractorcow]

if ($store->exists() && $store->getIsImage()) {
    $ext = File::get_extension($store->getFilename());
    $serviceExt = Image_Backend::class.'.'.$ext;
    if ($ext && Config::inst()->get(Injector::class, $serviceExt)) {
        $backend = Injector::inst()->createWithArgs($serviceExt, $params);
    } else {
      $backend = $this->creator->create($service, $params);
    }
}

[sminnee]

Yeah, seems reasonable to put it into ImageBackendFactory.

[maxime-rainville]

We disallow SVG upload out of the box, because it's a security risk. Is it really worth it adding convenience methods for a file types we don't technically support?

[sminnee]

It was a couple of years since we wrote this; at the time the discussion was about trying to allow for some kind of module that would scrub SVG. If you’re looking to prune the list of enhancements I’d probably roll everything up into a “support user-uploaded SVG in essentials recipe” story or something like that, close all these tickets that are specific solution ideas, and link to the closed tickets from that umbrella ticket.

Or we could just close it. I acknowledge that blocking SVGs breaks user-expectations, even though it’s tricky to solve securely.