Fixup script for files w/ bogus data saved into file record ('h bug')

3 files changed, 64 insertions, 24 deletions

diff --git a/classes/File.php b/classes/File.php
index a83ecac4c..ba8332841 100644
--- a/classes/File.php
+++ b/classes/File.php
@@ -84,19 +84,36 @@ class File extends Memcached_DataObject
         if (isset($redir_data['time']) && $redir_data['time'] > 0) $x->date = intval($redir_data['time']);
         $file_id = $x->insert();
 
+        $x->saveOembed($redir_data, $given_url);
+        return $x;
+    }
+
+    /**
+     * Save embedding information for this file, if applicable.
+     *
+     * Normally this won't need to be called manually, as File::saveNew()
+     * takes care of it.
+     *
+     * @param array $redir_data lookup data eg from File_redirection::where()
+     * @param string $given_url
+     * @return boolean success
+     */
+    public function saveOembed($redir_data, $given_url)
+    {
         if (isset($redir_data['type'])
             && (('text/html' === substr($redir_data['type'], 0, 9) || 'application/xhtml+xml' === substr($redir_data['type'], 0, 21)))
             && ($oembed_data = File_oembed::_getOembed($given_url))) {
 
-            $fo = File_oembed::staticGet('file_id', $file_id);
+            $fo = File_oembed::staticGet('file_id', $this->id);
 
             if (empty($fo)) {
-                File_oembed::saveNew($oembed_data, $file_id);
+                File_oembed::saveNew($oembed_data, $this->id);
+                return true;
             } else {
                 common_log(LOG_WARNING, "Strangely, a File_oembed object exists for new file $file_id", __FILE__);
             }
         }
-        return $x;
+        return false;
     }
 
     function processNew($given_url, $notice_id=null) {
@@ -112,6 +129,7 @@ class File extends Memcached_DataObject
                     $redir_url = $redir_data['url'];
                 } elseif (is_string($redir_data)) {
                     $redir_url = $redir_data;
+                    $redir_data = array();
                 } else {
                     throw new ServerException("Can't process url '$given_url'");
                 }
diff --git a/classes/File_oembed.php b/classes/File_oembed.php
index f59eaf24c..041b44740 100644
--- a/classes/File_oembed.php
+++ b/classes/File_oembed.php
@@ -84,10 +84,10 @@ class File_oembed extends Memcached_DataObject
     /**
      * Save embedding info for a new file.
      *
-     * @param array $data lookup data as from File_redirection::where
+     * @param object $data Services_oEmbed_Object_*
      * @param int $file_id
      */
-    function saveNew(array $data, $file_id) {
+    function saveNew($data, $file_id) {
         $file_oembed = new File_oembed;
         $file_oembed->file_id = $file_id;
         $file_oembed->version = $data->version;
diff --git a/classes/File_redirection.php b/classes/File_redirection.php
index d96979158..f128b3e07 100644
--- a/classes/File_redirection.php
+++ b/classes/File_redirection.php
@@ -58,24 +58,30 @@ class File_redirection extends Memcached_DataObject
         return $request;
     }
 
-    function _redirectWhere_imp($short_url, $redirs = 10, $protected = false) {
+    /**
+     * Check if this URL is a redirect and return redir info.
+     *
+     * Most code should call File_redirection::where instead, to check if we
+     * already know that redirection and avoid extra hits to the web.
+     *
+     * The URL is hit and any redirects are followed, up to 10 levels or until
+     * a protected URL is reached.
+     *
+     * @param string $in_url
+     * @return mixed one of:
+     *         string - target URL, if this is a direct link or can't be followed
+     *         array - redirect info if this is an *unknown* redirect:
+     *              associative array with the following elements:
+     *                code: HTTP status code
+     *                redirects: count of redirects followed
+     *                url: URL string of final target
+     *                type (optional): MIME type from Content-Type header
+     *                size (optional): byte size from Content-Length header
+     *                time (optional): timestamp from Last-Modified header
+     */
+    public function lookupWhere($short_url, $redirs = 10, $protected = false) {
         if ($redirs < 0) return false;
 
-        // let's see if we know this...
-        $a = File::staticGet('url', $short_url);
-
-        if (!empty($a)) {
-            // this is a direct link to $a->url
-            return $a->url;
-        } else {
-            $b = File_redirection::staticGet('url', $short_url);
-            if (!empty($b)) {
-                // this is a redirect to $b->file_id
-                $a = File::staticGet('id', $b->file_id);
-                return $a->url;
-            }
-        }
-
         if(strpos($short_url,'://') === false){
             return $short_url;
         }
@@ -93,12 +99,13 @@ class File_redirection extends Memcached_DataObject
             }
         } catch (Exception $e) {
             // Invalid URL or failure to reach server
+            common_log(LOG_ERR, "Error while following redirects for $short_url: " . $e->getMessage());
             return $short_url;
         }
 
         if ($response->getRedirectCount() && File::isProtected($response->getUrl())) {
             // Bump back up the redirect chain until we find a non-protected URL
-            return self::_redirectWhere_imp($short_url, $response->getRedirectCount() - 1, true);
+            return self::lookupWhere($short_url, $response->getRedirectCount() - 1, true);
         }
 
         $ret = array('code' => $response->getStatus()
@@ -136,8 +143,23 @@ class File_redirection extends Memcached_DataObject
      *                size (optional): byte size from Content-Length header
      *                time (optional): timestamp from Last-Modified header
      */
-    function where($in_url) {
-        $ret = File_redirection::_redirectWhere_imp($in_url);
+    public function where($in_url) {
+        // let's see if we know this...
+        $a = File::staticGet('url', $in_url);
+
+        if (!empty($a)) {
+            // this is a direct link to $a->url
+            return $a->url;
+        } else {
+            $b = File_redirection::staticGet('url', $in_url);
+            if (!empty($b)) {
+                // this is a redirect to $b->file_id
+                $a = File::staticGet('id', $b->file_id);
+                return $a->url;
+            }
+        }
+
+        $ret = File_redirection::lookupWhere($in_url);
         return $ret;
     }