[MEDIA] File downloader now in PHP, added proper name in the UI and changed the format for new attachment file names

The file downloader was changed from a simple redirect to the file to one
implemented in PHP, which should make it safer, by making it possible disallow
direct access to the file, to prevent executing of atttachments

The filename has a new format:
  bin2hex("{$original_name}")."-{$filehash}"
This format should be respected. Notice the dash, which is important to distinguish it from the previous
format, which was "{$hash}.{$ext}"

This change was made to both make the experience more user friendly, by
providing a readable name for files, as opposed to it's hash. This name is taken
from the upload filename, but, clearly, as this wasn't done before, it's
impossible to have a proper name for older files, so those are displayed as
"untitled.{$ext}".

This new name is displayed in the UI, instead of the previous name.

README.md

@@ -1,4 +1,4 @@
-# GNU social 1.20.x
+# GNU social 1.21.x
 (c) 2010-2019 Free Software Foundation, Inc
 
 This is the README file for GNU social, the free

actions/attachment.php

@@ -41,9 +41,8 @@ if (!defined('GNUSOCIAL')) { exit(1); }
 class AttachmentAction extends ManagedAction
 {
     /**
-     * Attachment object to show
+     * Attachment File object to show
      */
-
     var $attachment = null;
 
     /**

actions/attachment_download.php

@@ -15,6 +15,34 @@ class Attachment_downloadAction extends AttachmentAction
 {
     public function showPage()
     {
-        common_redirect($this->attachment->getUrl(), 302);
+        // Checks file exists or throws FileNotStoredLocallyException
+        $filepath = $this->attachment->getPath();
+
+        $filename = MediaFile::getDisplayName($this->attachment);
+
+        // Disable errors, to not mess with the file contents (suppress errors in case access to this
+        // function is blocked, like in some shared hosts). Automatically reset at the end of the
+        // 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");
+        header("Content-Type: {$this->attachment->mimetype}");
+        header("Content-Disposition: attachment; filename=\"{$filename}\"");
+        header('Expires: 0');
+        header('Content-Transfer-Encoding: binary'); // FIXME? Can this be different?
+        $filesize = $this->attachment->size;
+        // 'if available', it says, so ensure we have it
+        if (empty($filesize)) {
+            $filesize = filesize($this->attachment->filename);
+        }
+        header("Content-Length: {$filesize}");
+        // header('Cache-Control: private, no-transform, no-store, must-revalidate');
+
+        $ret = @readfile($filepath);
+
+        if ($ret === false || $ret !== $filesize) {
+            common_log(LOG_ERR, "The lengths of the file as recorded on the DB (or on disk) for the file " .
+                       "{$filepath}, with id={$this->attachment->id} differ from what was sent to the user.");
+        }
     }
 }

classes/File.php

@@ -746,7 +746,7 @@ class File extends Managed_DataObject
 
     public function getTitle()
     {
-        $title = $this->title ?: $this->filename;
+        $title = $this->title ?: MediaFile::getDisplayName($this);
 
         return $title ?: null;
     }

lib/framework.php

@@ -32,8 +32,8 @@ defined('GNUSOCIAL') || die();
 define('GNUSOCIAL_ENGINE', 'GNU social');
 define('GNUSOCIAL_ENGINE_URL', 'https://www.gnu.org/software/social/');
 
-define('GNUSOCIAL_BASE_VERSION', '1.20.9');
+define('GNUSOCIAL_BASE_VERSION', '1.21.0');
-define('GNUSOCIAL_LIFECYCLE', 'release'); // 'dev', 'alpha[0-9]+', 'beta[0-9]+', 'rc[0-9]+', 'release'
+define('GNUSOCIAL_LIFECYCLE', 'dev'); // 'dev', 'alpha[0-9]+', 'beta[0-9]+', 'rc[0-9]+', 'release'
 
 define('GNUSOCIAL_VERSION', GNUSOCIAL_BASE_VERSION . '-' . GNUSOCIAL_LIFECYCLE);
 

lib/mediafile.php

@@ -28,10 +28,7 @@
  * @link      https://www.gnu.org/software/social/
  */
 
-if (!defined('GNUSOCIAL')) {
+defined('GNUSOCIAL') || die();
-    exit(1);
-}
-
 
 /**
  * Class responsible for abstracting media files
@@ -307,6 +304,11 @@ class MediaFile
      *
      * Tries to set the mimetype correctly, using the most secure method available and rejects the file otherwise.
      * In case the upload is an image, this function returns an new ImageFile (which extends MediaFile)
+     * The filename has a new format:
+     *   bin2hex("{$original_name}.{$ext}")."-{$filehash}"
+     * This format should be respected. Notice the dash, which is important to distinguish it from the previous
+     * format ("{$hash}.{$ext}")
+     *
      * @param string $param
      * @param Profile|null $scoped
      * @return ImageFile|MediaFile
@@ -378,11 +380,28 @@ class MediaFile
             $mimetype = self::getUploadedMimeType($_FILES[$param]['tmp_name'], $_FILES[$param]['name']);
             $media = common_get_mime_media($mimetype);
 
-            $basename = basename($_FILES[$param]['name']);
+            $basename = preg_replace("/\..+$/i", '', basename($_FILES[$param]['name']));
-            $filename = $filehash . '.' . File::guessMimeExtension($mimetype, $basename);
+            $ext = File::guessMimeExtension($mimetype, $basename);
-            $filepath = File::path($filename);
-            $result = move_uploaded_file($_FILES[$param]['tmp_name'], $filepath);
 
+            if ($media === 'image') {
+                // Use -1 for the id to avoid adding this temporary file to the DB
+                $img = new ImageFile(-1, $_FILES[$param]['tmp_name']);
+                // Validate the image by reencoding it. Additionally normalizes old formats to PNG,
+                // keeping JPEG and GIF untouched
+                $outpath = $img->resizeTo($img->filepath);
+                $ext = image_type_to_extension($img->preferredType(), false);
+            }
+
+            // New file name format
+            $original_filename = bin2hex("{$basename}.{$ext}");
+            $filename = "{$original_filename}-{$filehash}";
+            $filepath = File::path($filename);
+
+            if ($media === 'image') {
+                $result = rename($outpath, $filepath);
+            } else {
+                $result = move_uploaded_file($_FILES[$param]['tmp_name'], $filepath);
+            }
             if (!$result) {
                 // TRANS: Client exception thrown when a file upload operation fails because the file could
                 // TRANS: not be moved from the temporary folder to the permanent file location.
@@ -391,15 +410,6 @@ class MediaFile
             }
 
             if ($media === 'image') {
-                // Use -1 for the id to avoid adding this temporary file to the DB
-                $img = new ImageFile(-1, $filepath);
-                // Validate the image by reencoding it. Additionally normalizes old formats to PNG,
-                // keeping JPEG and GIF untouched
-                $outpath = $img->resizeTo($img->filepath);
-                $ext = image_type_to_extension($img->preferredType());
-                $filename = $filehash . $ext;
-                $filepath = File::path($filename);
-                $result = rename($outpath, $filepath);
                 return new ImageFile(null, $filepath);
             }
         }
@@ -612,4 +622,49 @@ class MediaFile
         }
         throw new ClientException($hint);
     }
+
+    /**
+     * Title for a file, to display in the interface (if there's no better title) and
+     * for download filenames
+     * @param $file File object
+     * @returns string
+     */
+    public static function getDisplayName(File $file) : string {
+        // New file name format is "{bin2hex(original_name.ext)}-{$hash}"
+        $ret = preg_match('/^([^\-]+)-.+$/', $file->filename, $matches);
+        // If there was an error in the match, something's wrong with some piece
+        // of code (could be a file with utf8 chars in the name)
+        $user_error_mesg = "Invalid file name ({$file->filename}).";
+        $log_error_msg   = "Invalid file name for File with id={$file->id} " .
+                           "({$file->filename}). Some plugin probably did something wrong.";
+
+        if ($ret === false) {
+            common_log(LOG_ERR, $log_error_msg);
+            throw new ServerException($user_error_msg);
+        } elseif ($ret === 1) {
+            $filename = hex2bin($matches[1]);
+        } else {
+            // The old file name format was "{hash}.{ext}"
+            // This estracts the extension
+            $ret = preg_match('/^[^\.]+\.(.+)$/', $file->filename, $matches);
+            if ($ret !== 1) {
+                common_log(LOG_ERR, $log_error_msg);
+                throw new ServerException($user_error_msg);
+            }
+            $ext = $matches[1];
+            // Previously, there was a blacklisted extension array, which could have an alternative
+            // extension, such as phps, to replace php. We want to turn it back
+            $blacklist = common_config('attachments', 'extblacklist');
+            if (is_array($blacklist)) {
+                foreach ($blacklist as $upload_ext => $safe_ext) {
+                    if ($ext === $safe_ext) {
+                        $ext = $upload_ext;
+                        break;
+                    }
+                }
+            }
+            $filename = "untitled.{$ext}";
+        }
+        return $filename;
+    }
 }