From adfcb800bdda59e2127268756244546baef8175d Mon Sep 17 00:00:00 2001 From: Kasim Necdet Percinel Date: Tue, 24 Sep 2024 13:35:28 +0000 Subject: [PATCH 1/2] move from filegetcontents to readfile for stream movie in memorysafe mode\ --- src/Module/Movies.php | 37 ++++++++++++------- .../movies/HelioviewerMovieTest.php | 32 ++++++++++++++++ 2 files changed, 56 insertions(+), 13 deletions(-) diff --git a/src/Module/Movies.php b/src/Module/Movies.php index afb8a67f..2173a0d3 100644 --- a/src/Module/Movies.php +++ b/src/Module/Movies.php @@ -829,7 +829,7 @@ public function downloadMovie() { if ( $this->_verifyMediaExists($movie, $allowRegeneration=true) ) { - ini_set('memory_limit', '1024M'); + // Default options $defaults = array( "hq" => false @@ -839,27 +839,38 @@ public function downloadMovie() { // Get filepath $filepath = $movie->getFilepath($options['hq']); $filename = basename($filepath); + $filesize = filesize($filepath); + + if (FALSE === $filesize) { + http_response_code(500); + header('Content-type: text/html'); + echo "Server Error"; + Sentry::message("Error Download Movie: we couldn't serve file due to filesize error:".$filepath); + exit; + } // Set HTTP headers header('Pragma: public'); header('Expires: 0'); - header( - 'Cache-Control: must-revalidate, post-check=0, pre-check=0'); + header('Cache-Control: must-revalidate, post-check=0, pre-check=0'); header('Cache-Control: private', false); - header('Content-Disposition: attachment; filename="' . - $filename . '"'); + header('Content-Disposition: attachment; filename="' .$filename . '"'); header('Content-Transfer-Encoding: binary'); - header('Content-Length: ' . @filesize($filepath)); - if($params['format'] == 'gif'){ + header('Content-Length: ' . $filesize); + + if($params['format'] === 'gif'){ header('Content-type: image/gif'); - }else{ - header('Content-type: video/'.$params['format']); + } else{ + header('Content-type: video/'.$params['format']); } - // Return movie data - echo @file_get_contents($filepath); - } - else { + // Serve movie data + if (FALSE === readfile($filepath)) { + Sentry::message("Error Download Movie: we couldn't serve file due to readfile error:".$filepath); + exit; + } + + } else { // Reload movie, since it may have been requeued and the status may // have changed. $movie = new Movie_HelioviewerMovie($params['id'], diff --git a/tests/unit_tests/movies/HelioviewerMovieTest.php b/tests/unit_tests/movies/HelioviewerMovieTest.php index 55df2916..470fd131 100644 --- a/tests/unit_tests/movies/HelioviewerMovieTest.php +++ b/tests/unit_tests/movies/HelioviewerMovieTest.php @@ -11,6 +11,9 @@ include_once HV_ROOT_DIR.'/../src/Database/MovieDatabase.php'; include_once HV_ROOT_DIR.'/../src/Movie/HelioviewerMovie.php'; +// Include to test +include_once HV_ROOT_DIR.'/../src/Module/Movies.php'; + final class HelioviewerMovieTest extends TestCase { const TEST_LAYER = "[SOHO,LASCO,C2,white-light,2,100,0,60,1,2024-07-11T09:03:05.000Z]"; @@ -185,4 +188,33 @@ public function testGetTitle(string $movie_id) { $this->expectExceptionMessage($movie_id); $new_movie->getTitle(); } + + /** + * Sentry Error 138 - https://sentry.helioviewer.org/organizations/helioviewer/issues/138 + * Test for downloading files in stream fashion , instead of read them as a whole + * @runInSeparateProcess + * @depends testBuildMovie + */ + public function testItShouldDownloadMovieWithStreamingEvenWithLessMemoryConditions(string $movie_id) { + + // Check the duration of the processed movie. + $result = new Movie_HelioviewerMovie($movie_id); + + $params = [ + "id" => $result->publicId, + "format" => "mp4", + "hq" => "true", + ]; + + $api = new Module_Movies($params); + ob_start(); + $api->downloadMovie(); + $movie_string = ob_get_contents(); + ob_end_clean(); + + $finfo = new \finfo(FILEINFO_MIME); + $this->assertEquals($finfo->buffer($movie_string), 'video/mp4; charset=binary'); + } + + } From 17e106cd327aeea873282ffafddc491ab06bbddb Mon Sep 17 00:00:00 2001 From: Kasim Necdet Percinel Date: Tue, 24 Sep 2024 14:04:54 +0000 Subject: [PATCH 2/2] fix english wordings and better explanation for the functions --- tests/unit_tests/movies/HelioviewerMovieTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit_tests/movies/HelioviewerMovieTest.php b/tests/unit_tests/movies/HelioviewerMovieTest.php index 470fd131..aa79bb87 100644 --- a/tests/unit_tests/movies/HelioviewerMovieTest.php +++ b/tests/unit_tests/movies/HelioviewerMovieTest.php @@ -190,12 +190,12 @@ public function testGetTitle(string $movie_id) { } /** - * Sentry Error 138 - https://sentry.helioviewer.org/organizations/helioviewer/issues/138 - * Test for downloading files in stream fashion , instead of read them as a whole + * Test for downloading files in stream fashion + * Sentry Error 138 * @runInSeparateProcess * @depends testBuildMovie */ - public function testItShouldDownloadMovieWithStreamingEvenWithLessMemoryConditions(string $movie_id) { + public function testItShouldDownloadMovies(string $movie_id) { // Check the duration of the processed movie. $result = new Movie_HelioviewerMovie($movie_id);