Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve file handling #1309

Merged
merged 25 commits into from
May 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
31e907a
Moved methods from trait `Constants` to something better
nagmat84 Apr 21, 2022
bb655d6
A lot of refactoring.
nagmat84 Apr 23, 2022
1d8041f
MOved methods to `Photo`, removed `'raw'`, fixed some exception errors.
nagmat84 Apr 24, 2022
12efa17
Removed "file kind"
nagmat84 Apr 24, 2022
25fb7e8
Removed duplicate MIME type handling.
nagmat84 Apr 24, 2022
abab2b1
Avoid that getExifType throws an exception
nagmat84 Apr 24, 2022
96b2014
Fixed putIntoFinalDestination
nagmat84 Apr 24, 2022
82ef088
Removed some unproblematic references to paths.
nagmat84 Apr 24, 2022
d8695dc
Mmoved low-level methods for file access to file classes.
nagmat84 Apr 24, 2022
af89496
Fixed some silly bugs
nagmat84 Apr 24, 2022
b47c85a
Removed more absolute path and filesize invocations.
nagmat84 Apr 24, 2022
aa257d4
Make formatter happy.
nagmat84 Apr 25, 2022
184ce3f
Added `DownloadedFile`.
nagmat84 Apr 30, 2022
d8fae9f
Use safe variants of fopen/fclose/etc.
nagmat84 Apr 30, 2022
42f3351
Added __clone and __destruct to file classes.
nagmat84 Apr 30, 2022
5a78f7c
Added `getDisk` to FlysystemFile
nagmat84 May 7, 2022
d913b91
`Extractor` uses proper attributes
nagmat84 May 6, 2022
27f7639
Removed filesize from `Extractor`
nagmat84 May 6, 2022
f87300e
Remove `checksum` from `Extractor`
nagmat84 May 7, 2022
bd0fa1c
Removed filesize from ArchiveFileInfo again
nagmat84 May 7, 2022
7ed77c6
Merge branch 'master' into improve_file_handling
nagmat84 May 9, 2022
f18bf9d
Rmoved unnessary property from Import\Exec
nagmat84 May 9, 2022
4f7a9be
Changes due to review by @querty287.
nagmat84 May 9, 2022
1e99f73
Merge remote-tracking branch 'origin/master' into improve_file_handling
nagmat84 May 9, 2022
bffb452
Added move operation to File classes
nagmat84 May 9, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions app/Actions/Album/Archive.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
use App\Exceptions\Handler;
use App\Exceptions\Internal\FrameworkException;
use App\Facades\AccessControl;
use App\Facades\Helpers;
use App\Models\Album;
use App\Models\Configs;
use App\Models\Extensions\BaseAlbum;
Expand Down Expand Up @@ -176,16 +175,16 @@ private function compressAlbum(AbstractAlbum $album, array &$usedDirNames, ?stri
continue;
}

$fullPath = $photo->size_variants->getOriginal()->full_path;
$file = $photo->size_variants->getOriginal()->getFile();

// Set title for photo
$extension = Helpers::getExtension($fullPath, false);
// Generate name for file inside the ZIP archive
$fileBaseName = $this->makeUnique(self::createValidTitle($photo->title), $usedFileNames);
$fileName = $fullNameOfDirectory . '/' . $fileBaseName . $extension;
$fileName = $fullNameOfDirectory . '/' . $fileBaseName . $file->getExtension();

// Reset the execution timeout for every iteration.
set_time_limit(ini_get('max_execution_time'));
$zip->addFileFromPath($fileName, $fullPath);
$zip->addFileFromStream($fileName, $file->read());
$file->close();
} catch (\Throwable $e) {
Handler::reportSafely($e);
}
Expand Down
27 changes: 1 addition & 26 deletions app/Actions/Import/Exec.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

use App\Actions\Album\Create as AlbumCreate;
use App\Actions\Photo\Create as PhotoCreate;
use App\Actions\Photo\Extensions\Constants;
use App\Actions\Photo\Extensions\SourceFileInfo;
use App\Actions\Photo\Strategies\ImportMode;
use App\DTO\ImportEventReport;
use App\DTO\ImportProgressReport;
Expand All @@ -15,12 +13,9 @@
use App\Exceptions\ImportCancelledException;
use App\Exceptions\Internal\FrameworkException;
use App\Exceptions\InvalidDirectoryException;
use App\Exceptions\MediaFileUnsupportedException;
use App\Exceptions\ReservedDirectoryException;
use App\Facades\Helpers;
use App\Image\NativeLocalFile;
use App\Models\Album;
use App\Models\Configs;
use Illuminate\Contracts\Container\BindingResolutionException;
use Illuminate\Database\Eloquent\JsonEncodingException;
use Illuminate\Support\Facades\Session;
Expand All @@ -31,15 +26,12 @@

class Exec
{
use Constants;

protected ImportMode $importMode;
protected PhotoCreate $photoCreate;
protected AlbumCreate $albumCreate;
protected bool $enableCLIFormatting = false;
protected int $memLimit = 0;
protected bool $memWarningGiven = false;
private array $raw_formats;
private bool $firstReportGiven = false;

/**
Expand All @@ -55,7 +47,6 @@ public function __construct(ImportMode $importMode, bool $enableCLIFormatting, i
$this->albumCreate = new AlbumCreate();
$this->enableCLIFormatting = $enableCLIFormatting;
$this->memLimit = $memLimit;
$this->raw_formats = explode('|', strtolower(Configs::get_value('raw_formats', '')));
}

/**
Expand Down Expand Up @@ -275,23 +266,7 @@ public function do(
$filesCount++;

try {
$extension = Helpers::getExtension($file, false);
$is_raw = in_array(strtolower($extension), $this->raw_formats, true);
// TODO: Consolidate all mimetype/extension handling in one place; here we have another test whether the source file is supported which is inconsistent with tests elsewhere
// TODO: Probably the best place is \App\Image\MediaFile.
// TODO: Consider to make this test a general part of \App\Actions\Photo\Create::add. Then we don't need those tests at multiple places.
// Note: `exif_imagetype` may also throw an exception
// (instead of returning `false`), if the file is too small
// to read enough bytes to determine the file type.
// So we put `exif_imagetype` last in the condition and
// exploit lazy evaluation of boolean terms for the case
// that we import a "short" raw file.
if ($is_raw || in_array(strtolower($extension), $this->validExtensions, true) || exif_imagetype($file) !== false) {
$this->photoCreate->add(SourceFileInfo::createByLocalFile(new NativeLocalFile($file)), $parentAlbum);
} else {
// TODO: Separately throwing this particular exception should not be necessary, because `photoCreate->add` should do that; see above
throw new MediaFileUnsupportedException('Unsupported file type');
}
$this->photoCreate->add(new NativeLocalFile($file), $parentAlbum);
} catch (\Throwable $e) {
$this->report(ImportEventReport::createFromException($e, $file));
}
Expand Down
42 changes: 9 additions & 33 deletions app/Actions/Import/FromUrl.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,19 @@

use App\Actions\Import\Extensions\Checks;
use App\Actions\Photo\Create;
use App\Actions\Photo\Extensions\Constants;
use App\Actions\Photo\Extensions\SourceFileInfo;
use App\Actions\Photo\Strategies\ImportMode;
use App\Exceptions\Handler;
use App\Exceptions\InsufficientFilesystemPermissions;
use App\Exceptions\MassImportException;
use App\Exceptions\MediaFileOperationException;
use App\Exceptions\MediaFileUnsupportedException;
use App\Image\TemporaryLocalFile;
use App\Image\DownloadedFile;
use App\Image\MediaFile;
use App\Models\Album;
use App\Models\Configs;
use App\Models\Photo;
use Illuminate\Support\Collection;

class FromUrl
{
use Constants;
use Checks;

/**
Expand Down Expand Up @@ -61,37 +57,17 @@ public function do(array $urls, ?Album $album): Collection
set_time_limit(ini_get('max_execution_time'));

$path = parse_url($url, PHP_URL_PATH);
$basename = pathinfo($path, PATHINFO_FILENAME);
$extension = '.' . pathinfo($path, PATHINFO_EXTENSION);

// Validate photo type and extension even when $this->photo (=> $photo->add) will do the same.
// This prevents us from downloading invalid photos.
// Verify extension
if (!$this->isValidExtension($extension)) {
throw new MediaFileUnsupportedException('Photo format not supported (' . $url . ')');
}
// Validate photo extension even when `$create->add()` will do later.
// This prevents us from downloading unsupported files.
MediaFile::assertIsSupportedOrAcceptedFileExtension($extension);

// Download file, before exif checks the mimetype, otherwise we download it twice
$tmpFile = new TemporaryLocalFile($extension);
try {
$downloadStream = fopen($url, 'r');
$tmpFile->write($downloadStream);
fclose($downloadStream);
} catch (\Exception $e) {
throw new MediaFileOperationException('Could not download ' . $url . ' to ' . $tmpFile->getAbsolutePath(), $e);
}
// Download file
$downloadedFile = new DownloadedFile($url);

// Verify image
// TODO: Consider to make this test a general part of \App\Actions\Photo\Create::add. Then we don't need those tests at multiple places.
$type = exif_imagetype($tmpFile->getAbsolutePath());
if (!$this->isValidImageType($type) && !in_array(strtolower($extension), $this->validExtensions, true)) {
throw new MediaFileUnsupportedException('Photo format not supported (' . $url . ')');
}

// Import photo
$result->add(
$create->add(SourceFileInfo::createByTempFile($basename, $extension, $tmpFile), $album)
);
// Import photo/video/raw
$result->add($create->add($downloadedFile, $album));
} catch (\Throwable $e) {
$exceptions[] = $e;
Handler::reportSafely($e);
Expand Down
90 changes: 58 additions & 32 deletions app/Actions/Photo/Archive.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,22 @@
namespace App\Actions\Photo;

use App\Actions\Photo\Extensions\ArchiveFileInfo;
use App\Actions\Photo\Extensions\Constants;
use App\Contracts\LycheeException;
use App\Exceptions\Internal\FrameworkException;
use App\Exceptions\Internal\InvalidSizeVariantException;
use App\Exceptions\MediaFileMissingException;
use App\Image\FlysystemFile;
use App\Models\Configs;
use App\Models\Photo;
use App\Models\SizeVariant;
use DateTimeInterface;
use Illuminate\Database\Eloquent\Collection;
use Illuminate\Support\Facades\Storage;
use Symfony\Component\HttpFoundation\BinaryFileResponse;
use Symfony\Component\HttpFoundation\File\Exception\FileException;
use Symfony\Component\HttpFoundation\HeaderUtils;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpFoundation\ResponseHeaderBag;
use Symfony\Component\HttpFoundation\StreamedResponse;
use ZipStream\ZipStream;

class Archive
{
use Constants;

public const LIVEPHOTOVIDEO = 'LIVEPHOTOVIDEO';
public const FULL = 'FULL';
public const MEDIUM2X = 'MEDIUM2X';
Expand Down Expand Up @@ -83,11 +77,11 @@ public function __construct()
* {@link Archive::THUMB2X},
* {@link Archive::THUMB}
*
* @return Response
* @return StreamedResponse
*
* @throws LycheeException
*/
public function do(Collection $photos, string $variant): Response
public function do(Collection $photos, string $variant): StreamedResponse
{
if ($photos->count() === 1) {
$response = $this->file($photos->first(), $variant);
Expand All @@ -99,26 +93,62 @@ public function do(Collection $photos, string $variant): Response
}

/**
* @param Photo $photo
* @param $variant
* Streams a single size variant to the client.
*
* Note: This method will become quite inefficient, when the media files
* are not hosted on the same machine as Lychee, but on a remote file
* hosting service like AWS S3.
* In this case, the file would be streamed from the hoster to Lychee
* first and then streamed from Lychee to the client.
* It would be much more efficient, if the client would directly fetch
* the file from the hoster.
* Practically, we could use `->getUrl` of the size variant in combination
* with `Symfony\Component\HttpFoundation\RedirectResponse`.
* However, the client would not get a "nice" file name, but the
* random file name of the size variant.
*
* @return BinaryFileResponse
* @param Photo $photo the photo
* @param string $variant the requested size variant
*
* @return StreamedResponse
*
* @throws LycheeException
*/
protected function file(Photo $photo, $variant): BinaryFileResponse
protected function file(Photo $photo, string $variant): StreamedResponse
{
$archiveFileInfo = $this->extractFileInfo($photo, $variant);

$responseGenerator = function () use ($archiveFileInfo) {
$outputStream = fopen('php://output', 'wb');
stream_copy_to_stream($archiveFileInfo->getFile()->read(), $outputStream);
$archiveFileInfo->getFile()->close();
fclose($outputStream);
};

try {
$response = new BinaryFileResponse($archiveFileInfo->getFullPath());
} catch (\InvalidArgumentException|FileException $e) {
$response = new StreamedResponse($responseGenerator);
$disposition = HeaderUtils::makeDisposition(
HeaderUtils::DISPOSITION_ATTACHMENT,
$archiveFileInfo->getFilename()
);
$response->headers->set('Content-Type', $photo->type);
$response->headers->set('Content-Disposition', $disposition);
$response->headers->set('Content-Length', $archiveFileInfo->getFile()->getFilesize());
// Note: Using insecure hashing algorithm is fine here.
// The ETag header must only be different for different size variants
// Pre-image resistance and collision robustness is not required.
// If a size variant changes, the name of the (physical) file
// changes, too.
// The only reason why we don't use the path directly is that
// we must avoid illegal characters like `/` and md5 returns a
// hexadecimal string.
$response->headers->set('ETag', md5($archiveFileInfo->getFile()->getAbsolutePath()));
$response->headers->set('Last-Modified', $photo->updated_at->format(DateTimeInterface::RFC7231));

return $response;
} catch (\InvalidArgumentException $e) {
throw new FrameworkException('Symfony\'s response component', $e);
}

return $response->setContentDisposition(
ResponseHeaderBag::DISPOSITION_ATTACHMENT,
$archiveFileInfo->getFilename()
);
}

/**
Expand Down Expand Up @@ -225,7 +255,8 @@ protected function zip(Collection $photos, string $variant): StreamedResponse
);
} while (array_key_exists($filename, $uniqueFilenames));
}
$zip->addFileFromPath($filename, $archiveFileInfo->getFullPath());
$zip->addFileFromStream($filename, $archiveFileInfo->getFile()->read());
$archiveFileInfo->getFile()->close();
// Reset the execution timeout for every iteration.
set_time_limit(ini_get('max_execution_time'));
}
Expand Down Expand Up @@ -273,38 +304,33 @@ protected function zip(Collection $photos, string $variant): StreamedResponse
* @return ArchiveFileInfo the created archive info
*
* @throws InvalidSizeVariantException
* @throws MediaFileMissingException
*/
protected function extractFileInfo(Photo $photo, string $variant): ArchiveFileInfo
{
$baseFilename = str_replace($this->badChars, '', $photo->title) ?: 'Untitled';

if ($variant === self::LIVEPHOTOVIDEO) {
$shortPath = $photo->live_photo_short_path;
$sourceFile = new FlysystemFile(Storage::disk(), $photo->live_photo_short_path);
$baseFilenameAddon = '';
} elseif (array_key_exists($variant, self::VARIANT2VARIANT)) {
$sv = $photo->size_variants->getSizeVariant(self::VARIANT2VARIANT[$variant]);
$shortPath = '';
$baseFilenameAddon = '';
if ($sv) {
$shortPath = $sv->short_path;
$sourceFile = $sv->getFile();
// The filename of the original size variant shall get no
// particular suffix but remain as is.
// All other size variants (i.e. the generated, smaller ones)
// get size information as suffix.
if ($sv->type !== SizeVariant::ORIGINAL) {
$baseFilenameAddon = '-' . $sv->width . 'x' . $sv->height;
}
} else {
throw new InvalidSizeVariantException('Size variant missing');
}
} else {
throw new InvalidSizeVariantException('Invalid type of size variant ' . $variant);
}

// Check if file actually exists
if (empty($shortPath) || !Storage::exists($shortPath)) {
throw new MediaFileMissingException('File is missing: ' . $shortPath . ' (' . $baseFilename . ')');
}

return new ArchiveFileInfo($baseFilename, $baseFilenameAddon, Storage::path($shortPath));
return new ArchiveFileInfo($baseFilename, $baseFilenameAddon, $sourceFile);
}
}
Loading