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.
This commit is contained in:
Brion Vibber 2010-12-02 10:56:44 -08:00
parent c4f67f7647
commit aa96c3c1d9
15 changed files with 98 additions and 34 deletions

View File

@ -119,7 +119,7 @@ class ApiDirectMessageNewAction extends ApiAuthAction
$this->format $this->format
); );
} else { } else {
$content_shortened = common_shorten_links($this->content); $content_shortened = $this->auth_user->shortenLinks($this->content);
if (Message::contentTooLong($content_shortened)) { if (Message::contentTooLong($content_shortened)) {
$this->clientError( $this->clientError(
// TRANS: Client error displayed when message content is too long. // TRANS: Client error displayed when message content is too long.

View File

@ -231,7 +231,7 @@ class ApiStatusesUpdateAction extends ApiAuthAction
return; return;
} }
$status_shortened = common_shorten_links($this->status); $status_shortened = $this->auth_user->shortenlinks($this->status);
if (Notice::contentTooLong($status_shortened)) { if (Notice::contentTooLong($status_shortened)) {
// Note: Twitter truncates anything over 140, flags the status // Note: Twitter truncates anything over 140, flags the status

View File

@ -380,7 +380,7 @@ class ApiTimelineUserAction extends ApiBareAuthAction
$rendered = $this->purify($sourceContent); $rendered = $this->purify($sourceContent);
$content = html_entity_decode(strip_tags($rendered), ENT_QUOTES, 'UTF-8'); $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, $options = array('is_local' => Notice::LOCAL_PUBLIC,
'rendered' => $rendered, 'rendered' => $rendered,

View File

@ -144,7 +144,7 @@ class NewmessageAction extends Action
$this->showForm(_('No content!')); $this->showForm(_('No content!'));
return; return;
} else { } else {
$content_shortened = common_shorten_links($this->content); $content_shortened = $user->shortenLinks($this->content);
if (Message::contentTooLong($content_shortened)) { if (Message::contentTooLong($content_shortened)) {
// TRANS: Form validation error displayed when message content is too long. // TRANS: Form validation error displayed when message content is too long.

View File

@ -154,7 +154,7 @@ class NewnoticeAction extends Action
return; return;
} }
$content_shortened = common_shorten_links($content); $content_shortened = $user->shortenLinks($content);
if (Notice::contentTooLong($content_shortened)) { if (Notice::contentTooLong($content_shortened)) {
// TRANS: Client error displayed when the parameter "status" is missing. // TRANS: Client error displayed when the parameter "status" is missing.
// TRANS: %d is the maximum number of character for a notice. // TRANS: %d is the maximum number of character for a notice.

View File

@ -187,13 +187,14 @@ class File_redirection extends Memcached_DataObject
* may be saved. * may be saved.
* *
* @param string $long_url * @param string $long_url
* @param User $user whose shortening options to use; defaults to the current web session user
* @return string * @return string
*/ */
function makeShort($long_url) { function makeShort($long_url, $user=null) {
$canon = File_redirection::_canonUrl($long_url); $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? // Did we get one? Is it shorter?
if (!empty($short_url) && mb_strlen($short_url) < mb_strlen($long_url)) { 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) { function _userMakeShort($long_url, User $user=null) {
$short_url = common_shorten_url($long_url); $short_url = common_shorten_url($long_url, $user);
if (!empty($short_url) && $short_url != $long_url) { if (!empty($short_url) && $short_url != $long_url) {
$short_url = (string)$short_url; $short_url = (string)$short_url;
// store it // store it

View File

@ -45,11 +45,18 @@ class Message extends Memcached_DataObject
throw new ClientException(_('You are banned from sending direct messages.')); throw new ClientException(_('You are banned from sending direct messages.'));
} }
$user = User::staticGet('id', $sender->id);
$msg = new Message(); $msg = new Message();
$msg->from_profile = $from; $msg->from_profile = $from;
$msg->to_profile = $to; $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->rendered = common_render_text($content);
$msg->created = common_sql_now(); $msg->created = common_sql_now();
$msg->source = $source; $msg->source = $source;

View File

@ -256,9 +256,14 @@ class Notice extends Memcached_DataObject
$is_local = Notice::LOCAL_PUBLIC; $is_local = Notice::LOCAL_PUBLIC;
} }
$profile = Profile::staticGet($profile_id); $profile = Profile::staticGet('id', $profile_id);
$user = User::staticGet('id', $profile_id);
$final = common_shorten_links($content); 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)) { if (Notice::contentTooLong($final)) {
// TRANS: Client exception thrown if a notice contains too many characters. // TRANS: Client exception thrown if a notice contains too many characters.
@ -502,8 +507,7 @@ class Notice extends Memcached_DataObject
/** /**
* @private callback * @private callback
*/ */
function saveUrl($data) { function saveUrl($url, $notice_id) {
list($url, $notice_id) = $data;
File::processNew($url, $notice_id); File::processNew($url, $notice_id);
} }

View File

@ -927,4 +927,23 @@ class User extends Memcached_DataObject
throw new ServerException(_('Single-user mode code called when not enabled.')); 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);
}
} }

View File

@ -479,7 +479,7 @@ class MessageCommand extends Command
return; return;
} }
$this->text = common_shorten_links($this->text); $this->text = $this->user->shortenLinks($this->text);
if (Message::contentTooLong($this->text)) { if (Message::contentTooLong($this->text)) {
// XXX: i18n. Needs plural support. // XXX: i18n. Needs plural support.
@ -582,7 +582,7 @@ class ReplyCommand extends Command
return; return;
} }
$this->text = common_shorten_links($this->text); $this->text = $this->user->shortenLinks($this->text);
if (Notice::contentTooLong($this->text)) { if (Notice::contentTooLong($this->text)) {
// XXX: i18n. Needs plural support. // XXX: i18n. Needs plural support.

View File

@ -55,7 +55,7 @@ class MailHandler
return true; return true;
} }
$msg = $this->cleanup_msg($msg); $msg = $this->cleanup_msg($msg);
$msg = common_shorten_links($msg); $msg = $user->shortenLinks($msg);
if (Notice::contentTooLong($msg)) { if (Notice::contentTooLong($msg)) {
$this->error($from, sprintf(_('That\'s too long. Maximum notice size is %d character.', $this->error($from, sprintf(_('That\'s too long. Maximum notice size is %d character.',
'That\'s too long. Maximum notice size is %d characters.', 'That\'s too long. Maximum notice size is %d characters.',

View File

@ -789,7 +789,14 @@ function common_render_text($text)
return $r; 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 // Start off with a regex
$regex = '#'. $regex = '#'.
'(?:^|[\s\<\>\(\)\[\]\{\}\\\'\\\";]+)(?![\@\!\#])'. '(?:^|[\s\<\>\(\)\[\]\{\}\\\'\\\";]+)(?![\@\!\#])'.
@ -830,10 +837,21 @@ function common_replace_urls_callback($text, $callback, $notice_id = null) {
'#ixu'; '#ixu';
//preg_match_all($regex,$text,$matches); //preg_match_all($regex,$text,$matches);
//print_r($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]; $url=$matches[1];
$left = strpos($matches[0],$url); $left = strpos($matches[0],$url);
$right = $left+strlen($url); $right = $left+strlen($url);
@ -876,11 +894,7 @@ function callback_helper($matches, $callback, $notice_id) {
} }
}while($original_url!=$url); }while($original_url!=$url);
if(empty($notice_id)){ $result = call_user_func_array($callback, array($url, $arg));
$result = call_user_func_array($callback, array($url));
}else{
$result = call_user_func_array($callback, array(array($url,$notice_id)) );
}
return substr($matches[0],0,$left) . $result . substr($matches[0],$right); 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); 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(); $maxLength = Notice::maxContent();
if (!$always && ($maxLength == 0 || mb_strlen($text) <= $maxLength)) return $text; 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. * Length is not considered.
* *
* @param string $long_url * @param string $long_url
* @param User $user to specify a particular user's options
* @return string may return the original URL if shortening failed * @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 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); $long_url = trim($long_url);
$user = common_current_user(); if (empty($user)) {
// Current web session
$user = common_current_user();
}
if (empty($user)) { if (empty($user)) {
// common current user does not find a user when called from the XMPP daemon // 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 // therefore we'll set one here fix, so that XMPP given URLs may be shortened

View File

@ -398,7 +398,7 @@ class XmppManager extends IoManager
function add_notice(&$user, &$pl) function add_notice(&$user, &$pl)
{ {
$body = trim($pl['body']); $body = trim($pl['body']);
$content_shortened = common_shorten_links($body); $content_shortened = $user->shortenLinks($body);
if (Notice::contentTooLong($content_shortened)) { if (Notice::contentTooLong($content_shortened)) {
$from = jabber_normalize_jid($pl['from']); $from = jabber_normalize_jid($pl['from']);
// TRANS: Response to XMPP source when it sent too long a message. // TRANS: Response to XMPP source when it sent too long a message.

View File

@ -370,7 +370,7 @@ class FacebookAction extends Action
$this->showPage(_m('No notice content!')); $this->showPage(_m('No notice content!'));
return; return;
} else { } else {
$content_shortened = common_shorten_links($content); $content_shortened = $user->shortenLinks($content);
if (Notice::contentTooLong($content_shortened)) { if (Notice::contentTooLong($content_shortened)) {
// @todo FIXME: i18n: Needs plural. // @todo FIXME: i18n: Needs plural.

View File

@ -219,7 +219,7 @@ function postNote($user, $activity)
$rendered = purify($sourceContent); $rendered = purify($sourceContent);
$content = html_entity_decode(strip_tags($rendered), ENT_QUOTES, 'UTF-8'); $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, $options = array('is_local' => Notice::LOCAL_PUBLIC,
'uri' => $sourceUri, 'uri' => $sourceUri,