Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FinfoMimeTypeDetector: incomplete handing of INCONCLUSIVE_MIME_TYPES #1468

Open
marc-mabe opened this issue May 19, 2022 · 1 comment
Open

Comments

@marc-mabe
Copy link

marc-mabe commented May 19, 2022

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

  • INCONCLUSIVE_MIME_TYPES should be used in all cases a path exists and fall back to detectMimeTypeFromPath
  • if fallback detectMimeTypeFromPath returns null use the previous detected mime type

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
Copy link
Contributor

GuySartorelli commented Oct 10, 2023

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 #1710 to handle that part of this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants