[ActivityPub][Ostatus] Fetch avatars in accordance to the new media system

classes/Avatar.php

@@ -164,8 +164,13 @@ class Avatar extends Managed_DataObject
 
     /**
      * Where should the avatar go for this user?
+     * @param int $id user id
+     * @param string $extension file extension
+     * @param int|null $size file size
+     * @param string|null $extra extra bit for the filename
+     * @return string
      */
-    public static function filename($id, $extension, $size = null, $extra = null)
+    public static function filename(int $id, string $extension, ?int $size = null, ?string $extra = null)
     {
         if ($size) {
             return $id . '-' . $size . (($extra) ? ('-' . $extra) : '') . $extension;

classes/File.php

@@ -342,7 +342,7 @@ class File extends Managed_DataObject
 
     /**
      * @param string $filename
-     * @return string|bool Value from the 'extblacklist' array, in the config
+     * @return null|string|bool Value from the 'extblacklist' array, in the config
      * @throws ServerException
      */
     public static function getSafeExtension(string $filename)

lib/media/imagefile.php

@@ -248,6 +248,33 @@ class ImageFile extends MediaFile
         }
     }
 
+    /**
+     * Process a file upload
+     *
+     * Uses MediaFile's `fromURL` to do the majority of the work
+     * and ensures the uploaded file is in fact an image.
+     *
+     * @param string $url Remote image URL
+     * @param Profile|null $scoped
+     * @return ImageFile
+     * @throws ClientException
+     * @throws ServerException
+     */
+    public static function fromUrl(string $url, ?Profile $scoped = null): self
+    {
+        $mediafile = parent::fromUrl($url, $scoped);
+        if ($mediafile instanceof self) {
+            return $mediafile;
+        } else {
+            // We can conclude that we have failed to get the MIME type
+            // TRANS: Client exception thrown trying to upload an invalid image type.
+            // TRANS: %s is the file type that was denied
+            $hint = sprintf(_m('"%s" is not a supported file type on this server. ' .
+                'Try using another image format.'), $mediafile->mimetype);
+            throw new ClientException($hint);
+        }
+    }
+
     /**
      * Several obscure file types should be normalized to PNG on resize.
      *

lib/media/mediafile.php

@@ -247,10 +247,13 @@ class MediaFile
      * Encodes a file name and a file hash in the new file format, which is used to avoid
      * having an extension in the file, removing trust in extensions, while keeping the original name
      *
-     * @param mixed $original_name
+     * @param null|string $original_name
-     * @param null|mixed $ext
+     * @param string $filehash
+     * @param null|string|bool $ext from File::getSafeExtension
      *
+     * @return string
      * @throws ClientException
+     * @throws ServerException
      */
     public static function encodeFilename($original_name, string $filehash, $ext = null): string
     {
@@ -331,18 +334,17 @@ class MediaFile
      * format ("{$hash}.{$ext}")
      *
      * @param string $param Form name
-     * @param null|Profile $scoped
+     * @param Profile|null $scoped
-     *
      * @return ImageFile|MediaFile
+     * @throws ClientException
+     * @throws InvalidFilenameException
      * @throws NoResultException
      * @throws NoUploadedMediaException
      * @throws ServerException
      * @throws UnsupportedMediaException
      * @throws UseFileAsThumbnailException
-     *
-     * @throws ClientException
      */
-    public static function fromUpload(string $param = 'media', Profile $scoped = null)
+    public static function fromUpload(string $param = 'media', ?Profile $scoped = null)
     {
         // The existence of the "error" element means PHP has processed it properly even if it was ok.
         if (!(isset($_FILES[$param], $_FILES[$param]['error']))) {
@@ -434,6 +436,87 @@ class MediaFile
         return new self($filepath, $mimetype, $filehash);
     }
 
+    /**
+     * Create a new MediaFile or ImageFile object from an url
+     *
+     * Tries to set the mimetype correctly, using the most secure method available and rejects the file otherwise.
+     * In case the url is an image, this function returns an new ImageFile (which extends MediaFile)
+     * The filename has the following format: bin2hex("{$original_name}.{$ext}")."-{$filehash}"
+     *
+     * @param string $url Remote media URL
+     * @param Profile|null $scoped
+     * @return ImageFile|MediaFile
+     * @throws ServerException
+     */
+    public static function fromUrl(string $url, ?Profile $scoped = null)
+    {
+        if (!common_valid_http_url($url)) {
+            // TRANS: Server exception. %s is a URL.
+            throw new ServerException(sprintf('Invalid remote media URL %s.', $url));
+        }
+
+        $temp_filename = tempnam(sys_get_temp_dir(), 'tmp' . common_timestamp());
+
+        try {
+            $fileData = HTTPClient::quickGet($url);
+            file_put_contents($temp_filename, $fileData);
+            unset($fileData);    // No need to carry this in memory.
+
+            $filehash = strtolower(self::getHashOfFile($temp_filename));
+
+            try {
+                $file = File::getByHash($filehash);
+                // If no exception is thrown the file exists locally, so we'll use that and just add redirections.
+                // but if the _actual_ locally stored file doesn't exist, getPath will throw FileNotFoundException
+                $filepath = $file->getPath();
+                $mimetype = $file->mimetype;
+            } catch (FileNotFoundException | NoResultException $e) {
+                // We have to save the downloaded as a new local file. This is the normal course of action.
+                if ($scoped instanceof Profile) {
+                    // Throws exception if additional size does not respect quota
+                    // This test is only needed, of course, if we're uploading something new.
+                    File::respectsQuota($scoped, filesize($temp_filename));
+                }
+
+                $mimetype = self::getUploadedMimeType($temp_filename);
+                $media = common_get_mime_media($mimetype);
+
+                $basename = basename($temp_filename);
+
+                if ($media == 'image') {
+                    // Use -1 for the id to avoid adding this temporary file to the DB
+                    $img = new ImageFile(-1, $temp_filename);
+                    // Validate the image by re-encoding 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);
+                }
+                $filename = self::encodeFilename($basename, $filehash, isset($ext) ? $ext : File::getSafeExtension($basename));
+
+                $filepath = File::path($filename);
+
+                if ($media == 'image') {
+                    $result = rename($outpath, $filepath);
+                } else {
+                    $result = rename($temp_filename, $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.
+                    throw new ServerException(_m('File could not be moved to destination directory.'));
+                }
+
+                if ($media == 'image') {
+                    return new ImageFile(null, $filepath);
+                }
+            }
+            return new self($filepath, $mimetype, $filehash);
+        } catch (Exception $e) {
+            unlink($temp_filename); // Garbage collect
+            throw $e;
+        }
+    }
+
     public static function fromFilehandle($fh, Profile $scoped = null)
     {
         $stream = stream_get_meta_data($fh);

plugins/ActivityPub/lib/explorer.php

@@ -281,72 +281,6 @@ class Activitypub_explorer
         return $profile;
     }
 
-    /**
-     * Download and update given avatar image
-     *
-     * @param Profile $profile
-     * @param string $url
-     * @return Avatar    The Avatar we have on disk.
-     * @throws Exception in various failure cases
-     * @author GNU social
-     */
-    public static function update_avatar(Profile $profile, $url)
-    {
-        common_debug('ActivityPub Explorer: Started grabbing remote avatar from: ' . $url);
-        if (!filter_var($url, FILTER_VALIDATE_URL)) {
-            // TRANS: Server exception. %s is a URL.
-            common_debug('ActivityPub Explorer: Failed because it is an invalid url: ' . $url);
-            throw new ServerException(sprintf('Invalid avatar URL %s.', $url));
-        }
-
-        // @todo FIXME: This should be better encapsulated
-        // ripped from oauthstore.php (for old OMB client)
-        $temp_filename = tempnam(sys_get_temp_dir(), 'listener_avatar');
-        try {
-            $imgData = HTTPClient::quickGet($url);
-            // Make sure it's at least an image file. ImageFile can do the rest.
-            if (false === getimagesizefromstring($imgData)) {
-                common_debug('ActivityPub Explorer: Failed because the downloaded avatar: ' . $url . 'is not a valid image.');
-                throw new UnsupportedMediaException('Downloaded avatar was not an image.');
-            }
-            file_put_contents($temp_filename, $imgData);
-            unset($imgData);    // No need to carry this in memory.
-            common_debug('ActivityPub Explorer: Stored dowloaded avatar in: ' . $temp_filename);
-
-            $id = $profile->getID();
-
-            $imagefile = new ImageFile(null, $temp_filename);
-            $filename = Avatar::filename(
-                $id,
-                image_type_to_extension($imagefile->type),
-                null,
-                common_timestamp()
-            );
-            rename($temp_filename, Avatar::path($filename));
-            common_debug('ActivityPub Explorer: Moved avatar from: ' . $temp_filename . ' to ' . $filename);
-        } catch (Exception $e) {
-            common_debug('ActivityPub Explorer: Something went wrong while processing the avatar from: ' . $url . ' details: ' . $e->getMessage());
-            unlink($temp_filename);
-            throw $e;
-        }
-        // @todo FIXME: Hardcoded chmod is lame, but seems to be necessary to
-        // keep from accidentally saving images from command-line (queues)
-        // that can't be read from web server, which causes hard-to-notice
-        // problems later on:
-        //
-        // http://status.net/open-source/issues/2663
-        chmod(Avatar::path($filename), 0644);
-
-        $profile->setOriginal($filename);
-
-        $orig = clone($profile);
-        $profile->avatar = $url;
-        $profile->update($orig);
-
-        common_debug('ActivityPub Explorer: Seted Avatar from: ' . $url . ' to profile.');
-        return Avatar::getUploaded($profile);
-    }
-
     /**
      * Validates a remote response in order to determine whether this
      * response is a valid profile or not
@@ -417,6 +351,47 @@ class Activitypub_explorer
         return false;
     }
 
+    /**
+     * Download and update given avatar image
+     * TODO: Avoid updating an avatar if its URL didn't change. (this is something OStatus already does)
+     * TODO: Should be in AProfile instead?
+     *
+     * @param Profile $profile
+     * @param string $url
+     * @return Avatar    The Avatar we have on disk. (seldom used)
+     * @throws Exception in various failure cases
+     * @author Diogo Cordeiro <diogo@fc.up.pt>
+     */
+    public static function update_avatar(Profile $profile, string $url): Avatar
+    {
+        common_debug('ActivityPub Explorer: Started grabbing remote avatar from: ' . $url);
+        // ImageFile throws exception if something goes wrong, which we'll let go on its merry way
+        $imagefile = ImageFile::fromURL($url);
+
+        $id = $profile->getID();
+
+        $type = $imagefile->preferredType();
+        $filename = Avatar::filename(
+            $id,
+            image_type_to_extension($type),
+            null,
+            'tmp' . common_timestamp()
+        );
+
+        $filepath = Avatar::path($filename);
+        /*$imagefile = */$imagefile->copyTo($filepath);
+
+        common_debug('ActivityPub Explorer: Stored avatar in: ' . $filepath);
+
+        // XXX: Do we need this?
+        chmod($filepath, 0644);
+
+        $profile->setOriginal($filename);
+
+        common_debug('ActivityPub Explorer: Seted Avatar from: ' . $url . ' to profile.');
+        return Avatar::getUploaded($profile);
+    }
+
     /**
      * Allows the Explorer to transverse a collection of persons.
      *

plugins/OStatus/classes/Ostatus_profile.php

@@ -909,56 +909,38 @@ class Ostatus_profile extends Managed_DataObject
             // If avatar URL differs: update. If URLs were identical but we're forced: update.
             if ($url == $this->avatar && !$force) {
                 // If there's no locally stored avatar, throw an exception and continue fetching below.
-                $avatar = Avatar::getUploaded($this->localProfile()) instanceof Avatar;
+                $avatar = Avatar::getUploaded($this->localProfile());
                 return $avatar;
             }
         } catch (NoAvatarException $e) {
             // No avatar available, let's fetch it.
         }
         
-        if (!common_valid_http_url($url)) {
+        // ImageFile throws exception if something goes wrong, which we'll let go on its merry way
-            // TRANS: Server exception. %s is a URL.
+        $imagefile = ImageFile::fromURL($url);
-            throw new ServerException(sprintf(_m('Invalid avatar URL %s.'), $url));
-        }
 
         $self = $this->localProfile();
 
-        // @todo FIXME: This should be better encapsulated
+        $id = $self->getID();
-        // ripped from oauthstore.php (for old OMB client)
+
-        $temp_filename = tempnam(sys_get_temp_dir(), 'listener_avatar');
+        $type = $imagefile->preferredType();
-        try {
+        $filename = Avatar::filename(
-            $imgData = HTTPClient::quickGet($url);
+            $id,
-            // Make sure it's at least an image file. ImageFile can do the rest.
+            image_type_to_extension($type),
-            if (false === getimagesizefromstring($imgData)) {
+            null,
-                throw new UnsupportedMediaException(_('Downloaded group avatar was not an image.'));
+            'tmp' . common_timestamp()
-            }
+        );
-            file_put_contents($temp_filename, $imgData);
+
-            unset($imgData);    // No need to carry this in memory.
+        $filepath = Avatar::path($filename);
+        /*$imagefile = */$imagefile->copyTo($filepath);
 
-            if ($this->isGroup()) {
-                $id = $this->group_id;
-            } else {
-                $id = $this->profile_id;
-            }
-            $imagefile = new ImageFile(null, $temp_filename);
-            $filename = Avatar::filename(
-                $id,
-                image_type_to_extension($imagefile->type),
-                null,
-                common_timestamp()
-            );
-            rename($temp_filename, Avatar::path($filename));
-        } catch (Exception $e) {
-            unlink($temp_filename);
-            throw $e;
-        }
         // @todo FIXME: Hardcoded chmod is lame, but seems to be necessary to
         // keep from accidentally saving images from command-line (queues)
         // that can't be read from web server, which causes hard-to-notice
         // problems later on:
         //
         // http://status.net/open-source/issues/2663
-        chmod(Avatar::path($filename), 0644);
+        chmod($filepath, 0644);
 
         $self->setOriginal($filename);