thephpleague / flysystem

Abstraction for local and remote filesystems
https://flysystem.thephpleague.com
MIT License
13.36k stars 827 forks source link

FinfoMimeTypeDetector: incomplete handing of INCONCLUSIVE_MIME_TYPES #1468

Open marc-mabe opened 2 years ago

marc-mabe commented 2 years ago

Bug Report

Q A
BC Break yes/no
Version 3.0.7

Summary

FinfoMimeTypeDetector has defined INCONCLUSIVE_MIME_TYPES which contains text/plain. This will be used on detectMimeType and if the detected type in this list it falls back to detectMimeTypeFromPath.

On the other hand the local file adapter uses detectMimeTypeFromFile which does not check for INCONCLUSIVE_MIME_TYPES at all.

How to reproduce

  1. Create a tsv file (tab separated values)

  2. Use local file adapter mimeType() (which used FinfoMimeTypeDetector::detectMimeTypeFromFile)

  3. you will get text/plain

  4. Create the same tsv file in memory adapter

  5. Use local file adapter mimeType() (which used FinfoMimeTypeDetector::detectMimeType)

  6. you will get text/tab-separated-values

Idea how to fix

Something like this:

    public function detectMimeType(string $path, $contents): ?string
    {
            $mimeType = is_string($contents)
                ? $this->detectMimeTypeFromBuffer($contents)
                : null;

            if ($mimeType !== null && ! in_array($mimeType, self::INCONCLUSIVE_MIME_TYPES)) {
                return $mimeType;
            }

            return $this->detectMimeTypeFromPath($path) ?: $mimeType;
    }

    public function detectMimeTypeFromFile(string $path): ?string
    {
        $mimeType = @$this->finfo->file($path) ?: null;

        if ($mimeType !== null && ! in_array($mimeType, self::INCONCLUSIVE_MIME_TYPES)) {
            return $mimeType;
        }

        return $this->detectMimeTypeFromPath($path) ?: $mimeType;
    }
GuySartorelli commented 1 year ago

Yup, this is causing us some grief as well. Specifically the lack of

if fallback detectMimeTypeFromPath returns null use the previous detected mime type

We're getting a valid text/plain from detectMimeTypeFromFile(), but then it falls back to detectMimeTypeFromPath() which returns null and results in an exception being thrown.

I've raised https://github.com/thephpleague/flysystem/pull/1710 to handle that part of this issue.