From aa96c3c1d9823382e9e6de0da5084fcc111f2ee5 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 2 Dec 2010 10:56:44 -0800 Subject: [PATCH] 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. --- actions/apidirectmessagenew.php | 2 +- actions/apistatusesupdate.php | 2 +- actions/apitimelineuser.php | 2 +- actions/newmessage.php | 2 +- actions/newnotice.php | 2 +- classes/File_redirection.php | 9 +++-- classes/Message.php | 9 ++++- classes/Notice.php | 14 ++++--- classes/User.php | 19 ++++++++++ lib/command.php | 4 +- lib/mailhandler.php | 2 +- lib/util.php | 59 ++++++++++++++++++++++------- lib/xmppmanager.php | 2 +- plugins/Facebook/facebookaction.php | 2 +- scripts/restoreuser.php | 2 +- 15 files changed, 98 insertions(+), 34 deletions(-) diff --git a/actions/apidirectmessagenew.php b/actions/apidirectmessagenew.php index b335a9c93e..978c753532 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 1a3b549004..a8ec7f8bb9 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 b0c8b4df8d..d90507aa44 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 c58ed3849e..447a00580c 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 0d4dcfccd5..faafd9551d 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 4ee43026b7..53c15bf8b2 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 353dc01f99..c4508187bb 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 dd9438f160..4169f1a1c7 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 92180a9fbc..041c64c38f 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 2a8075e7ba..3fb4d76c75 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 69eb26bdd8..459657ffe0 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 42762b22fb..26b20b5e11 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 238696664a..585d044c74 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 4c15fc0397..e4edcea0d9 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 8327c796c2..b37e9db741 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,