Skip to content

Commit

Permalink
[IMPROVEMENT] Several missing fixes in Zip/unzip/IRSS/Filesystem
Browse files Browse the repository at this point in the history
  • Loading branch information
chfsx committed Dec 10, 2024
1 parent 0b73370 commit cee291b
Show file tree
Hide file tree
Showing 24 changed files with 876 additions and 496 deletions.
12 changes: 7 additions & 5 deletions Services/FileDelivery/classes/class.ilSecureTokenSrcBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,10 @@

declare(strict_types=1);

use ILIAS\GlobalScreen\Scope\MainMenu\Collector\Renderer\Hasher;
use ILIAS\ResourceStorage\Consumer\InlineSrcBuilder;
use ILIAS\ResourceStorage\Consumer\SrcBuilder;
use ILIAS\ResourceStorage\Flavour\Flavour;
use ILIAS\ResourceStorage\Revision\Revision;
use ILIAS\ResourceStorage\StorageHandler\StorageHandler;
use ILIAS\FileDelivery\Delivery\Disposition;
use ILIAS\FileDelivery\Services;

Expand All @@ -39,14 +37,18 @@ public function __construct(
$this->inline = new InlineSrcBuilder($file_delivery);
}

public function getRevisionURL(Revision $revision, bool $signed = true, float $valid_for_at_least_minutes = 60.0): string
{
public function getRevisionURL(
Revision $revision,
bool $signed = true,
float $valid_for_at_least_minutes = 60.0,
string $filename = null
): string {
// get stream from revision
$stream = $revision->maybeStreamResolver()?->getStream();

return (string) $this->file_delivery->buildTokenURL(
$stream,
$revision->getTitle(),
$filename ?? $revision->getTitle(),
Disposition::INLINE,
$GLOBALS['ilUser']->getId() ?? 0,
(int) (ceil($valid_for_at_least_minutes / 60))
Expand Down
14 changes: 7 additions & 7 deletions src/Filesystem/Stream/Stream.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,11 @@ public function __construct($stream, StreamOptions $options = null)
$this->readable = array_key_exists(
$mode,
self::$accessMap
) && (bool) (self::$accessMap[$mode]&self::MASK_ACCESS_READ);
) && (bool) (self::$accessMap[$mode] & self::MASK_ACCESS_READ);
$this->writeable = array_key_exists(
$mode,
self::$accessMap
) && (bool) (self::$accessMap[$mode]&self::MASK_ACCESS_WRITE);
) && (bool) (self::$accessMap[$mode] & self::MASK_ACCESS_WRITE);
$this->seekable = $meta['seekable'];
$this->uri = $this->getMetadata('uri');
}
Expand Down Expand Up @@ -150,7 +150,7 @@ public function getSize(): ?int
clearstatcache(true, $this->uri);
}

$stats = fstat($this->stream);
$stats = fstat($this->stream) ?: [];
if (array_key_exists('size', $stats)) {
$this->size = $stats['size'];
return $this->size;
Expand All @@ -163,7 +163,7 @@ public function getSize(): ?int
/**
* @inheritDoc
*/
public function tell(): int|bool
public function tell(): int
{
$this->assertStreamAttached();

Expand Down Expand Up @@ -229,7 +229,7 @@ public function isWritable(): bool
/**
* @inheritDoc
*/
public function write($string): int|bool
public function write($string): int
{
$this->assertStreamAttached();

Expand Down Expand Up @@ -259,7 +259,7 @@ public function isReadable(): bool
/**
* @inheritDoc
*/
public function read($length): string|bool
public function read($length): string
{
$this->assertStreamAttached();

Expand All @@ -286,7 +286,7 @@ public function read($length): string|bool
/**
* @inheritDoc
*/
public function getContents(): string|bool
public function getContents(): string
{
$this->assertStreamAttached();

Expand Down
4 changes: 4 additions & 0 deletions src/Filesystem/Util/Archive/Archives.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ public function __construct()

public function zip(array $file_streams, ?ZipOptions $zip_options = null): Zip
{
if (empty($file_streams)) {
$file_streams = [Zip::DOT_EMPTY => Streams::ofString('')];
}

return new Zip(
$this->mergeZipOptions($zip_options),
...$file_streams
Expand Down
22 changes: 11 additions & 11 deletions src/Filesystem/Util/Archive/Unzip.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@

use ILIAS\Filesystem\Stream\FileStream;
use ILIAS\Filesystem\Stream\Streams;
use ILIAS\ResourceStorage\StorageHandler\StorageHandlerFactory;
use ILIAS\ResourceStorage\Resource\StorableResource;
use ILIAS\Filesystem\Util;

/**
Expand Down Expand Up @@ -127,14 +125,18 @@ public function getDirectories(): \Generator
foreach ($this->getPaths() as $path) {
if (substr($path, -1) === self::DS_UNIX || substr($path, -1) === self::DS_WIN) {
$directories[] = $path;
continue;
}
if ((str_contains($path, self::DS_UNIX) || str_contains($path, self::DS_WIN))) {
$directories[] = dirname($path) . self::DIRECTORY_SEPARATOR;
}
}

$directories_with_parents = [];

foreach ($directories as $directory) {
$parent = dirname($directory) . self::DIRECTORY_SEPARATOR;
if ($parent !== self::BASE_DIR . self::DIRECTORY_SEPARATOR && !in_array($parent, $directories)) {
if ($parent !== self::BASE_DIR . self::DIRECTORY_SEPARATOR && !in_array($parent, $directories, true)) {
$directories_with_parents[] = $parent;
}
$directories_with_parents[] = $directory;
Expand Down Expand Up @@ -207,8 +209,8 @@ public function extract(): bool
case ZipDirectoryHandling::ENSURE_SINGLE_TOP_DIR:
// top directory with same name as the ZIP without suffix
$zip_path = $this->stream->getMetadata(self::URI);
$sufix = '.' . pathinfo($zip_path, PATHINFO_EXTENSION);
$top_directory = basename($zip_path, $sufix);
$sufix = '.' . pathinfo((string) $zip_path, PATHINFO_EXTENSION);
$top_directory = basename((string) $zip_path, $sufix);

// first we check if the ZIP contains the top directory
$has_top_directory = true;
Expand All @@ -222,18 +224,16 @@ public function extract(): bool
}
break;
case ZipDirectoryHandling::FLAT_STRUCTURE:
if (!is_dir($destination_path)) {
if (!mkdir($destination_path, 0777, true) && !is_dir($destination_path)) {
throw new \RuntimeException(sprintf('Directory "%s" was not created', $destination_path));
}
if (!is_dir($destination_path) && (!mkdir($destination_path, 0777, true) && !is_dir($destination_path))) {
throw new \RuntimeException(sprintf('Directory "%s" was not created', $destination_path));
}

foreach ($this->getStreams() as $stream) {
$uri = $stream->getMetadata(self::URI);
if (substr($uri, -1) === self::DIRECTORY_SEPARATOR) {
if (substr((string) $uri, -1) === self::DIRECTORY_SEPARATOR) {
continue; // Skip directories
}
$file_name = Util::sanitizeFileName($destination_path . self::DIRECTORY_SEPARATOR . basename($uri));
$file_name = Util::sanitizeFileName($destination_path . self::DIRECTORY_SEPARATOR . basename((string) $uri));
file_put_contents(
$file_name,
$stream->getContents()
Expand Down
47 changes: 23 additions & 24 deletions src/Filesystem/Util/Archive/Zip.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,9 @@

namespace ILIAS\Filesystem\Util\Archive;

use ILIAS\ResourceStorage\Identification\ResourceIdentification;
use ILIAS\ResourceStorage\StorageHandler\StorageHandler;
use ILIAS\ResourceStorage\StorageHandler\StorageHandlerFactory;
use ILIAS\ResourceStorage\Revision\Revision;
use ILIAS\ResourceStorage\Resource\StorableResource;
use ILIAS\Filesystem\Stream\Stream;
use ILIAS\Filesystem\Stream\Streams;
use ILIAS\Filesystem\Stream\FileStream;
use ILIAS\Filesystem\Util\Archive\BaseZip;

/**
* @author Fabian Schmid <[email protected]>
Expand All @@ -36,6 +31,7 @@ class Zip
{
use PathHelper;

public const DOT_EMPTY = '.empty';
private string $zip_output_file = '';
protected \ZipArchive $zip;
private int $iteration_limit;
Expand All @@ -51,9 +47,7 @@ public function __construct(
protected ZipOptions $options,
...$streams
) {
$this->streams = array_filter($streams, function ($stream): bool {
return $stream instanceof FileStream;
});
$this->streams = array_filter($streams, fn($stream): bool => $stream instanceof FileStream);

if ($options->getZipOutputPath() !== null && $options->getZipOutputName() !== null) {
$this->zip_output_file = $this->ensureDirectorySeperator(
Expand All @@ -69,14 +63,10 @@ public function __construct(
}
$system_limit = (int) shell_exec('ulimit -n') ?: 0;

if ($system_limit < 10) { // aka we cannot determine the system limit properly
$this->iteration_limit = 100;
} else {
$this->iteration_limit = min(
$system_limit / 2,
5000
);
}
$this->iteration_limit = $system_limit < 10 ? 100 : min(
$system_limit / 2,
5000
);

$this->zip = new \ZipArchive();
if (!file_exists($this->zip_output_file)) {
Expand Down Expand Up @@ -113,7 +103,7 @@ private function storeZIPtoFilesystem(): void
$this->zip->open($this->zip_output_file);
}
if (is_int($path_inside_zip)) {
$path_inside_zip = basename($path);
$path_inside_zip = basename((string) $path);
}

if ($path === 'php://memory') {
Expand All @@ -138,7 +128,7 @@ private function storeZIPtoFilesystem(): void
}
}

public function get(): \ILIAS\Filesystem\Stream\Stream
public function get(): Stream
{
$this->storeZIPtoFilesystem();

Expand All @@ -155,14 +145,24 @@ public function get(): \ILIAS\Filesystem\Stream\Stream
*/
public function addPath(string $path, ?string $path_inside_zip = null): void
{
$path_inside_zip = $path_inside_zip ?? basename($path);

// create directory if it does not exist
$this->zip->addEmptyDir(rtrim(dirname($path_inside_zip), '/') . '/');

$this->addStream(
Streams::ofResource(fopen($path, 'r')),
$path_inside_zip ?? basename($path)
Streams::ofResource(fopen($path, 'rb')),
$path_inside_zip
);
}

public function addStream(FileStream $stream, string $path_inside_zip): void
{
// we remove the "empty zip file" now if possible
if (count($this->streams) === 1 && isset($this->streams[self::DOT_EMPTY])) {
unset($this->streams[self::DOT_EMPTY]);
}

// we must store the ZIP to e temporary files every 1000 files, otherwise we will get a Too Many Open Files error
$this->streams[$path_inside_zip] = $stream;

Expand Down Expand Up @@ -193,7 +193,6 @@ public function addDirectory(string $directory_to_zip): void
\RecursiveIteratorIterator::SELF_FIRST
);


switch ($this->options->getDirectoryHandling()) {
case ZipDirectoryHandling::KEEP_STRUCTURE:
$pattern = null;
Expand All @@ -205,7 +204,6 @@ public function addDirectory(string $directory_to_zip): void
break;
}


foreach ($files as $file) {
$pathname = $file->getPathname();
$path_inside_zip = str_replace($directory_to_zip . '/', '', $pathname);
Expand All @@ -216,7 +214,8 @@ public function addDirectory(string $directory_to_zip): void
/** @var $file \SplFileInfo */
if ($file->isDir()) {
// add directory to zip if it's empty
if (count(scandir($pathname)) === 2) {
$sub_items = array_filter(scandir($pathname), static fn($d): bool => !str_contains((string) $d, '.DS_Store'));
if (count($sub_items) === 2) {
$this->zip->addEmptyDir($path_inside_zip);
}
continue;
Expand Down
20 changes: 8 additions & 12 deletions src/Filesystem/Util/Convert/ImageConverter.php
Original file line number Diff line number Diff line change
Expand Up @@ -173,17 +173,13 @@ protected function handleFormatAndQuality(): void
self::RESOLUTION,
self::RESOLUTION
);
try {
$this->image->resampleImage(
self::RESOLUTION,
self::RESOLUTION,
\Imagick::FILTER_LANCZOS,
1
);
} catch (\Throwable $t) {
// Cannot resample image, continue without resampling
}

// High density images do not support Resampling, cache to small. we deactivate this
/*$this->image->resampleImage(
self::RESOLUTION,
self::RESOLUTION,
\Imagick::FILTER_LANCZOS,
1
);*/
$quality = $this->output_options->getQuality();

// if $this->output_options->getFormat() is 'keep', we map it to the original format
Expand Down Expand Up @@ -221,7 +217,7 @@ protected function handleFormatAndQuality(): void
$this->image->setImageAlphaChannel(\Imagick::ALPHACHANNEL_ACTIVATE);
}
$this->image->setImageFormat('png');
$this->image->setOption('png:compression-level', (string)$png_compression_level);
$this->image->setOption('png:compression-level', (string) $png_compression_level);
break;
}
$this->image->stripImage();
Expand Down
2 changes: 1 addition & 1 deletion src/ResourceStorage/Consumer/ConsumerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ public function containerURI(
StorableContainerResource $resource,
SrcBuilder $src_builder,
string $start_file = 'index.html',
float $valid_for_at_least_minutes = 60.0
float $valid_for_at_least_minutes = 120.0
): ContainerConsumer {
return new ContainerURIConsumer(
$src_builder,
Expand Down
Loading

0 comments on commit cee291b

Please sign in to comment.