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

"Unknown Mimetype" exception thrown #23

Closed
MrMooky opened this issue Jun 2, 2022 · 5 comments
Closed

"Unknown Mimetype" exception thrown #23

MrMooky opened this issue Jun 2, 2022 · 5 comments

Comments

@MrMooky
Copy link

MrMooky commented Jun 2, 2022

After successfully using the package in a Laravel 8 project, I wanted to check it out with Laravel 9 and Statamic 3.3. I created the filesystem and also followed the Laravel 9 instructions.

When connecting the filesystem to Statamic, I get the following exception:

Screenshot 2022-06-02 at 10 20 59

I can upload files (they appear in the bunny.net File Manager) but are not shown in Statamic, instead, the mentioned exception is thrown.

@stevenmond
Copy link
Contributor

stevenmond commented Jun 21, 2022

The issue is the API doesn't actually return a value for ContentType which is used in code to determine the FileAttribute's MimeType through the normalizeObject method. A few potential options are reading the raw file and creating a mime type for it or just guessing one based on the file extension.

I'd be happy to do a PR for this

** Edit **
Deleted a code section where I misread the intention of the function

** Edit 2 **
Symphony has a few options - there's the full guesser (guessMimeType) which tries to parse the file or there's a function called getMimeTypes which will return an array of potential options based on the file extension.

https://symfony.com/doc/current/components/mime.html
https://github.com/symfony/symfony/tree/6.2/src/Symfony/Component/Mime

** Edit 3 **
League has an extension specifically for this functionality. Just a matter of deciding the priority on this (i.e. if there's a desire to read the file first and use extension as a fallback or vice versa to save a downloading the file from Bunny)
https://flysystem.thephpleague.com/docs/advanced/mime-type-detection/

@stevenmond
Copy link
Contributor

stevenmond commented Jun 22, 2022

Adding on to the prior comment what appears to work would be something like:

  1. Adding the following dependency - league/mime-type-detection
  2. Add "use League\MimeTypeDetection\FinfoMimeTypeDetector;" to the top of BunnyCDNAdapter
  3. Add one of functions below to the BunnyCDNAdapter:

The standard implementation would look like the below, but there's an issue with large file sizes as it has to download the whole file to function.

 /**
     * Detects the mime type from the provided file path
     * 
     * @param string $path
     * @return ?string
     */
    public function detectMimeType(string $path): ?string {
        $detector = new FinfoMimeTypeDetector();
        return $detector->detectMimeType($path, $this->read($path));
    }

Below is a tweaked version to handle larger file sizes. The 80 byte number comes from https://en.wikipedia.org/wiki/List_of_file_signatures listed in https://stackoverflow.com/questions/8673407/how-many-bytes-are-required-for-accurate-mime-type-detection. 80 bytes was longer than anything listed although it's not an exhaustive list. The stream needs the byte length limiter as otherwise it will read the whole stream (and be the same as read).

 /**
     * Detects the mime type from the provided file path
     * 
     * @param string $path
     * @return ?string
     */
    public function detectMimeType(string $path): ?string {
        $detector = new FinfoMimeTypeDetector();
        return $detector->detectMimeType($path, stream_get_contents($this->readStream($path), 80));
    }

The final option would be something like the below that puts preference towards detection via file name in order to avoid downloading or streaming the file. The default behavior of the function detectMimeType does the opposite. It tries to read the file (or stream) and then use the extension as a fallback. Detecting via file reading should be more accurate while this version is quicker/doesn't use extra API calls where not needed.

/**
     * Detects the mime type from the provided file path
     * 
     * @param string $path
     * @return ?string
     */
    public function detectMimeType(string $path): ?string {
        $detector = new FinfoMimeTypeDetector();
        $mimeType = $detector->detectMimeTypeFromPath($path);

        if(!$mimeType) {
            return $detector->detectMimeTypeFromBuffer(stream_get_contents($this->readStream($path), 80));
        }

        return $mimeType;        
    }
  1. Change normalizeObject to as follows to use the new mime-type checker instead
/**
     * @param array $bunny_file_array
     * @return StorageAttributes
     */
    protected function normalizeObject(array $bunny_file_array): StorageAttributes
    {
        return match ($bunny_file_array['IsDirectory']) {
            true => new DirectoryAttributes(
                Util::normalizePath(
                    str_replace(
                        $bunny_file_array['StorageZoneName'] . '/',
                        '/',
                        $bunny_file_array['Path'] . $bunny_file_array['ObjectName']
                    )
                )
            ),
            false => new FileAttributes(
                Util::normalizePath(
                    str_replace(
                        $bunny_file_array['StorageZoneName'] . '/',
                        '/',
                        $bunny_file_array['Path'] . $bunny_file_array['ObjectName']
                    )
                ),
                $bunny_file_array['Length'],
                Visibility::PUBLIC,
                date_create_from_format('Y-m-d\TH:i:s.v', $bunny_file_array['LastChanged'])->getTimestamp(),
                $this->detectMimeType($bunny_file_array['Path'] . $bunny_file_array['ObjectName']),
                $this->extractExtraMetadata($bunny_file_array)
            )
        };
    }

@ben182
Copy link
Contributor

ben182 commented Jul 13, 2022

@stevenmond works great, can you provide a PR?

@sifex
Copy link
Collaborator

sifex commented Jul 13, 2022

This should now be working now, massive thanks to @stevenmond 👏🏻.

I wasn't happy with the 2× HTTP calls (1× for directory listing and 1× for contents) but I've test that it only occurs when the user asks for the mimeType explicitly.

The only two other notes are:

  • I haven't backported this to v1 and v2. Please let me know if you require that.
  • The mimetype text/plain has to throw according to FilesystemAdapterTestCase::fetching_unknown_mime_type_of_a_file

I'll reach out to Bunny and see if there's an ETA on first party mimeType support

@sifex
Copy link
Collaborator

sifex commented Jul 13, 2022

Closing with the release of https://github.com/PlatformCommunity/flysystem-bunnycdn/releases/tag/v3.1.2

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

4 participants