Clean up remote avatar temporary files if we fail before saving them into avatars directory (OMB core, OStatus, WikiHowProfile, YammerImport)

3 files changed, 54 insertions, 39 deletions

diff --git a/plugins/OStatus/classes/Ostatus_profile.php b/plugins/OStatus/classes/Ostatus_profile.php
index 47aee15f8..03fcb71df 100644
--- a/plugins/OStatus/classes/Ostatus_profile.php
+++ b/plugins/OStatus/classes/Ostatus_profile.php
@@ -1053,22 +1053,27 @@ class Ostatus_profile extends Memcached_DataObject
         // @fixme this should be better encapsulated
         // ripped from oauthstore.php (for old OMB client)
         $temp_filename = tempnam(sys_get_temp_dir(), 'listener_avatar');
-        if (!copy($url, $temp_filename)) {
-            throw new ServerException(sprintf(_m("Unable to fetch avatar from %s."), $url));
-        }
+        try {
+            if (!copy($url, $temp_filename)) {
+                throw new ServerException(sprintf(_m("Unable to fetch avatar from %s."), $url));
+            }
 
-        if ($this->isGroup()) {
-            $id = $this->group_id;
-        } else {
-            $id = $this->profile_id;
-        }
-        // @fixme should we be using different ids?
-        $imagefile = new ImageFile($id, $temp_filename);
-        $filename = Avatar::filename($id,
-                                     image_type_to_extension($imagefile->type),
-                                     null,
-                                     common_timestamp());
-        rename($temp_filename, Avatar::path($filename));
+            if ($this->isGroup()) {
+                $id = $this->group_id;
+            } else {
+                $id = $this->profile_id;
+            }
+            // @fixme should we be using different ids?
+            $imagefile = new ImageFile($id, $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;
+        }
         // @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
diff --git a/plugins/WikiHowProfile/WikiHowProfilePlugin.php b/plugins/WikiHowProfile/WikiHowProfilePlugin.php
index fa683c483..753dff5a3 100644
--- a/plugins/WikiHowProfile/WikiHowProfilePlugin.php
+++ b/plugins/WikiHowProfile/WikiHowProfilePlugin.php
@@ -174,20 +174,25 @@ class WikiHowProfilePlugin extends Plugin
         // @fixme this should be better encapsulated
         // ripped from OStatus via oauthstore.php (for old OMB client)
         $temp_filename = tempnam(sys_get_temp_dir(), 'listener_avatar');
-        if (!copy($url, $temp_filename)) {
-            throw new ServerException(sprintf(_m("Unable to fetch avatar from %s."), $url));
-        }
-
-        $profile = $user->getProfile();
-        $id = $profile->id;
-        // @fixme should we be using different ids?
+        try {
+            if (!copy($url, $temp_filename)) {
+                throw new ServerException(sprintf(_m("Unable to fetch avatar from %s."), $url));
+            }
 
-        $imagefile = new ImageFile($id, $temp_filename);
-        $filename = Avatar::filename($id,
-                                     image_type_to_extension($imagefile->type),
-                                     null,
-                                     common_timestamp());
-        rename($temp_filename, Avatar::path($filename));
+            $profile = $user->getProfile();
+            $id = $profile->id;
+            // @fixme should we be using different ids?
+
+            $imagefile = new ImageFile($id, $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;
+        }
         $profile->setOriginal($filename);
     }
 }
diff --git a/plugins/YammerImport/lib/yammerimporter.php b/plugins/YammerImport/lib/yammerimporter.php
index 80cbcff8e..93bc96d52 100644
--- a/plugins/YammerImport/lib/yammerimporter.php
+++ b/plugins/YammerImport/lib/yammerimporter.php
@@ -436,18 +436,23 @@ class YammerImporter
         // @fixme this should be better encapsulated
         // ripped from oauthstore.php (for old OMB client)
         $temp_filename = tempnam(sys_get_temp_dir(), 'listener_avatar');
-        if (!copy($url, $temp_filename)) {
-            throw new ServerException(sprintf(_m("Unable to fetch avatar from %s."), $url));
-        }
+        try {
+            if (!copy($url, $temp_filename)) {
+                throw new ServerException(sprintf(_m("Unable to fetch avatar from %s."), $url));
+            }
 
-        $id = $dest->id;
-        // @fixme should we be using different ids?
-        $imagefile = new ImageFile($id, $temp_filename);
-        $filename = Avatar::filename($id,
-                                     image_type_to_extension($imagefile->type),
-                                     null,
-                                     common_timestamp());
-        rename($temp_filename, Avatar::path($filename));
+            $id = $dest->id;
+            // @fixme should we be using different ids?
+            $imagefile = new ImageFile($id, $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;
+        }
         // @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