silverstripe / silverstripe-assets

Silverstripe Assets component
BSD 3-Clause "New" or "Revised" License
9 stars 65 forks source link

`ImageManipulation::variantParts()` throws exceptions for some valid variants #586

Open GuySartorelli opened 5 months ago

GuySartorelli commented 5 months ago

Module version(s) affected

Found in 5.x-dev but presumably has been around since 4.0

Description

The ImageManipulation::variantParts() throws an exception for some valid variants, such as FitMax variants.

This is the case for any variant which has another variant method name as part of it. For example, Fit variants work, but FitMax variants do not.

How to reproduce

The following code will die on line 4 with InvalidArgumentException: Invalid variant name arguments: FitMaxWzEwMCwyMDBd

use SilverStripe\Assets\File;
$imageManip = new File();
$name = $imageManip->variantName('FitMax', '100', '200');
$parts =  $imageManip->variantParts($name);

Possible Solution

Possibly sorting the array of methods in alphabetical descending order before using it in the regex could work, though that feels pretty flaky - and would still potentially incorrectly think a Fit variant is actually a FitMax variant if the base64 component starts with Max (though I'm not sure there's anything we can do about that edge case).

More robust (but more complex - probably just go with the first option) would be to find all combinations of methods like this, and then instead of writing the regex like (Fit|FitMax|AnotherMethod), we'd have (Fit(Max)?)|AnotherMethod)

Additional Context

No response

Validations