diff --git a/src/Bootstrappers/FilesystemTenancyBootstrapper.php b/src/Bootstrappers/FilesystemTenancyBootstrapper.php index 05d480e4..57f98114 100644 --- a/src/Bootstrappers/FilesystemTenancyBootstrapper.php +++ b/src/Bootstrappers/FilesystemTenancyBootstrapper.php @@ -128,9 +128,9 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper $scopedDisks = []; foreach ($this->app['config']['filesystems.disks'] as $name => $disk) { - if (isset($disk['driver'], $disk['disk']) + if (isset($disk['driver']) && $disk['driver'] === 'scoped' - && in_array($disk['disk'], $tenantDisks, true)) { + && in_array(static::baseDiskName($name), $tenantDisks, true)) { $scopedDisks[] = $name; } } @@ -340,4 +340,34 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper return $bootstrapper->tenantStoragePath($bootstrapper->suffix($tenant)); } + + /** + * Name of the disk whose root the passed disk uses. + * + * Disks using the 'scoped' driver have no root or url of their own -- they inherit these from their parent disk, + * which can be scoped as well, so only the final/base parent has to be tenant-aware. + * + * Returns null if the chain doesn't end with a named disk, i.e. when a parent disk is + * configured inline or when the disks reference each other. + */ + public static function baseDiskName(string $disk): string|null + { + // Keep track of visited disks to avoid infinite loops in case of disks referencing each other + $visited = []; + + while (config("filesystems.disks.$disk.driver") === 'scoped') { + if (in_array($disk, $visited, true)) { + return null; + } + + $visited[] = $disk; + + if (! is_string($disk = config("filesystems.disks.$disk.disk"))) { + // Laravel allows configuring the parent disk inline as an array, and such a disk has no name + return null; + } + } + + return $disk; + } } diff --git a/src/Controllers/TenantAssetController.php b/src/Controllers/TenantAssetController.php index 8ba466bd..20ff6f99 100644 --- a/src/Controllers/TenantAssetController.php +++ b/src/Controllers/TenantAssetController.php @@ -102,7 +102,11 @@ class TenantAssetController implements HasMiddleware throw new Exception('Disk [' . static::$publicDisk . '] is not a local disk. Only local disks can be used for serving assets.'); } - $baseDiskName = $this->baseDiskName(static::$publicDisk); + $baseDiskName = FilesystemTenancyBootstrapper::baseDiskName(static::$publicDisk); + + if ($baseDiskName === null) { + throw new Exception('Disk [' . static::$publicDisk . '] has an unnamed parent disk. Use a named parent disk listed in tenancy.filesystem.disks.'); + } if (! in_array($baseDiskName, config('tenancy.filesystem.disks'), true)) { // FilesystemTenancyBootstrapper only scopes the roots of disks listed in tenancy.filesystem.disks. @@ -123,26 +127,6 @@ class TenantAssetController implements HasMiddleware return storage_path('app/public'); } - /** - * Name of the disk whose root the passed disk uses. - * - * Disks using the 'scoped' driver have no root of their own -- they inherit the root of their parent disk, - * which can be scoped as well, so the final/base parent is what has to be tenant-aware. - */ - protected function baseDiskName(string $disk): string - { - while (config("filesystems.disks.$disk.driver") === 'scoped') { - if (! is_string($parent = config("filesystems.disks.$disk.disk"))) { - // Laravel allows configuring the parent inline as an array, in which case it has no name - throw new Exception("Disk [$disk] has its parent disk configured inline. Use a named parent disk listed in tenancy.filesystem.disks."); - } - - $disk = $parent; - } - - return $disk; - } - /** * Prevent path traversal attacks. This is generally a non-issue on modern * webservers but it's still worth handling on the application level as well. diff --git a/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php b/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php index 1aa422be..cf4e56ae 100644 --- a/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php +++ b/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php @@ -356,6 +356,31 @@ test('adding a scoped disk to tenancy.filesystem.disks has no effect on the disk ]); }); +test('scoped disks referencing each other do not make bootstrapper hang', function () { + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + 'filesystems.disks.foo' => [ + 'driver' => 'scoped', + 'disk' => 'bar', + 'prefix' => 'foo', + ], + 'filesystems.disks.bar' => [ + 'driver' => 'scoped', + 'disk' => 'foo', + 'prefix' => 'bar', + ], + ]); + + expect(FilesystemTenancyBootstrapper::baseDiskName('foo'))->toBeNull(); + expect(FilesystemTenancyBootstrapper::baseDiskName('bar'))->toBeNull(); + + tenancy()->initialize(Tenant::create()); + + expect(tenant())->not()->toBeNull(); +}); + test('file cache stores get their paths scoped on bootstrap and restored back on revert', function () { $fooPath = storage_path('framework/cache/foo_file'); $barPath = storage_path('framework/cache/bar_file'); diff --git a/tests/TenantAssetTest.php b/tests/TenantAssetTest.php index e1c0b630..ee3ef911 100644 --- a/tests/TenantAssetTest.php +++ b/tests/TenantAssetTest.php @@ -199,7 +199,7 @@ test('tenant asset controller throws when the disk used for serving assets is no })->with([ 'disk' => ['media', 'Disk [media] is not tenant-aware.'], 'scoped disk' => ['scoped_media', 'Disk [media] is not tenant-aware.'], - 'scoped disk with an inline parent disk' => ['inline_scoped_media', 'Disk [inline_scoped_media] has its parent disk configured inline.'], + 'scoped disk with an inline parent disk' => ['inline_scoped_media', 'Disk [inline_scoped_media] has an unnamed parent disk.'], ]); test('tenant assets are served from the resolved root of a scoped disk', function () {