Merge pull request #414 from mudhoney/fix/sentry/138

Fix out of memory error for download movies
Helioviewer-Project · Sep 24, 2024 · f1e359b · f1e359b
2 parents e4ab435 + 17e106c
commit f1e359b
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 13 deletions.
diff --git 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
@@ -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();
     }
+
+    /**
+     * Test for downloading files in stream fashion
+     * Sentry Error 138
+     * @runInSeparateProcess
+     * @depends testBuildMovie
+     */
+    public function testItShouldDownloadMovies(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');
+    }
+
+
 }