Laravel-medialibrary: Error on save when conversion tries to clean up tmp dir

[ammonkc]

Every time I attach an image the media conversion throws an error. It looks like it's trying to delete /tmp to clean up the temp items. What am I doing wrong?

    "message": "rmdir(/tmp): Permission denied",
    "exception": "ErrorException",
    "file": "/var/www/html/ptpkg/vendor/spatie/image/src/GlideConversion.php",
    "line": 107,

[freekmurze]

Could you try setting the temporary_directory_path key in the medialibrary config file to another value?

[ammonkc]

Setting that config value doesn't seem to fix it. I set it to:

'temporary_directory_path' => storage_path('medialibrary/temp'),

medialibrary/temp is used as the temp folder, but it never gets cleaned up because it still tries to delete /tmp

"message": "rmdir(/tmp): Permission denied",
    "exception": "ErrorException",
    "file": "/var/www/html/ptpkg/vendor/spatie/image/src/GlideConversion.php",
    "line": 107,
    "trace":
....

EDIT: Here are details about the development server I'm using incase this helps.

OS Release........: CentOS Linux release 7.4.1708 (Core)
kernel............: 3.10.0-693.11.1.el7.x86_64
Apache............: 2.4.6
PHP...............: 7.1.12
MySQL.............: 5.7.20

[ptiuma]

I have same error after last update
ErrorException In GlideConversion.php line 107 :

rmdir(/tmp): Permission denied

[lostincode]

Getting this today as well. What version of medialibrary is everyone using?

[ptiuma]

 "spatie/laravel-medialibrary": "6.0.0",

i set in config 'temporary_directory_path' => storage_path('medialibrary/temp'),
and can view temp files in this dir - medialibrary/temp
but error same - rmdir(/tmp): Permission denied
Working only after i've commented line 107 In GlideConversion.php

[lostincode]

fwiw, the server I was getting this on was also running PHP 7.1.12. I provisioned a fresh server running 7.2 and the issue was not there. _Not saying PHP is the culprit for sure_ but a good place to check. I didn't have enough time to dive into it. I did check folder permissions and those looked good.

[ammonkc]

I'm on version 6.6.7

"spatie/laravel-medialibrary": "^6.0"

[htulibacki]

@ammonkc @freekmurze @lostincode @ptiuma
I came across a similar problem today, and what I found out is that medialibrary is trying to delete a temporary folder only when it's empty. So if you create for example an empty folder in /tmp dir, everything works as expected.

[freekmurze]

Feel free to PR a fix for this.

I would think that just using this config variable to set the temp directory should work for most users: https://github.com/spatie/laravel-medialibrary/blob/f3d8c33/config/medialibrary.php#L107

[htulibacki]

I tried this earlier, but somehow it did not work (it still wants to delete /tmp). I'll try to look at this after work.

[htulibacki]

Ok, it's related to https://github.com/spatie/image/issues/38, so i think this issue can be closed.

[dbpolito]

I would think that just using this config variable to set the temp directory should work for most users: https://github.com/spatie/laravel-medialibrary/blob/f3d8c33/config/medialibrary.php#L107

This won't work, because it uses sys_get_temp_dir() at https://github.com/spatie/image/blob/master/src/GlideConversion.php#L50

[Gummibeer]

Hey,
I don't have problems with the /tmp dir but the configured temp directory got flooded in my case. 😕
My solution is to override the FileManipulator and move from the TemporaryDirectory::deleteDirectory() to the filesystem one app('files')->deleteDirectory($temporaryDirectory->path());. I've also changed the directory names from all the random strings to speaking ones media-[id] like the real storage is organized. With these changes it's also possible to identify if the directory is removable or not cause I can search for the Media-ID and don't have to do some magic like if last_modified < now-1h or something like this.
Putting the delete call in the finally and using a try-catch block there is also no exception that can stop the deletion of the directory.

I think some of the namings are a bit too specific for my case (primary original) but I think that it's a good starting point for a discussion. If these changes are welcome I can also put them in a PR.

<?php

namespace App\Libs;

use Exception;
use Spatie\MediaLibrary\Conversion\Conversion;
use Spatie\MediaLibrary\Conversion\ConversionCollection;
use Spatie\MediaLibrary\Events\ConversionHasBeenCompleted;
use Spatie\MediaLibrary\Events\ConversionWillStart;
use Spatie\MediaLibrary\FileManipulator as SpatieFileManipulator;
use Spatie\MediaLibrary\Filesystem\Filesystem;
use Spatie\MediaLibrary\Helpers\File as MediaLibraryFileHelper;
use Spatie\MediaLibrary\Media;
use Spatie\TemporaryDirectory\TemporaryDirectory;

class FileManipulator extends SpatieFileManipulator
{
    public function performConversions(ConversionCollection $conversions, Media $media, $onlyIfMissing = false)
    {
        if ($conversions->isEmpty()) {
            return;
        }

        $imageGenerator = $this->determineImageGenerator($media);

        if (!$imageGenerator) {
            return;
        }

        $temporaryDirectory = new TemporaryDirectory($this->getTmpPathByMedia($media));

        try {
            $copiedOriginalFile = app(Filesystem::class)->copyFromMediaLibrary(
                $media,
                $temporaryDirectory->path('original.'.$media->extension)
            );

            $conversions
            ->reject(function (Conversion $conversion) use ($onlyIfMissing, $media) {
                return $onlyIfMissing && file_exists($media->getPath($conversion->getName()));
            })
            ->each(function (Conversion $conversion) use ($media, $imageGenerator, $copiedOriginalFile) {
                event(new ConversionWillStart($media, $conversion));

                $copiedOriginalFile = $imageGenerator->convert($copiedOriginalFile, $conversion);

                $conversionResult = $this->performConversion($media, $conversion, $copiedOriginalFile);

                $newFileName = $conversion->getName().'.'.$conversion->getResultExtension(pathinfo($copiedOriginalFile, PATHINFO_EXTENSION));

                $renamedFile = MediaLibraryFileHelper::renameInDirectory($conversionResult, $newFileName);

                app(Filesystem::class)->copyToMediaLibrary($renamedFile, $media, true);

                event(new ConversionHasBeenCompleted($media, $conversion));
            });
        } catch (Exception $exception) {
            \Log::warning($exception->getMessage(), ['exception' => $exception]);
        } finally {
            app('files')->deleteDirectory($temporaryDirectory->path());
        }
    }

    protected function getTmpPathByMedia(Media $media): string
    {
        $path = is_null(config('medialibrary.temporary_directory_path'))
            ? storage_path('medialibrary/temp')
            : config('medialibrary.temporary_directory_path');

        return implode(DIRECTORY_SEPARATOR, [
            $path,
            'media-'.$media->getKey(),
        ]);
    }
}

[freekmurze]

I can't seem to reproduce this. Could it be that your issues are caused by permission settings on your servers?

[Gummibeer]

@freekmurze no, cause by using the flysystem one it's no problem for php to delete and my storage is owned by the PHP process user and has 0775.

[brendt]

I have a fix PR open for this: https://github.com/spatie/image/pull/45

[Gummibeer]

@brendt even if this is good cause it prevents an exception this doesn't solve the real problem and just produces the next one: flooded storage directory.
And like I said, by using the flysystem deleteDirectory() it works like a charm.
And this is also something I don't understand, the flysystem is already used by this package - so why create an already available method again? 🤔

[brendt]

@Gummibeer thanks for the input. I suggest we move this discussion to the PR: https://github.com/spatie/image/pull/45

[Gummibeer]

@brendt like I said it fixes the problem of an Exception rmdir(/tmp): Permission denied. So I don't want to say that this shouldn't get into the code. But it doesn't solve my issue with the temp directory configured and used in this package.
Here I like my solution with finally and using the, existing, filestystem deleteDirectory method - cause with finally it will remove the directory even if during the conversion process an exception were thrown.

[ammonkc]

After much digging I've found the cause of this issue. I've already posted this here in
spatie/image#38, but I'll post it here as well since it's relative.

It's related to something called "systemd /tmp isolation". The php service (and others like httpd) have the PrivateTmp option to true in the systemd script (/usr/lib/systemd/system).

It's a security measure where a new /tmp is created and isolated from the other ones. All the data saved inside are deleted once the service is stopped.

If running on a Linux system where systemd has PrivateTmp=true (which is the default on CentOS 7 and perhaps other newer distros), sys_get_temp_dir() function will simply return "/tmp", not the true, much longer, somewhat dynamic path.
(example: systemd-private-5049768e046a4449b2d8dd552949bb17-php-fpm.service-FYJIWf/tmp/ )
see: http://php.net/manual/en/function.sys-get-temp-dir.php#121073

as a quick fix you can disable the PrivateTmp option. see https://gryzli.info/2015/06/21/centos-7-missing-phpapache-temporary-files-in-tmp-systemd-private-temp/.
But since this is a security feature of linux, a better long term solution would be to avoid using the system /tmp directory to cache conversion files, and instead use a directory within the project.

[brendt]

This bug is fixed in spatie/image ^1.5.1: https://github.com/spatie/image/releases/tag/1.5.1. More info on how we fixed it: https://github.com/spatie/image/issues/38

If you're using the latest medialibrary version, you can run composer up to get the bugfix.

[crslp]

I think that this still is an issue on certain systems.
In my shared environment I encounter the issue that /tmp is not readable despite beeing set to 777.
To quote my hosting provider: "You can write to this directory but not read from it. It is possible that files from other customers are present."
This causes a problem: The fix in spatie/image#38 just checks if it is writable. Somehow, I think, the configuration on my server causes this implementation to break.
Also, setting a path in config/medialibrary.php _temporary_directory_path_ does not seem to have any effect. What's the cause for that?

[brendt]

We're unable to test on all systems. If you can come up with a PR that fixes it for you, and also keeps all tests working, we could add a fix.

[graker]

Got into this error today. Solved it temporarily by adding a subdir to sys_get_temp_dir() call, so conversions would be saved in my tmp/conversions .
But, is it really necessary to take the whole tmp/ dir and try and remove it? Is there a reason not to keep it in a subdir? What if some other process is using tmp for something at the same time?

Also, setting a path in config/medialibrary.php temporary_directory_path does not seem to have any effect.

It won't have effect because the conversion picks tmp directory from system settings with sys_get_temp_dir(), not from the config (like I believe it should be).

[brendt]

@graker Unfortunately we're not able to further look into this. For most of our users, the temp_dir problem isn't an issue, it's an edge case for _some_ people. I hope that your workaround works good enough for you, since we're unable to spend more time on this particular issue.