Use a single source of truth for temporary filenames
authorAlexander Ebert <ebert@woltlab.com>
Thu, 28 Dec 2023 16:01:54 +0000 (17:01 +0100)
committerAlexander Ebert <ebert@woltlab.com>
Wed, 27 Mar 2024 22:55:27 +0000 (23:55 +0100)
wcfsetup/install/files/lib/action/FileUploadAction.class.php
wcfsetup/install/files/lib/action/FileUploadPreflightAction.class.php
wcfsetup/install/files/lib/data/file/temporary/FileTemporary.class.php

index 9c5e34d779ea0b03764a51f123dfbfd69ba831f1..81edeed8dfdfbdc19c2376b7267a106fe81525e5 100644 (file)
@@ -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();
index 727fe5d2c1c7c1f5586f98a02f9c0dedf0cc58fd..7ab5871e3c93c58f2bee2c7aa4e75c08f37f8ef6 100644 (file)
@@ -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'],
             ],
         ]);
 
index c9fd98ee360c3be3bb32823474b9b26fa732ded5..bf87bda872a528b7b28f52f4d82f1126261c7d67 100644 (file)
@@ -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'));