Replace ZipStream by using the native ZipArchive

First try to get this work... ampache#3928
usox · Apr 11, 2024 · 373e3a3 · 373e3a3
1 parent f158134
commit 373e3a3
Show file tree

Hide file tree

Showing 6 changed files with 98 additions and 159 deletions.
diff --git a/composer.json b/composer.json
@@ -48,6 +48,7 @@
         "ext-pdo": "*",
         "ext-simplexml": "*",
         "ext-xml": "*",
+        "ext-zip": "*",
         "adhocore/cli": "^1.6",
         "ampache/ampacheapi-php": "dev-master",
         "cboden/ratchet": "0.4.*",
@@ -59,7 +60,6 @@
         "jwilsson/spotify-web-api-php": "^6",
         "krixon/xbmc-php-rpc": "dev-master",
         "kunalvarma05/dropbox-php-sdk": "^0.4",
-        "maennchen/zipstream-php": "^3.1",
         "mikealmond/musicbrainz": "dev-master",
         "moinax/tvdb": "1.*",
         "nyholm/psr7": "^1.3",

diff --git a/composer.lock b/composer.lock
diff --git a/src/Module/Application/Batch/DefaultAction.php b/src/Module/Application/Batch/DefaultAction.php
@@ -40,40 +40,24 @@
 use Ampache\Module\Util\ObjectTypeToClassNameMapper;
 use Ampache\Module\Util\ZipHandlerInterface;
 use Ampache\Repository\SongRepositoryInterface;
+use Psr\Http\Message\ResponseFactoryInterface;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Log\LoggerInterface;
 
-final class DefaultAction implements ApplicationActionInterface
+final readonly class DefaultAction implements ApplicationActionInterface
 {
     public const REQUEST_KEY = 'default';
 
-    private RequestParserInterface $requestParser;
-
-    private ModelFactoryInterface $modelFactory;
-
-    private LoggerInterface $logger;
-
-    private ZipHandlerInterface $zipHandler;
-
-    private FunctionCheckerInterface $functionChecker;
-
-    private SongRepositoryInterface $songRepository;
-
     public function __construct(
-        RequestParserInterface $requestParser,
-        ModelFactoryInterface $modelFactory,
-        LoggerInterface $logger,
-        ZipHandlerInterface $zipHandler,
-        FunctionCheckerInterface $functionChecker,
-        SongRepositoryInterface $songRepository
+        private RequestParserInterface $requestParser,
+        private ModelFactoryInterface $modelFactory,
+        private LoggerInterface $logger,
+        private ZipHandlerInterface $zipHandler,
+        private FunctionCheckerInterface $functionChecker,
+        private SongRepositoryInterface $songRepository,
+        private ResponseFactoryInterface $responseFactory,
     ) {
-        $this->requestParser   = $requestParser;
-        $this->modelFactory    = $modelFactory;
-        $this->logger          = $logger;
-        $this->zipHandler      = $zipHandler;
-        $this->functionChecker = $functionChecker;
-        $this->songRepository  = $songRepository;
     }
 
     public function run(ServerRequestInterface $request, GuiGatekeeperInterface $gatekeeper): ?ResponseInterface
@@ -191,7 +175,13 @@ public function run(ServerRequestInterface $request, GuiGatekeeperInterface $gat
         $media_files = $this->getMediaFiles($media_ids);
         if (is_array($media_files['0'])) {
             set_memory_limit($media_files['1'] + 32);
-            $this->zipHandler->zip($name, $media_files['0'], $flat_path);
+
+            return $this->zipHandler->zip(
+                $this->responseFactory->createResponse(),
+                $name,
+                $media_files['0'],
+                $flat_path
+            );
         }
 
         return null;

diff --git a/src/Module/Util/ZipHandler.php b/src/Module/Util/ZipHandler.php
@@ -28,22 +28,20 @@
 use Ampache\Config\ConfigContainerInterface;
 use Ampache\Config\ConfigurationKeyEnum;
 use Ampache\Module\System\LegacyLogger;
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\StreamFactoryInterface;
 use Psr\Log\LoggerInterface;
-use ZipStream\Exception;
-use ZipStream\ZipStream;
+use ZipArchive;
 
 final class ZipHandler implements ZipHandlerInterface
 {
-    private ConfigContainerInterface $configContainer;
-
-    private LoggerInterface $logger;
+    private ?string $zipFile = null;
 
     public function __construct(
-        ConfigContainerInterface $configContainer,
-        LoggerInterface $logger
+        private readonly ConfigContainerInterface $configContainer,
+        private readonly StreamFactoryInterface $streamFactory,
+        private readonly LoggerInterface $logger
     ) {
-        $this->configContainer = $configContainer;
-        $this->logger          = $logger;
     }
 
     /**
@@ -65,53 +63,73 @@ public function isZipable(string $object_type): bool
      * @param array $media_files array of full paths to medias to zip create w/ call to get_media_files
      * @param bool $flat_path put the files into a single folder
      */
-    public function zip(string $name, array $media_files, bool $flat_path): void
-    {
+    public function zip(
+        ResponseInterface $response,
+        string $name,
+        array $media_files,
+        bool $flat_path
+    ): ResponseInterface {
         $art      = $this->configContainer->get(ConfigurationKeyEnum::ALBUM_ART_PREFERRED_FILENAME);
         $addart   = $this->configContainer->isFeatureEnabled(ConfigurationKeyEnum::ART_ZIP_ADD);
         $filter   = preg_replace('/[^a-zA-Z0-9. -]/', '', $name);
-        $arc      = new ZipStream(
-            comment: (string) $this->configContainer->get(ConfigurationKeyEnum::FILE_ZIP_COMMENT),
-            outputName: $filter . ".zip"
-        );
+
+        $name = sys_get_temp_dir() . '/' . uniqid('ampache-zip');
+
+        $this->zipFile = $name;
+
+        $arc = new ZipArchive();
+        $arc->open($name, ZipArchive::CREATE);
+        $arc->setArchiveComment((string) $this->configContainer->get(ConfigurationKeyEnum::FILE_ZIP_COMMENT));
+
         $playlist = '';
 
-        foreach ($media_files as $dir => $files) {
+        foreach ($media_files as $files) {
             foreach ($files as $file) {
                 $dirname = ($flat_path)
                     ? $filter
                     : dirname($file);
                 $artpath = $dirname . '/' . $art;
                 $folder  = explode('/', $dirname)[substr_count($dirname, "/")];
                 $playlist .= $folder . "/" . basename($file) . "\n";
-                try {
-                    $arc->addFileFromPath($folder . '/' . basename($file), $file);
-                } catch (Exception $e) {
-                    $this->logger->error(
-                        $e->getMessage(),
-                        [LegacyLogger::CONTEXT_TYPE => __CLASS__]
-                    );
-                }
+
+                $arc->addFile($file, $folder . '/' . basename($file));
             }
             if ($addart === true && !empty($folder) && !empty($artpath)) {
-                try {
-                    $arc->addFileFromPath($folder . '/' . $art, $artpath);
-                } catch (Exception $e) {
-                    $this->logger->error(
-                        $e->getMessage(),
-                        [LegacyLogger::CONTEXT_TYPE => __CLASS__]
-                    );
-                }
+                $arc->addFile($artpath, $folder . '/' . $art, );
             }
         }
         if (!empty($playlist)) {
-            $arc->addFile($filter . ".m3u", $playlist);
+            $arc->addFromString($filter . ".m3u", $playlist);
         }
         $this->logger->debug(
             'Sending Zip ' . $filter,
             [LegacyLogger::CONTEXT_TYPE => __CLASS__]
         );
 
-        $arc->finish();
+        $arc->close();
+
+        // Various different browsers dislike various characters here. Strip them all for safety.
+        $safeOutput = trim(str_replace(['"', "'", '\\', ';', "\n", "\r"], '', $filter . '.zip'));
+
+        // Check if we need to UTF-8 encode the filename
+        $urlencoded = rawurlencode($safeOutput);
+
+        return $response
+            ->withHeader('Content-Type', 'application/zip')
+            ->withHeader('Content-Disposition', sprintf('attachment; filename*=UTF-8\'\'%s', $urlencoded))
+            ->withHeader('Pragma', 'public')
+            ->withHeader('Cache-Control', 'public, must-revalidate')
+            ->withHeader('Content-Transfer-Encoding', 'binary')
+            ->withBody(
+                $this->streamFactory->createStreamFromFile($name)
+            );
+    }
+
+    public function __destruct()
+    {
+        // cleanup the generated file
+        if ($this->zipFile) {
+            @unlink($this->zipFile);
+        }
     }
 }