[MEDIA] Move AttachmentAction::sendFile to common_send_file

This fixed the wrong content type and status code returned by the Avatar action

actions/attachment.php

@@ -203,35 +203,4 @@ class AttachmentAction extends ManagedAction
         }
     }
 
-    /**
-     * Include $filepath in the response, for viewing and downloading.
-     * If provided, $filesize is used to size the HTTP request,
-     * otherwise it's value is calculated
-     * @throws ServerException
-     */
-    public function sendFile(): void
-    {
-        if (is_string(common_config('site', 'x-static-delivery'))) {
-            $tmp = explode(INSTALLDIR, $this->filepath);
-            $relative_path = end($tmp);
-            common_debug("Using Static Delivery with header: '" .
-                         common_config('site', 'x-static-delivery') . ": {$relative_path}'");
-            header(common_config('site', 'x-static-delivery') . ": {$relative_path}");
-        } else {
-            if (empty($this->filesize)) {
-                $this->filesize = filesize($this->filepath);
-            }
-            header("Content-Length: {$this->filesize}");
-            // header('Cache-Control: private, no-transform, no-store, must-revalidate');
-
-            $ret = @readfile($this->filepath);
-
-            if ($ret === false) {
-                common_log(LOG_ERR, "Couldn't read file at {$this->filepath}.");
-            } elseif ($ret !== $this->filesize) {
-                common_log(LOG_ERR, "The lengths of the file as recorded on the DB (or on disk) for the file " .
-                           "{$this->filepath} differ from what was sent to the user ({$this->filesize} vs {$ret}).");
-            }
-        }
-    }
 }

actions/attachment_download.php

@@ -20,12 +20,6 @@ class Attachment_downloadAction extends AttachmentAction
         // script execution, and we don't want to have any more errors until then, so don't reset it
         @ini_set('display_errors', 0);
 
-        header("Content-Description: File Transfer");
+        common_send_file($this->filepath, $this->mimetype, $this->filename, 'attachment');
-        header("Content-Type: {$this->mimetype}");
-        header("Content-Disposition: attachment; filename=\"{$this->filename}\"");
-        header('Expires: 0');
-        header('Content-Transfer-Encoding: binary'); // FIXME? Can this be different?
-
-        parent::sendFile();
     }
 }

actions/attachment_thumbnail.php

@@ -67,12 +67,6 @@ class Attachment_thumbnailAction extends AttachmentAction
         // script execution, and we don't want to have any more errors until then, so don't reset it
         @ini_set('display_errors', 0);
 
-        header("Content-Description: File Transfer");
+        common_send_file($this->filepath, $this->mimetype, $this->filename, 'inline');
-        header("Content-Type: {$this->mimetype}");
-        header("Content-Disposition: inline; filename=\"{$this->filename}\"");
-        header('Expires: 0');
-        header('Content-Transfer-Encoding: binary');
-
-        parent::sendFile();
     }
 }

actions/attachment_view.php

@@ -32,16 +32,7 @@ class Attachment_viewAction extends AttachmentAction
         // script execution, and we don't want to have any more errors until then, so don't reset it
         @ini_set('display_errors', 0);
 
-        header("Content-Description: File Transfer");
+        $disposition = in_array(common_get_mime_media($this->mimetype), ['image', 'video']) ? 'inline' : 'attachment';
-        header("Content-Type: {$this->mimetype}");
+        common_send_file($this->filepath, $this->mimetype, $this->filename, $disposition);
-        if (in_array(common_get_mime_media($this->mimetype), ['image', 'video'])) {
-            header("Content-Disposition: inline; filename=\"{$this->filename}\"");
-        } else {
-            header("Content-Disposition: attachment; filename=\"{$this->filename}\"");
-        }
-        header('Expires: 0');
-        header('Content-Transfer-Encoding: binary');
-
-        parent::sendFile();
     }
 }

actions/avatar.php

@@ -60,9 +60,13 @@ class AvatarAction extends Action
         if (is_string($srv = common_config('avatar', 'server')) && $srv != '') {
             common_redirect(Avatar::url($this->filename), 302);
         } else {
-            $attachment           = new AttachmentAction(); // kludge...
+            $filepath = common_config('avatar', 'dir') . $this->filename;
-            $attachment->filepath = common_config('avatar', 'dir') . $this->filename;
+            $info = @getimagesize($filepath);
-            $attachment->sendFile();
+            if ($info !== false) {
+                common_send_file($filepath, $info['mime'], $this->filename, 'inline');
+            } else {
+                throw new UnsupportedMediaException(_m("Avatar is not an image."));
+            }
         }
         return true;
     }

lib/util/util.php

@@ -2723,6 +2723,47 @@ function common_get_preferred_php_upload_limit(): int
     );
 }
 
+/**
+ * Include $filepath in the response, for viewing and downloading.
+ *
+ * @throws ServerException
+ */
+function common_send_file(string $filepath, string $mimetype, string $filename, string $disposition = 'inline'): void
+{
+    if (is_string(common_config('site', 'x-static-delivery'))) {
+        $tmp = explode(INSTALLDIR, $filepath);
+        $relative_path = end($tmp);
+        common_debug("Using Static Delivery with header: '" .
+                     common_config('site', 'x-static-delivery') . ": {$relative_path}'");
+        header(common_config('site', 'x-static-delivery') . ": {$relative_path}");
+    } else {
+        header("Content-Description: File Transfer");
+        header("Content-Type: {$mimetype}");
+        header("Content-Disposition: {$disposition}; filename=\"{$filename}\"");
+        header('Expires: 0');
+        header('Content-Transfer-Encoding: binary');
+
+        $filesize = filesize($filepath);
+
+        if (file_exists($filepath)) {
+            http_response_code(200);
+            header("Content-Length: {$filesize}");
+            // header('Cache-Control: private, no-transform, no-store, must-revalidate');
+
+            $ret = @readfile($filepath);
+
+        } elseif ($ret === false) {
+            http_response_code(404);
+            common_log(LOG_ERR, "Couldn't read file at {$filepath}.");
+        } elseif ($ret !== $filesize) {
+            http_response_code(500);
+            common_log(LOG_ERR, "The lengths of the file as recorded on the DB (or on disk) for the file " .
+                       "{$filepath} differ from what was sent to the user ({$filesize} vs {$ret}).");
+        }
+    }
+
+}
+
 function html_sprintf()
 {
     $args = func_get_args();