Fix for tickets #2917, #2262: user URL shortening options not being applied in non-web channels

common_shorten_links() can only access the web session's logged-in user, so never properly took user options into effect for posting via XMPP, API, mail, etc.

Adds an optional $user parameter on common_shorten_links(), and a $user->shortenLinks() as a clearer interface for that.
Tweaked some lower-level functions so $user gets passed down -- making the $notice_id param previously there for saving URLs at notice save time generalized a little.

Note also ticket #2919: there's a lot of duplicate code calling the shortening, checking the length, and reporting near-identical error messages. These should be consolidated to aid in code and translation maintenance.

15 files changed, 98 insertions, 34 deletions

diff --git a/actions/apidirectmessagenew.php b/actions/apidirectmessagenew.php
index b335a9c93..978c75353 100644
--- a/actions/apidirectmessagenew.php
+++ b/actions/apidirectmessagenew.php
@@ -119,7 +119,7 @@ class ApiDirectMessageNewAction extends ApiAuthAction
                 $this->format
             );
         } else {
-            $content_shortened = common_shorten_links($this->content);
+            $content_shortened = $this->auth_user->shortenLinks($this->content);
             if (Message::contentTooLong($content_shortened)) {
                 $this->clientError(
                     // TRANS: Client error displayed when message content is too long.
diff --git a/actions/apistatusesupdate.php b/actions/apistatusesupdate.php
index 1a3b54900..a8ec7f8bb 100644
--- a/actions/apistatusesupdate.php
+++ b/actions/apistatusesupdate.php
@@ -231,7 +231,7 @@ class ApiStatusesUpdateAction extends ApiAuthAction
             return;
         }
 
-        $status_shortened = common_shorten_links($this->status);
+        $status_shortened = $this->auth_user->shortenlinks($this->status);
 
         if (Notice::contentTooLong($status_shortened)) {
             // Note: Twitter truncates anything over 140, flags the status
diff --git a/actions/apitimelineuser.php b/actions/apitimelineuser.php
index b0c8b4df8..d90507aa4 100644
--- a/actions/apitimelineuser.php
+++ b/actions/apitimelineuser.php
@@ -380,7 +380,7 @@ class ApiTimelineUserAction extends ApiBareAuthAction
         $rendered = $this->purify($sourceContent);
         $content = html_entity_decode(strip_tags($rendered), ENT_QUOTES, 'UTF-8');
 
-        $shortened = common_shorten_links($content);
+        $shortened = $this->auth_user->shortenLinks($content);
 
         $options = array('is_local' => Notice::LOCAL_PUBLIC,
                          'rendered' => $rendered,
diff --git a/actions/newmessage.php b/actions/newmessage.php
index c58ed3849..447a00580 100644
--- a/actions/newmessage.php
+++ b/actions/newmessage.php
@@ -144,7 +144,7 @@ class NewmessageAction extends Action
             $this->showForm(_('No content!'));
             return;
         } else {
-            $content_shortened = common_shorten_links($this->content);
+            $content_shortened = $user->shortenLinks($this->content);
 
             if (Message::contentTooLong($content_shortened)) {
                 // TRANS: Form validation error displayed when message content is too long.
diff --git a/actions/newnotice.php b/actions/newnotice.php
index 0d4dcfccd..faafd9551 100644
--- a/actions/newnotice.php
+++ b/actions/newnotice.php
@@ -154,7 +154,7 @@ class NewnoticeAction extends Action
             return;
         }
 
-        $content_shortened = common_shorten_links($content);
+        $content_shortened = $user->shortenLinks($content);
         if (Notice::contentTooLong($content_shortened)) {
             // TRANS: Client error displayed when the parameter "status" is missing.
             // TRANS: %d is the maximum number of character for a notice.
diff --git a/classes/File_redirection.php b/classes/File_redirection.php
index 4ee43026b..53c15bf8b 100644
--- a/classes/File_redirection.php
+++ b/classes/File_redirection.php
@@ -187,13 +187,14 @@ class File_redirection extends Memcached_DataObject
      * may be saved.
      *
      * @param string $long_url
+     * @param User $user whose shortening options to use; defaults to the current web session user
      * @return string
      */
-    function makeShort($long_url) {
+    function makeShort($long_url, $user=null) {
 
         $canon = File_redirection::_canonUrl($long_url);
 
-        $short_url = File_redirection::_userMakeShort($canon);
+        $short_url = File_redirection::_userMakeShort($canon, $user);
 
         // Did we get one? Is it shorter?
         if (!empty($short_url) && mb_strlen($short_url) < mb_strlen($long_url)) {
@@ -203,8 +204,8 @@ class File_redirection extends Memcached_DataObject
         }
     }
 
-    function _userMakeShort($long_url) {
-        $short_url = common_shorten_url($long_url);
+    function _userMakeShort($long_url, User $user=null) {
+        $short_url = common_shorten_url($long_url, $user);
         if (!empty($short_url) && $short_url != $long_url) {
             $short_url = (string)$short_url;
             // store it
diff --git a/classes/Message.php b/classes/Message.php
index 353dc01f9..c4508187b 100644
--- a/classes/Message.php
+++ b/classes/Message.php
@@ -45,11 +45,18 @@ class Message extends Memcached_DataObject
             throw new ClientException(_('You are banned from sending direct messages.'));
         }
 
+        $user = User::staticGet('id', $sender->id);
+
         $msg = new Message();
 
         $msg->from_profile = $from;
         $msg->to_profile = $to;
-        $msg->content = common_shorten_links($content);
+        if ($user) {
+            // Use the sender's URL shortening options.
+            $msg->content = $user->shortenLinks($content);
+        } else {
+            $msg->content = common_shorten_links($content);
+        }
         $msg->rendered = common_render_text($content);
         $msg->created = common_sql_now();
         $msg->source = $source;
diff --git a/classes/Notice.php b/classes/Notice.php
index dd9438f16..4169f1a1c 100644
--- a/classes/Notice.php
+++ b/classes/Notice.php
@@ -256,9 +256,14 @@ class Notice extends Memcached_DataObject
             $is_local = Notice::LOCAL_PUBLIC;
         }
 
-        $profile = Profile::staticGet($profile_id);
-
-        $final = common_shorten_links($content);
+        $profile = Profile::staticGet('id', $profile_id);
+        $user = User::staticGet('id', $profile_id);
+        if ($user) {
+            // Use the local user's shortening preferences, if applicable.
+            $final = $user->shortenLinks($content);
+        } else {
+            $final = common_shorten_links($content);
+        }
 
         if (Notice::contentTooLong($final)) {
             // TRANS: Client exception thrown if a notice contains too many characters.
@@ -502,8 +507,7 @@ class Notice extends Memcached_DataObject
     /**
      * @private callback
      */
-    function saveUrl($data) {
-        list($url, $notice_id) = $data;
+    function saveUrl($url, $notice_id) {
         File::processNew($url, $notice_id);
     }
 
diff --git a/classes/User.php b/classes/User.php
index 92180a9fb..041c64c38 100644
--- a/classes/User.php
+++ b/classes/User.php
@@ -927,4 +927,23 @@ class User extends Memcached_DataObject
             throw new ServerException(_('Single-user mode code called when not enabled.'));
         }
     }
+
+    /**
+     * Find and shorten links in the given text using this user's URL shortening
+     * settings.
+     *
+     * By default, links will be left untouched if the text is shorter than the
+     * configured maximum notice length. Pass true for the $always parameter
+     * to force all links to be shortened regardless.
+     *
+     * Side effects: may save file and file_redirection records for referenced URLs.
+     *
+     * @param string $text
+     * @param boolean $always
+     * @return string
+     */
+    public function shortenLinks($text, $always=false)
+    {
+        return common_shorten_links($text, $always, $this);
+    }
 }
diff --git a/lib/command.php b/lib/command.php
index 2a8075e7b..3fb4d76c7 100644
--- a/lib/command.php
+++ b/lib/command.php
@@ -479,7 +479,7 @@ class MessageCommand extends Command
             return;
         }
 
-        $this->text = common_shorten_links($this->text);
+        $this->text = $this->user->shortenLinks($this->text);
 
         if (Message::contentTooLong($this->text)) {
             // XXX: i18n. Needs plural support.
@@ -582,7 +582,7 @@ class ReplyCommand extends Command
             return;
         }
 
-        $this->text = common_shorten_links($this->text);
+        $this->text = $this->user->shortenLinks($this->text);
 
         if (Notice::contentTooLong($this->text)) {
             // XXX: i18n. Needs plural support.
diff --git a/lib/mailhandler.php b/lib/mailhandler.php
index 69eb26bdd..459657ffe 100644
--- a/lib/mailhandler.php
+++ b/lib/mailhandler.php
@@ -55,7 +55,7 @@ class MailHandler
             return true;
         }
         $msg = $this->cleanup_msg($msg);
-        $msg = common_shorten_links($msg);
+        $msg = $user->shortenLinks($msg);
         if (Notice::contentTooLong($msg)) {
             $this->error($from, sprintf(_('That\'s too long. Maximum notice size is %d character.',
                                           'That\'s too long. Maximum notice size is %d characters.',
diff --git a/lib/util.php b/lib/util.php
index 42762b22f..26b20b5e1 100644
--- a/lib/util.php
+++ b/lib/util.php
@@ -789,7 +789,14 @@ function common_render_text($text)
     return $r;
 }
 
-function common_replace_urls_callback($text, $callback, $notice_id = null) {
+/**
+ * Find links in the given text and pass them to the given callback function.
+ *
+ * @param string $text
+ * @param function($text, $arg) $callback: return replacement text
+ * @param mixed $arg: optional argument will be passed on to the callback
+ */
+function common_replace_urls_callback($text, $callback, $arg = null) {
     // Start off with a regex
     $regex = '#'.
     '(?:^|[\s\<\>\(\)\[\]\{\}\\\'\\\";]+)(?![\@\!\#])'.
@@ -830,10 +837,21 @@ function common_replace_urls_callback($text, $callback, $notice_id = null) {
     '#ixu';
     //preg_match_all($regex,$text,$matches);
     //print_r($matches);
-    return preg_replace_callback($regex, curry('callback_helper',$callback,$notice_id) ,$text);
+    return preg_replace_callback($regex, curry('callback_helper',$callback,$arg) ,$text);
 }
 
-function callback_helper($matches, $callback, $notice_id) {
+/**
+ * Intermediate callback for common_replace_links(), helps resolve some
+ * ambiguous link forms before passing on to the final callback.
+ *
+ * @param array $matches
+ * @param callable $callback
+ * @param mixed $arg optional argument to pass on as second param to callback
+ * @return string
+ * 
+ * @access private
+ */
+function callback_helper($matches, $callback, $arg=null) {
     $url=$matches[1];
     $left = strpos($matches[0],$url);
     $right = $left+strlen($url);
@@ -876,11 +894,7 @@ function callback_helper($matches, $callback, $notice_id) {
         }
     }while($original_url!=$url);
 
-    if(empty($notice_id)){
-        $result = call_user_func_array($callback, array($url));
-    }else{
-        $result = call_user_func_array($callback, array(array($url,$notice_id)) );
-    }
+    $result = call_user_func_array($callback, array($url, $arg));
     return substr($matches[0],0,$left) . $result . substr($matches[0],$right);
 }
 
@@ -980,11 +994,27 @@ function common_linkify($url) {
     return XMLStringer::estring('a', $attrs, $url);
 }
 
-function common_shorten_links($text, $always = false)
+/**
+ * Find and shorten links in a given chunk of text if it's longer than the
+ * configured notice content limit (or unconditionally).
+ *
+ * Side effects: may save file and file_redirection records for referenced URLs.
+ *
+ * Pass the $user option or call $user->shortenLinks($text) to ensure the proper
+ * user's options are used; otherwise the current web session user's setitngs
+ * will be used or ur1.ca if there is no active web login.
+ *
+ * @param string $text
+ * @param boolean $always (optional)
+ * @param User $user (optional)
+ *
+ * @return string
+ */
+function common_shorten_links($text, $always = false, User $user=null)
 {
     $maxLength = Notice::maxContent();
     if (!$always && ($maxLength == 0 || mb_strlen($text) <= $maxLength)) return $text;
-    return common_replace_urls_callback($text, array('File_redirection', 'makeShort'));
+    return common_replace_urls_callback($text, array('File_redirection', 'makeShort'), $user);
 }
 
 /**
@@ -2028,15 +2058,18 @@ function common_database_tablename($tablename)
  * Length is not considered.
  *
  * @param string $long_url
+ * @param User $user to specify a particular user's options
  * @return string may return the original URL if shortening failed
  *
  * @fixme provide a way to specify a particular shortener
- * @fixme provide a way to specify to use a given user's shortening preferences
  */
-function common_shorten_url($long_url)
+function common_shorten_url($long_url, User $user=null)
 {
     $long_url = trim($long_url);
-    $user = common_current_user();
+    if (empty($user)) {
+        // Current web session
+        $user = common_current_user();
+    }
     if (empty($user)) {
         // common current user does not find a user when called from the XMPP daemon
         // therefore we'll set one here fix, so that XMPP given URLs may be shortened
diff --git a/lib/xmppmanager.php b/lib/xmppmanager.php
index 238696664..585d044c7 100644
--- a/lib/xmppmanager.php
+++ b/lib/xmppmanager.php
@@ -398,7 +398,7 @@ class XmppManager extends IoManager
     function add_notice(&$user, &$pl)
     {
         $body = trim($pl['body']);
-        $content_shortened = common_shorten_links($body);
+        $content_shortened = $user->shortenLinks($body);
         if (Notice::contentTooLong($content_shortened)) {
           $from = jabber_normalize_jid($pl['from']);
           // TRANS: Response to XMPP source when it sent too long a message.
diff --git a/plugins/Facebook/facebookaction.php b/plugins/Facebook/facebookaction.php
index 4c15fc039..e4edcea0d 100644
--- a/plugins/Facebook/facebookaction.php
+++ b/plugins/Facebook/facebookaction.php
@@ -370,7 +370,7 @@ class FacebookAction extends Action
             $this->showPage(_m('No notice content!'));
             return;
         } else {
-            $content_shortened = common_shorten_links($content);
+            $content_shortened = $user->shortenLinks($content);
 
             if (Notice::contentTooLong($content_shortened)) {
                 // @todo FIXME: i18n: Needs plural.
diff --git a/scripts/restoreuser.php b/scripts/restoreuser.php
index 8327c796c..b37e9db74 100644
--- a/scripts/restoreuser.php
+++ b/scripts/restoreuser.php
@@ -219,7 +219,7 @@ function postNote($user, $activity)
     $rendered = purify($sourceContent);
     $content = html_entity_decode(strip_tags($rendered), ENT_QUOTES, 'UTF-8');
 
-    $shortened = common_shorten_links($content);
+    $shortened = $user->shortenLinks($content);
 
     $options = array('is_local' => Notice::LOCAL_PUBLIC,
                      'uri' => $sourceUri,