Skip to content

Commit

Permalink
[Bug][Regression] Fix dir permissions (#1569)
Browse files Browse the repository at this point in the history
* Fix dir permissions

* Remove outdated comment
  • Loading branch information
nagmat84 authored Oct 28, 2022
1 parent b4cbfc4 commit 6c1a304
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 11 deletions.
12 changes: 1 addition & 11 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,6 @@ DB_LOG_SQL=false
# Some rare setups may require directories and files to be world writeable.
# In this case, use "world" here.
# USE WITH PRECAUTIONS: world writeable files and folders may be a SECURITY RISK.
# Note at the time of writing, the Flysystem package v1 has a bug which
# prevents the "world" preset from working.
# This is a temporary problem and will be solved after
# https://github.com/thephpleague/flysystem/pull/1523 will have been merged
# upstream or after Lychee will have migrated to Laravel 9.
# Until then, if you need to use world-writable directories, please edit
# `config/filesystems.php` and re-define the values for
# `disks.images.permissions.(file|dir).public` such that they equal
# `disks.images.permissions.(file|dir).world` and use the preset `public`
# instead.
# LYCHEE_IMAGE_VISIBILITY=public

# folders in which the files will be stored
Expand Down Expand Up @@ -91,4 +81,4 @@ MAIL_FROM_ADDRESS=
TRUSTED_PROXIES=null

# Comma-separated list of class names of diagnostics checks that should be skipped.
#SKIP_DIAGNOSTICS_CHECKS=
#SKIP_DIAGNOSTICS_CHECKS=
21 changes: 21 additions & 0 deletions app/Image/FlysystemFile.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,30 @@ public function write($stream, bool $collectStatistics = false): ?StreamStat
try {
$streamStat = $collectStatistics ? static::appendStatFilter($stream) : null;

// The underlying Flysystem currently behaves inconsistent
// with respect to whether it honors the umask value or not.
// Hence, we explicitly set umask to zero to achieve consistent
// behaviour.
// Setting umask can be removed, after
// https://github.com/thephpleague/flysystem/issues/1584
// has been solved.
// Also consider the warning in
// https://www.php.net/manual/en/function.umask.php
// regarding timing issues:
// "Avoid using this function [...]. It is better to change the
// file permissions with chmod() after creating the file. Using
// umask() can lead to unexpected behavior of concurrently running
// scripts and the webserver itself because they all use the same
// umask."
// However, Lychee cannot use `chmod`, because Flysystem may
// also recursively create missing parent directories and/or
// not be local.
// This problem must be fixed on the library layer.
$umask = \umask(0);
if (!$this->disk->writeStream($this->relativePath, $stream)) {
throw new FlySystemLycheeException('Filesystem::writeStream failed');
}
\umask($umask);

return $streamStat;
} catch (\ErrorException|FilesystemException $e) {
Expand Down
1 change: 1 addition & 0 deletions config/filesystems.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
'root' => env('LYCHEE_UPLOADS', public_path('uploads/')),
'url' => env('LYCHEE_UPLOADS_URL', 'uploads/'),
'visibility' => env('LYCHEE_IMAGE_VISIBILITY', 'public'),
'directory_visibility' => env('LYCHEE_IMAGE_VISIBILITY', 'public'),
'permissions' => [
'file' => [
'world' => 00666,
Expand Down

0 comments on commit 6c1a304

Please sign in to comment.