From: Alexander Ebert <ebert@woltlab.com>
Date: Thu, 28 Dec 2023 16:01:54 +0000 (+0100)
Subject: Use a single source of truth for temporary filenames
X-Git-Url: https://git.stricted.de/?p=GitHub%2FWoltLab%2FWCF.git;a=commitdiff_plain;h=4767af20f9cbc9e803534b2540ff54bcbd10f91f

Use a single source of truth for temporary filenames
---

diff --git a/wcfsetup/install/files/lib/action/FileUploadAction.class.php b/wcfsetup/install/files/lib/action/FileUploadAction.class.php
index 9c5e34d779..81edeed8df 100644
--- a/wcfsetup/install/files/lib/action/FileUploadAction.class.php
+++ b/wcfsetup/install/files/lib/action/FileUploadAction.class.php
@@ -14,6 +14,12 @@ use wcf\system\io\File;
 
 final class FileUploadAction implements RequestHandlerInterface
 {
+    /**
+     * Read data in chunks to avoid hitting the memory limit.
+     * See https://stackoverflow.com/a/61997147
+     */
+    private const FREAD_BUFFER_SIZE = 10 * 1_024 * 1_024;
+
     public function handle(ServerRequestInterface $request): ResponseInterface
     {
         // TODO: `sequenceNo` should be of type `non-negative-int`, but requires Valinor 1.7+
@@ -43,10 +49,9 @@ final class FileUploadAction implements RequestHandlerInterface
 
         // Check if the checksum matches the received data.
         $ctx = \hash_init('sha256');
-        $bufferSize = 1 * 1024 * 1024;
         $stream = $request->getBody();
         while (!$stream->eof()) {
-            \hash_update($ctx, $stream->read($bufferSize));
+            \hash_update($ctx, $stream->read(self::FREAD_BUFFER_SIZE));
         }
         $result = \hash_final($ctx);
         $stream->rewind();
@@ -68,19 +73,12 @@ final class FileUploadAction implements RequestHandlerInterface
             \mkdir($tmpPath, recursive: true);
         }
 
-        $filename = \sprintf(
-            '%s-%d.bin',
-            $fileTemporary->identifier,
-            $parameters['sequenceNo'],
-        );
-
         // Write the chunk using a buffer to avoid blowing up the memory limit.
         // See https://stackoverflow.com/a/61997147
-        $result = new AtomicWriter($tmpPath . $filename);
-        $bufferSize = 1 * 1024 * 1024;
+        $result = new AtomicWriter($tmpPath . $fileTemporary->getChunkFilename($parameters['sequenceNo']));
 
         while (!$stream->eof()) {
-            $result->write($stream->read($bufferSize));
+            $result->write($stream->read(self::FREAD_BUFFER_SIZE));
         }
 
         $result->flush();
@@ -88,14 +86,10 @@ final class FileUploadAction implements RequestHandlerInterface
         // Check if we have all chunks.
         $data = [];
         for ($i = 0; $i < $numberOfChunks; $i++) {
-            $filename = \sprintf(
-                '%s-%d.bin',
-                $fileTemporary->identifier,
-                $i,
-            );
-
-            if (\file_exists($tmpPath . $filename)) {
-                $data[] = $tmpPath . $filename;
+            $chunkFilename = $fileTemporary->getChunkFilename($i);
+
+            if (\file_exists($tmpPath . $chunkFilename)) {
+                $data[] = $tmpPath . $chunkFilename;
             }
         }
 
@@ -103,15 +97,14 @@ final class FileUploadAction implements RequestHandlerInterface
             // Concatenate the files by reading only a limited buffer at a time
             // to avoid blowing up the memory limit.
             // See https://stackoverflow.com/a/61997147
-            $bufferSize = 1 * 1024 * 1024;
 
-            $newFilename = \sprintf('%s-final.bin', $fileTemporary->identifier);
-            $result = new AtomicWriter($tmpPath . $newFilename);
+            $resultFilename = $fileTemporary->getResultFilename();
+            $result = new AtomicWriter($tmpPath . $resultFilename);
             foreach ($data as $fileChunk) {
                 $source = new File($fileChunk, 'rb');
                 try {
                     while (!$source->eof()) {
-                        $result->write($source->read($bufferSize));
+                        $result->write($source->read(self::FREAD_BUFFER_SIZE));
                     }
                 } finally {
                     $source->close();
@@ -121,7 +114,7 @@ final class FileUploadAction implements RequestHandlerInterface
             $result->flush();
 
             // Check if the final result matches the expected checksum.
-            $checksum = \hash_file('sha256', $tmpPath . $newFilename);
+            $checksum = \hash_file('sha256', $tmpPath . $resultFilename);
             if ($checksum !== $fileTemporary->fileHash) {
                 // TODO: Proper error message
                 throw new IllegalLinkException();
diff --git a/wcfsetup/install/files/lib/action/FileUploadPreflightAction.class.php b/wcfsetup/install/files/lib/action/FileUploadPreflightAction.class.php
index 727fe5d2c1..7ab5871e3c 100644
--- a/wcfsetup/install/files/lib/action/FileUploadPreflightAction.class.php
+++ b/wcfsetup/install/files/lib/action/FileUploadPreflightAction.class.php
@@ -52,11 +52,11 @@ final class FileUploadPreflightAction implements RequestHandlerInterface
 
         $action = new FileTemporaryAction([], 'create', [
             'data' => [
-                'identifier'=>$identifier,
-                'time'=>\TIME_NOW,
-                'filename'=>$parameters['filename'],
-                'fileSize'=>$parameters['fileSize'],
-                'fileHash'=>$parameters['fileHash'],
+                'identifier' => $identifier,
+                'time' => \TIME_NOW,
+                'filename' => $parameters['filename'],
+                'fileSize' => $parameters['fileSize'],
+                'fileHash' => $parameters['fileHash'],
             ],
         ]);
 
diff --git a/wcfsetup/install/files/lib/data/file/temporary/FileTemporary.class.php b/wcfsetup/install/files/lib/data/file/temporary/FileTemporary.class.php
index c9fd98ee36..bf87bda872 100644
--- a/wcfsetup/install/files/lib/data/file/temporary/FileTemporary.class.php
+++ b/wcfsetup/install/files/lib/data/file/temporary/FileTemporary.class.php
@@ -27,6 +27,20 @@ class FileTemporary extends DatabaseObject
         return \ceil($this->fileSize / $this->getOptimalChunkSize());
     }
 
+    public function getChunkFilename(int $sequenceNo): string
+    {
+        return \sprintf(
+            "%s-%d.bin",
+            $this->identifier,
+            $sequenceNo,
+        );
+    }
+
+    public function getResultFilename(): string
+    {
+        return \sprintf("%s-final.bin", $this->identifier);
+    }
+
     private function getOptimalChunkSize(): int
     {
         $postMaxSize = \ini_parse_quantity(\ini_get('post_max_size'));