diff options
author | Elmer Miroslav Mosher Golovin <miroslav@mishamosher.com> | 2023-04-25 14:44:25 +0300 |
---|---|---|
committer | Elmer Miroslav Mosher Golovin <miroslav@mishamosher.com> | 2023-04-26 16:39:25 +0300 |
commit | 585f1aba26b2c6a1a4fa1548cb3b0cb4e00b5d39 (patch) | |
tree | 3e7b9dd8f6d4f4da36f2582f5ddbd15862681637 | |
parent | 9aed218ee1aead4c79a270fbd284d0dd4246ba91 (diff) |
Avoid out-of-memory errors by using a resource instead of loading entire files into memory
Signed-off-by: Elmer Miroslav Mosher Golovin <miroslav@mishamosher.com>
-rw-r--r-- | lib/Service/RecordingService.php | 46 | ||||
-rw-r--r-- | tests/php/Service/RecordingServiceTest.php | 29 |
2 files changed, 50 insertions, 25 deletions
diff --git a/lib/Service/RecordingService.php b/lib/Service/RecordingService.php index 00dac2e26..a5c2defe4 100644 --- a/lib/Service/RecordingService.php +++ b/lib/Service/RecordingService.php @@ -2,7 +2,8 @@ declare(strict_types=1); /** - * @copyright Copyright (c) 2022 Vitor Mattos <vitor@php.rio> + * @copyright Copyright (c) 2022, Vitor Mattos <vitor@php.rio> + * @copyright Copyright (c) 2023, Elmer Miroslav Mosher Golovin (miroslav@mishamosher.com) * * @author Vitor Mattos <vitor@php.rio> * @@ -127,14 +128,16 @@ class RecordingService { throw new InvalidArgumentException('owner_participant'); } - $content = $this->getContentFromFileArray($file, $room, $participant); + $resource = $this->getResourceFromFileArray($file, $room, $participant); $fileName = basename($file['name']); - $this->validateFileFormat($fileName, $content); + $fileRealPath = realpath($file['tmp_name']); + + $this->validateFileFormat($fileName, $fileRealPath); try { $recordingFolder = $this->getRecordingFolder($owner, $room->getToken()); - $fileNode = $recordingFolder->newFile($fileName, $content); + $fileNode = $recordingFolder->newFile($fileName, $resource); $this->notifyStoredRecording($room, $participant, $fileNode); } catch (NoUserException $e) { throw new InvalidArgumentException('owner_invalid'); @@ -203,7 +206,15 @@ class RecordingService { $this->notificationManager->notify($notification); } - public function getContentFromFileArray(array $file, Room $room, Participant $participant): string { + /** + * Gets a resource that represents the file contents of the file array. + * + * @param array $file File array from which a resource will be returned + * @param Room $room The Talk room that requests the resource + * @param Participant $participant The Talk participant that requests the resource + * @return resource Resource representing the file contents of the file array + */ + public function getResourceFromFileArray(array $file, Room $room, Participant $participant) { if ($file['error'] !== 0) { $error = self::UPLOAD_ERRORS[$file['error']]; $this->logger->error($error); @@ -220,17 +231,30 @@ class RecordingService { throw new InvalidArgumentException('invalid_file'); } - $content = file_get_contents($file['tmp_name']); - unlink($file['tmp_name']); + $resource = fopen($file['tmp_name'], 'r'); + if ($resource === false) { + throw new InvalidArgumentException('fopen_failed'); + } - if (!$content) { + $resourceStat = fstat($resource); + if ($resourceStat === false) { + throw new InvalidArgumentException('fstat_failed'); + } + + if ($resourceStat['size'] === 0) { throw new InvalidArgumentException('empty_file'); } - return $content; + + return $resource; } - public function validateFileFormat(string $fileName, $content): void { - $mimeType = $this->mimeTypeDetector->detectString($content); + public function validateFileFormat(string $fileName, string $fileRealPath): void { + if (!is_file($fileRealPath)) { + $this->logger->warning("An invalid file path ($fileRealPath) was provided"); + throw new InvalidArgumentException('file_invalid_path'); + } + + $mimeType = $this->mimeTypeDetector->detectContent($fileRealPath); $allowed = self::DEFAULT_ALLOWED_RECORDING_FORMATS; if (!array_key_exists($mimeType, $allowed)) { $this->logger->warning("Uploaded file detected mime type ($mimeType) is not allowed"); diff --git a/tests/php/Service/RecordingServiceTest.php b/tests/php/Service/RecordingServiceTest.php index 1ff6fcea6..94e9ae27b 100644 --- a/tests/php/Service/RecordingServiceTest.php +++ b/tests/php/Service/RecordingServiceTest.php @@ -3,6 +3,7 @@ declare(strict_types=1); /** * @copyright Copyright (c) 2022, Vitor Mattos <vitor@php.rio> + * @copyright Copyright (c) 2023, Elmer Miroslav Mosher Golovin (miroslav@mishamosher.com) * * @author Vitor Mattos <vitor@php.rio> * @@ -119,34 +120,35 @@ class RecordingServiceTest extends TestCase { } /** @dataProvider dataValidateFileFormat */ - public function testValidateFileFormat(string $fileName, string $content, string $exceptionMessage):void { + public function testValidateFileFormat(string $fileName, string $fileRealPath, string $exceptionMessage): void { if ($exceptionMessage) { $this->expectExceptionMessage($exceptionMessage); } else { $this->expectNotToPerformAssertions(); } - $this->recordingService->validateFileFormat($fileName, $content); + $this->recordingService->validateFileFormat($fileName, $fileRealPath); } public function dataValidateFileFormat(): array { return [ + # file_invalid_path + ['', '', 'file_invalid_path'], # file_mimetype - ['', '', 'file_mimetype'], - ['', file_get_contents(__DIR__ . '/../../../img/app.svg'), 'file_mimetype'], - ['name.ogg', file_get_contents(__DIR__ . '/../../../img/app.svg'), 'file_mimetype'], + ['', realpath(__DIR__ . '/../../../img/app.svg'), 'file_mimetype'], + ['name.ogg', realpath(__DIR__ . '/../../../img/app.svg'), 'file_mimetype'], # file_extension - ['', file_get_contents(__DIR__ . '/../../../img/join_call.ogg'), 'file_extension'], - ['name', file_get_contents(__DIR__ . '/../../../img/join_call.ogg'), 'file_extension'], - ['name.mp3', file_get_contents(__DIR__ . '/../../../img/join_call.ogg'), 'file_extension'], + ['', realpath(__DIR__ . '/../../../img/join_call.ogg'), 'file_extension'], + ['name', realpath(__DIR__ . '/../../../img/join_call.ogg'), 'file_extension'], + ['name.mp3', realpath(__DIR__ . '/../../../img/join_call.ogg'), 'file_extension'], # Success - ['name.ogg', file_get_contents(__DIR__ . '/../../../img/join_call.ogg'), ''], + ['name.ogg', realpath(__DIR__ . '/../../../img/join_call.ogg'), ''], ]; } /** - * @dataProvider dataGetContentFromFileArray + * @dataProvider dataGetResourceFromFileArray */ - public function testGetContentFromFileArray(array $file, $expected, string $exceptionMessage): void { + public function testGetResourceFromFileArray(array $file, $expected, string $exceptionMessage): void { if ($exceptionMessage) { $this->expectExceptionMessage($exceptionMessage); } @@ -158,12 +160,11 @@ class RecordingServiceTest extends TestCase { ]); $participant = new Participant($room, $attendee, null); - $actual = $this->recordingService->getContentFromFileArray($file, $room, $participant); + $actual = stream_get_contents($this->recordingService->getResourceFromFileArray($file, $room, $participant)); $this->assertEquals($expected, $actual); - $this->assertFileDoesNotExist($file['tmp_name']); } - public function dataGetContentFromFileArray(): array { + public function dataGetResourceFromFileArray(): array { $fileWithContent = tempnam(sys_get_temp_dir(), 'txt'); file_put_contents($fileWithContent, 'bla'); return [ |