[MEDIA][OEMBED] Fixed regression in OEmbed, because it relied on accessing the files directly, which previous commits broke. The File table really should have a bool...

actions/attachment.php

@@ -71,7 +71,11 @@ class AttachmentAction extends ManagedAction
         if (!$this->attachment instanceof File) {
             // TRANS: Client error displayed trying to get a non-existing attachment.
             $this->clientError(_('No such attachment.'), 404);
-        } elseif (empty($this->attachment->filename)) {
+        }
+
+        $filename = $this->attachment->getFileOrThumbnailPath();
+
+        if (empty($filename)) {
             $this->clientError(_('Requested local URL for a file that is not stored locally.'), 404);
         }
         return true;
@@ -100,7 +104,7 @@ class AttachmentAction extends ManagedAction
 
     public function showPage()
     {
-        if (empty($this->attachment->filename)) {
+        if (empty($this->attachment->getFileOrThumbnailPath())) {
             // if it's not a local file, gtfo
             common_redirect($this->attachment->getUrl(), 303);
         }
@@ -150,9 +154,10 @@ class AttachmentAction extends ManagedAction
         if (common_config('site', 'use_x_sendfile')) {
             return null;
         }
-        try {
+        $path = $this->attachment->getFileOrThumbnailPath();
-            return filemtime($this->attachment->getPath());
+        if (!empty($path)) {
-        } catch (InvalidFilenameException $e) {
+            return filemtime($path);
+        } else {
             return null;
         }
     }
@@ -168,26 +173,32 @@ class AttachmentAction extends ManagedAction
     function etag()
     {
         if (common_config('site', 'use_x_sendfile')) {
+
             return null;
         }
 
+        $path = $this->attachment->getFileOrThumbnailPath();
+
         $cache = Cache::instance();
         if($cache) {
-            try {
+            if (empty($path)) {
-                $key = Cache::key('attachments:etag:' . $this->attachment->getPath());
+                return null;
+            }
+            $key = Cache::key('attachments:etag:' . $path);
             $etag = $cache->get($key);
             if($etag === false) {
-                    $etag = crc32(file_get_contents($this->attachment->getPath()));
+                $etag = crc32(file_get_contents($path));
                 $cache->set($key,$etag);
             }
-            } catch (InvalidFilenameException $e) {
-                return null;
-            }
             return $etag;
         }
 
-        $stat = stat($this->path);
+        if (!empty($path)) {
+            $stat = stat($path);
             return '"' . $stat['ino'] . '-' . $stat['size'] . '-' . $stat['mtime'] . '"';
+        } else {
+            return null;
+        }
     }
 
     /**
@@ -205,7 +216,7 @@ class AttachmentAction extends ManagedAction
 
             $ret = @readfile($filepath);
 
-            if (ret === false) {
+            if ($ret === false) {
                 common_log(LOG_ERR, "Couldn't read file at {$filepath}.");
             } elseif ($ret !== $filesize) {
                 common_log(LOG_ERR, "The lengths of the file as recorded on the DB (or on disk) for the file " .

actions/attachment_download.php

@@ -15,9 +15,15 @@ class Attachment_downloadAction extends AttachmentAction
 {
     public function showPage()
     {
-        // Checks file exists or throws FileNotStoredLocallyException
+        // Checks file exists or throws FileNotFoundException
-        $filepath = $this->attachment->getPath();
+        $filepath = $this->attachment->getFileOrThumbnailPath();
-        $filesize = $this->attachment->size;
+        $filesize = $this->attachment->size ?: 0;
+        $mimetype = $this->attachment->getFileOrThumbnailMimetype();
+
+        if (empty($filepath)) {
+            $thiis->clientError(_('No such attachment'), 404);
+        }
+
         $filename = MediaFile::getDisplayName($this->attachment);
 
         // Disable errors, to not mess with the file contents (suppress errors in case access to this
@@ -26,7 +32,7 @@ class Attachment_downloadAction extends AttachmentAction
         @ini_set('display_errors', 0);
 
         header("Content-Description: File Transfer");
-        header("Content-Type: {$this->attachment->mimetype}");
+        header("Content-Type: {$mimetype}");
         header("Content-Disposition: attachment; filename=\"{$filename}\"");
         header('Expires: 0');
         header('Content-Transfer-Encoding: binary'); // FIXME? Can this be different?

actions/attachment_thumbnail.php

@@ -55,31 +55,22 @@ class Attachment_thumbnailAction extends AttachmentAction
 
     public function showPage()
     {
+        // Checks file exists or throws FileNotFoundException
+        $size = $this->attachment->size ?: 0;
+
         // Returns a File_thumbnail object or throws exception if not available
         try {
-            $thumb = $this->attachment->getThumbnail($this->thumb_w, $this->thumb_h, $this->thumb_c);
+            $thumbnail = $this->attachment->getThumbnail($this->thumb_w, $this->thumb_h, $this->thumb_c);
-            $file = $thumb->getFile();
+            $file = $thumbnail->getFile();
         } catch (UseFileAsThumbnailException $e) {
             // With this exception, the file exists locally
             $file = $e->file;
+        } catch(FileNotFoundException $e) {
+            $this->clientError(_('No such attachment'), 404);
         }
 
-        try {
+        $filepath = $file->getFileOrThumbnailPath();
-            if (!empty($thumb)) {
+        $mimetype = $file->getFileOrThumbnailMimetype();
-                $filepath = $thumb->getPath();
-                $size = 0;
-            } elseif ($file->isLocal()) {
-                $filepath = $file->getPath();
-                $size = $file->size;
-            }
-            // XXX PHP: Upgrade to PHP 7.1
-            // FileNotFoundException | InvalidFilenameException
-        } catch (Exception $e) {
-            // We don't have a file to display
-            $this->clientError(_('No such attachment.'), 404);
-            return false;
-        }
-
         $filename = MediaFile::getDisplayName($file);
 
         // Disable errors, to not mess with the file contents (suppress errors in case access to this
@@ -88,7 +79,7 @@ class Attachment_thumbnailAction extends AttachmentAction
         @ini_set('display_errors', 0);
 
         header("Content-Description: File Transfer");
-        header("Content-Type: {$file->mimetype}");
+        header("Content-Type: {$mimetype}");
         header("Content-Disposition: inline; filename=\"{$filename}\"");
         header('Expires: 0');
         header('Content-Transfer-Encoding: binary');

actions/attachment_view.php

@@ -13,9 +13,14 @@ class Attachment_viewAction extends AttachmentAction
 {
     public function showPage()
     {
-        // Checks file exists or throws FileNotStoredLocallyException
+        // Checks file exists or throws FileNotFoundException
-        $filepath = $this->attachment->getPath();
+        $filepath = $this->attachment->getFileOrThumbnailPath();
-        $filesize = $this->attachment->size;
+        $filesize = $this->attachment->size ?: 0;
+        $mimetype = $this->attachment->getFileOrThumbnailMimetype();
+
+        if (empty($filepath)) {
+            $thiis->clientError(_('No such attachment'), 404);
+        }
 
         $filename = MediaFile::getDisplayName($this->attachment);
 
@@ -25,8 +30,8 @@ class Attachment_viewAction extends AttachmentAction
         @ini_set('display_errors', 0);
 
         header("Content-Description: File Transfer");
-        header("Content-Type: {$this->attachment->mimetype}");
+        header("Content-Type: {$mimetype}");
-        if (in_array(common_get_mime_media($this->attachment->mimetype), ['image', 'video'])) {
+        if (in_array(common_get_mime_media($mimetype), ['image', 'video'])) {
             header("Content-Disposition: inline; filename=\"{$filename}\"");
         } else {
             header("Content-Disposition: attachment; filename=\"{$filename}\"");

classes/File.php

@@ -589,6 +589,58 @@ class File extends Managed_DataObject
         return $filepath;
     }
 
+    /**
+     * Returns the path to either a file, or it's thumbnail if the file doesn't exist.
+     * This is useful in case the original file is deleted, or, as is the case for Embed
+     * thumbnails, we only have a thumbnail and not a file
+     * @return string Path
+     * @throws FileNotFoundException
+     * @throws FileNotStoredLocallyException
+     * @throws InvalidFilenameException
+     * @throws ServerException
+     */
+    public function getFileOrThumbnailPath() : string
+    {
+        if (!empty($this->filename)) {
+            $filepath = self::path($this->filename);
+            if (file_exists($filepath)) {
+                return $filepath;
+            } else {
+                throw new FileNotFoundException($filepath);
+            }
+        } else {
+            try {
+                return File_thumbnail::byFile($this)->getPath();
+            } catch (NoResultException $e) {
+                // File not stored locally
+                throw new FileNotStoredLocallyException($this);
+            }
+        }
+    }
+
+    /**
+     * Return the mime type of the thumbnail if we have it, or, if not, of the File
+     * @return string
+     * @throws FileNotFoundException
+     * @throws NoResultException
+     * @throws ServerException
+     * @throws UnsupportedMediaException
+     */
+    public function getFileOrThumbnailMimetype() : string
+    {
+        if (empty($this->filename)) {
+            $filepath = File_thumbnail::byFile($this)->getPath();
+            $info = @getimagesize($filepath);
+            if ($info !== false) {
+                return $info['mime'];
+            } else {
+                throw new UnsupportedMediaException(_("Thumbnail is not an image."));
+            }
+        } else {
+            return $this->mimetype;
+        }
+    }
+
     public function getAttachmentUrl()
     {
         return common_local_url('attachment', array('attachment'=>$this->getID()));

lib/attachmentlist.php

@@ -77,7 +77,7 @@ class AttachmentList extends Widget
     	$attachments = $this->notice->attachments();
         foreach ($attachments as $key=>$att) {
             // Remove attachments which are not representable with neither a title nor thumbnail
-            if ($att->getTitle() === null && !$att->hasThumbnail()) {
+            if ($att->getTitle() === _('Untitled attachment') && !$att->hasThumbnail()) {
                 unset($attachments[$key]);
             }
         }

lib/attachmentlistitem.php

@@ -146,7 +146,9 @@ class AttachmentListItem extends Widget
                     } else {
                         try {
                             // getUrl(true) because we don't want to hotlink, could be made configurable
-                            $this->out->element('img', ['class'=>'u-photo', 'src'=>$this->attachment->getUrl(true), 'alt' => $this->attachment->getTitle()]);
+                            $this->out->element('img', ['class'=>'u-photo',
+                                                        'src'=>$this->attachment->getUrl(true),
+                                                        'alt' => $this->attachment->getTitle()]);
                         } catch (FileNotStoredLocallyException $e) {
                             $url = $e->file->getUrl(false);
                             $this->out->element('a', ['href'=>$url, 'rel'=>'external'], $url);

lib/imagefile.php

@@ -535,10 +535,8 @@ class ImageFile extends MediaFile
             throw new ServerException('No File object attached to this ImageFile object.');
         }
 
-        // File not stored locally, can't generate a thumbnail
+        // Throws FileNotFoundException or FileNotStoredLocallyException
-        if (empty($this->fileRecord->filename)) {
+        $this->filepath = $this->fileRecord->getFileOrThumbnailPath();
-            throw new FileNotStoredLocallyException($this->fileRecord);
-        }
 
         if ($width === null) {
             $width  = common_config('thumbnail', 'width');
@@ -575,7 +573,7 @@ class ImageFile extends MediaFile
             return $thumb;
         }
 
-        $filename = $this->fileRecord->filehash ?: $this->filename;    // Remote files don't have $this->filehash
+        $filename = basename($this->filepath);
         $extension = File::guessMimeExtension($this->mimetype);
         $outname = "thumb-{$this->fileRecord->getID()}-{$width}x{$height}-{$filename}." . $extension;
         $outpath = File_thumbnail::path($outname);

plugins/Oembed/OembedPlugin.php

@@ -411,7 +411,7 @@ class OembedPlugin extends Plugin
      * states where it isn't oEmbed data, so it doesn't mess up the event handle
      * for other things hooked into it), or the exception if it fails.
      */
-    public function onCreateFileImageThumbnailSource(File $file, &$imgPath)
+    public function onCreateFileImageThumbnailSource(File $file, &$imgPath, $media)
     {
         // If we are on a private node, we won't do any remote calls (just as a precaution until
         // we can configure this from config.php for the private nodes)