Uploading unknown filetypes is no longer possible

[edwilde]

This appears to be caused by league/flysystem v3 changing to throwing exceptions rather than returning null - see a similar issue in laravel.

I have added the appropriate configurations to allow brf (braille format) files, as per the documentation.

This works just fine in CMS 4.13, but fails in CMS 5.0 with the following error:

error-log.ERROR: Uncaught Exception League\Flysystem\UnableToRetrieveMetadata: "Unable to retrieve the mime_type for file at location: c7481fdf07/brf-sample.brf. " at /var/www/mysite/www/vendor/league/flysystem/src/UnableToRetrieveMetadata.php line 49 {"exception":"[object] (League\\Flysystem\\UnableToRetrieveMetadata(code: 0): Unable to retrieve the mime_type for file at location: c7481fdf07/brf-sample.brf.  at /var/www/mysite/www/vendor/league/flysystem/src/UnableToRetrieveMetadata.php:49)"} []

The exception is raised on the flysystem adapter and unhandled in the Silverstripe Asset store class.

That was fixed, released, and then rolled back. It is now an opt-in to fix that behaviour by passing a new argument to the constructor.

Acceptance Criteria

• Pass true to the new opt-in argument in the FallbackMimeTypeDetector constructor.

NOTE

Once this has been resolved, we should raise a new PR to add https://github.com/silverstripe/silverstripe-assets/issues/573 to the changelog. No sense adding it until this is resolved.

PRs

• https://github.com/silverstripe/silverstripe-assets/pull/582

[GuySartorelli]

Turns out this is a bug upstream: https://github.com/thephpleague/flysystem/issues/1468 I've raised a PR to fix it: https://github.com/thephpleague/flysystem/pull/1710

I'll keep this issue open in the meantime - if they decide they don't consider it to be a bug, we may need to work around it.

[GuySartorelli]

The PR for flysystem has been merged - once a release has been tagged which includes that code I'll close this issue.

[GuySartorelli]

Frustratingly, the bug was fixed and released - and then immediately they decided to unfix it and make it opt-in. So we either have to bump the dependency (which we can't do in a patch) and pass in the new constructor argument to opt in to the fixed behaviour, or we have to implement a fix on our end which we can release as a patch.

I'll put this in our backlog to discuss.

[kinglozzer]

This is created via LocalFilesystemAdapter, right? Could we add this line to __construct() in that class?

$mimeTypeDetector = $mimeTypeDetector ?? new FallbackMimeTypeDetector(new FinfoMimeTypeDetector(), true);

If anyone is using < 3.23.0 it shouldn’t cause any harm, the extra constructor arg will just be discarded.

[GuySartorelli]

Right, that's exactly the opt-in behaviour I mention in my comment.

I forgot that constructor args are discarded if not used.... I guess we could ship that in a patch without bumping the dependency, then. We wouldn't be able to add a test for it though because the prefer-lowest build would fail.

[GuySartorelli]

@edwilde I've raised a PR to opt-in to the bug fix. Would you like to create a PR to add the braille format change into the 5.2.0 changelog?

[emteknetnz]

Linked PR has been merged, will be automatically patch released shortly

[edwilde]

@GuySartorelli PR for the changelog update is added 🚀