mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-17 17:15:08 +00:00
ignacionelson
57540164fa
Two routes still assumed every file sits on local disk, which stopped
being true the moment external storage was switched on. A share link
answered with X-Accel-Redirect whatever the file's disk said, pointing
nginx at a path it has nothing behind; a public listing built a
thumbnail from Storage::disk('files')->path(), which for an externally
stored file is a path nobody ever wrote. Both fail only for installs
using S3, and only on those two routes, so the same file downloading
correctly from the file manager made the share link look like the
broken thing rather than where the file lives.
Neither is a new rule. FileDownloadController and
FileThumbnailController already did it right, which is the actual
finding: the knowledge was sitting in a private method on one class and
inline in another, so the next caller could not inherit it and did not.
Both are now objects with one job.
StoredFileResponse replaces InlineFileResponse and grows an
attachment() alongside inline(), since the two differ only by
disposition. LocalSourceFile takes a closure rather than returning a
path: the version that returned one also left the caller to unlink it,
and both of those are exactly the mistakes made here.
The regression tests fail against the previous controllers — checked in
both directions rather than assumed.
81 lines
2.3 KiB
PHP
81 lines
2.3 KiB
PHP
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
namespace App\Modules\Files\Thumbnails;
|
|
|
|
use App\Modules\Files\Models\File;
|
|
use Illuminate\Support\Facades\Storage;
|
|
use RuntimeException;
|
|
|
|
/**
|
|
* A real path on this machine for a stored file, so that something which
|
|
* can only work on local bytes — image and video rendering, all of which
|
|
* shells out or hands a path to a C library — can work on any file
|
|
* whatever disk it lives on.
|
|
*
|
|
* A local file is used where it lies. Anything else is stream-copied to a
|
|
* temp file and removed afterwards.
|
|
*
|
|
* The callback shape is the point. This started as a private method on
|
|
* one controller that returned a path and left the caller to unlink it,
|
|
* and the second place that needed it did not call it at all — it passed
|
|
* the *local* disk's path() for a file on external storage, which is a
|
|
* path that does not exist, so every public-listing thumbnail of an
|
|
* externally stored file failed. Handing back a path is an invitation to
|
|
* both of those mistakes; a closure that owns the lifetime is not.
|
|
*/
|
|
class LocalSourceFile
|
|
{
|
|
/**
|
|
* @template TReturn
|
|
*
|
|
* @param callable(string): TReturn $work
|
|
* @return TReturn
|
|
*/
|
|
public function use(File $file, callable $work): mixed
|
|
{
|
|
if ($file->disk === 'files') {
|
|
return $work(Storage::disk('files')->path($file->path));
|
|
}
|
|
|
|
$tempPath = tempnam(sys_get_temp_dir(), 'thumb-src-');
|
|
|
|
if ($tempPath === false) {
|
|
throw new RuntimeException('Could not create a temp file for '.$file->original_name);
|
|
}
|
|
|
|
try {
|
|
$this->copyDown($file, $tempPath);
|
|
|
|
return $work($tempPath);
|
|
} finally {
|
|
@unlink($tempPath);
|
|
}
|
|
}
|
|
|
|
private function copyDown(File $file, string $tempPath): void
|
|
{
|
|
$stream = Storage::disk($file->disk)->readStream($file->path);
|
|
$out = fopen($tempPath, 'wb');
|
|
|
|
if ($stream === null || $out === false) {
|
|
if (is_resource($out)) {
|
|
fclose($out);
|
|
}
|
|
|
|
throw new RuntimeException('Could not read '.$file->original_name.' from its storage disk.');
|
|
}
|
|
|
|
try {
|
|
stream_copy_to_stream($stream, $out);
|
|
} finally {
|
|
fclose($out);
|
|
|
|
if (is_resource($stream)) {
|
|
fclose($stream);
|
|
}
|
|
}
|
|
}
|
|
}
|