Directory permissions of implicitly created directories not set correctly for local adapter

[nagmat84]

Bug Report

Q	A
Flysystem Version	3.10.2
Adapter Name	LocalFilesystemAdapter
Adapter version	3.10.2

Summary

Several methods (e.g. writeStream) implicitly and recursively create the parent directories, if they don't exist yet. Internally, this is ensured by calling ensureRootDirectoryExists. This method creates the missing directories by calling mkdir($location, $permissions, true). While $permissions is set according the the configuration file, this method does not take into account that $permissions is ANDed with the current umask value, see https://www.php.net/manual/en/function.umask.php.

IMHO, the method should either explicitly set permissions after the directories have been created or temporarily set the umask value to 07777 and restore the previous umask value afterwards.

Moreover, the method createDirectory seems odd, too. If it is called for an existing directory, then the method explicitly sets the permissions via setPermissions and returns. If the directory does not yet exist, the methods creates the directory, but does not set its permissions. Effectively, this means one has to call the method createDirectoy twice in order to ensure correct permissions: the first time to create the directory, the second time to set its permissions.

[frankdejonge]

This behaviour is the same for all adapter, implicit directory creation is only done when the directory does not exist and permission are used for missing directories. As for the umask, this is something that cannot be standardised as the function is not safe for multithreaded environments.

[frankdejonge]

If you require a different umask, you can either do so by decorating the adapter, or setting the umask in a always loaded php file.

[nagmat84]

As for the umask, this is something that cannot be standardised as the function is not safe for multithreaded environments.

I totally agree. There is also a warning in the PHP doc which states that usmask should not be used, but that chmod should be used explicitly after the file or directory has been created.

This behaviour is the same for all adapter, implicit directory creation is only done when the directory does not exist and permission are used for missing directories.

This is the point at which I would like to object, in particular the last part of the sentence which states that "permission are used for missing directories". IMHO this is not what happens, at least not consistently across the library.

A couple of functions internally call setPermissions which implements the chmod approach mentioned above, these methods are createDirectory, setVisibility, writeToFile. This means if one calls one of those functions, the resulting effective permissions are exactly the permissions which have been passed to those methods or are configured.

Another bunch of functions do not call setPermissions and hence, they are affected by the current umask value, namely ensureDirectoryExists and those methods which rely on ensureDirectoryExist, namely createDirectory (upon the first call) and writeToFile for all intermediate directories. This means if one calls one of those functions, the resulting effective permissions are the permissions which have been passed to those methods ANDed with the current umask value.

Surely, one could make the design decision that passed permissions are subject to the umask value. But IMHO this design decision should be made consistently such that the library always exhibits the same external behavior. Currently, the outcome of createDirectory depends on whether the parent directories already exist or not. (Same holds for writeToFile). This makes the result of this methods somewhat unpredictable (unless umask equals zero of course).

[odan]

I have the same (or a similar) issue with the LocalFilesystemAdapter, also in v2.

The problem is that setVisibility() will only be called if the Filesystem instance Config key visibility is set

• https://github.com/thephpleague/flysystem/blob/712478fdaab4ad79b14e01d3f378208555f33e56/src/Local/LocalFilesystemAdapter.php#L123-L124

or when you pass the Config option as 3. Parameter to the write method.

• https://github.com/thephpleague/flysystem/blob/712478fdaab4ad79b14e01d3f378208555f33e56/src/Local/LocalFilesystemAdapter.php#L96

To solve this, you may try to create a Filesystem with an extra option:

// The internal adapter
$adapter = new \League\Flysystem\Local\LocalFilesystemAdapter(
    __DIR__.'/root/directory/', 
    PortableVisibilityConverter::fromArray([...]),
);

// Extra
$filesystemConfig = [
    'visibility' => \League\Flysystem\Visibility::PUBLIC,
];

// The FilesystemOperator
$filesystem = new \League\Flysystem\Filesystem($adapter, $filesystemConfig);

[escopecz]

It actually worked in v1 but disappeared in v3: https://github.com/thephpleague/flysystem/blob/1.x/src/Adapter/Local.php#L383

I did not notice any BC break announcement in changelog.md nor in the GH releases section.