NEW: Add the ability to specify the MimeTypeDetector for the LocalFilesystemAdapter

[madmatt]

The AssetAdapter class has a really weird constructor which has made this super confusing.

SilverStripe\Assets\Flysystem\AssetAdapter extends SilverStripe\Assets\Flysystem\LocalFilesystemAdapter which in turn extends League\Flysystem\Local\LocalFilesystemAdapter but the constructors are totally different.

• AssetAdapter takes arguments string? $root, int $writeFlags, int $linkHandling
• The Flysystem LocalFilesystemAdapter takes arguments string $location, VisibilityConverter $visibility, int $writeFlags, int $linkHandling, MimeTypeDetector $mimeTypeDetector, bool $lazyRootCreation.

Basically, I have a weird file type (*.brf files which is a Braille file format used for physical Braille devices that convert words into Braille. It's essentially a fancy ASCII document, but finfo detects it as text/plain which according to Flysystem is an "inconclusive file type" and therefore is not a valid mime type.

Not sure if this requires documentation or not, happy to make a PR to add docs to the developer-docs repo if you think it does and I can link them here.

Here's how you use it (to allow text/plain for example):

In app/_config/assets.yml:

# Use the FinfoMimeTypeDetector directly instead of the Fallback. We remove text/plain
SilverStripe\Assets\Flysystem\AssetAdapter:
  default_mime_type_detector: '%$League\MimeTypeDetection\FinfoMimeTypeDetector'
SilverStripe\Core\Injector\Injector:
  League\MimeTypeDetection\FinfoMimeTypeDetector:
    constructor:
      MagicFile: ''
      ExtensionMap: null
      BufferSampleSize: null
      InconclusiveMimeTypes:
        - 'application/x-empty'
        - 'text/x-asm'
        - 'application/octet-stream'
        - 'inode/x-empty'

[madmatt]

Also, I can't figure out a good way to write a test for this to be honest, without injecting some custom file into the repo which feels a bit overkill. Thoughts welcome on how to test that, or if it's worth it to do so.

[michalkleiner]

Is there not a simple way how to treat a file based on file extension and skip all the guesswork?

[madmatt]

By default Flysystem uses the finfo mimetype checker. There's a separate ExtensionMimeTypeDetector which does it based on file extension but that is just doing a 'dumb' lookup between file extension and mime type so ultimately it's worse than finfo in the majority of cases because it doesn't actually look at the file contents, only the extension, which I'd say is a pretty big security risk.

[michalkleiner]

Yeah, I was thinking just for the inconclusives as a fallback, not to have it as the primary/default detector. But that might be even more complicated than what you've already achieved.

[GuySartorelli]

For your specific use case of the brf file, see https://github.com/silverstripe/silverstripe-assets/issues/572 - a bug in flysytem resulted in "inconclusive" file types returning null instead of the detected file type, which resulted in an exception. A PR to fix that has been accepted and merged, and once that is tagged I think that will solve your use case.

I do agree though that improved flexibility here could be potentially beneficial. I haven't looked properly at this PR, just thought you'd like to know that there is a solution waiting to be tagged for the use case you mentioned in the description.

[GuySartorelli]

@madmatt https://github.com/silverstripe/silverstripe-assets/pull/582 has just been merged and tagged - do you still have a use case for this change?

[madmatt]

Nope I don't think so, thanks @GuySartorelli! I'll look at updating to the tag and make sure it still works on our side. Thanks for investigating!