diff --git a/actions/apigroupcreate.php b/actions/apigroupcreate.php index 54875a7188..d01504bc80 100644 --- a/actions/apigroupcreate.php +++ b/actions/apigroupcreate.php @@ -73,7 +73,7 @@ class ApiGroupCreateAction extends ApiAuthAction $this->user = $this->auth_user; - $this->nickname = $this->arg('nickname'); + $this->nickname = Nickname::normalize($this->arg('nickname')); $this->fullname = $this->arg('full_name'); $this->homepage = $this->arg('homepage'); $this->description = $this->arg('description'); @@ -150,26 +150,7 @@ class ApiGroupCreateAction extends ApiAuthAction */ function validateParams() { - $valid = Validate::string( - $this->nickname, array( - 'min_length' => 1, - 'max_length' => 64, - 'format' => NICKNAME_FMT - ) - ); - - if (!$valid) { - $this->clientError( - // TRANS: Validation error in form for group creation. - _( - 'Nickname must have only lowercase letters ' . - 'and numbers and no spaces.' - ), - 403, - $this->format - ); - return false; - } elseif ($this->groupNicknameExists($this->nickname)) { + if ($this->groupNicknameExists($this->nickname)) { $this->clientError( // TRANS: Client error trying to create a group with a nickname this is already in use. _('Nickname already in use. Try another one.'), @@ -265,15 +246,7 @@ class ApiGroupCreateAction extends ApiAuthAction foreach ($this->aliases as $alias) { - $valid = Validate::string( - $alias, array( - 'min_length' => 1, - 'max_length' => 64, - 'format' => NICKNAME_FMT - ) - ); - - if (!$valid) { + if (!Nickname::isValid($alias)) { $this->clientError( // TRANS: Client error shown when providing an invalid alias during group creation. // TRANS: %s is the invalid alias. diff --git a/actions/editgroup.php b/actions/editgroup.php index 4d3af34c7b..ab4dbb2836 100644 --- a/actions/editgroup.php +++ b/actions/editgroup.php @@ -177,21 +177,14 @@ class EditgroupAction extends GroupDesignAction return; } - $nickname = common_canonical_nickname($this->trimmed('nickname')); + $nickname = Nickname::normalize($this->trimmed('nickname')); $fullname = $this->trimmed('fullname'); $homepage = $this->trimmed('homepage'); $description = $this->trimmed('description'); $location = $this->trimmed('location'); $aliasstring = $this->trimmed('aliases'); - if (!Validate::string($nickname, array('min_length' => 1, - 'max_length' => 64, - 'format' => NICKNAME_FMT))) { - // TRANS: Group edit form validation error. - $this->showForm(_('Nickname must have only lowercase letters '. - 'and numbers and no spaces.')); - return; - } else if ($this->nicknameExists($nickname)) { + if ($this->nicknameExists($nickname)) { // TRANS: Group edit form validation error. $this->showForm(_('Nickname already in use. Try another one.')); return; @@ -241,9 +234,7 @@ class EditgroupAction extends GroupDesignAction } foreach ($aliases as $alias) { - if (!Validate::string($alias, array('min_length' => 1, - 'max_length' => 64, - 'format' => NICKNAME_FMT))) { + if (!Nickname::isValid($alias)) { // TRANS: Group edit form validation error. $this->showForm(sprintf(_('Invalid alias: "%s"'), $alias)); return; diff --git a/actions/newgroup.php b/actions/newgroup.php index e0e7978c32..95af6415e5 100644 --- a/actions/newgroup.php +++ b/actions/newgroup.php @@ -113,21 +113,18 @@ class NewgroupAction extends Action function trySave() { - $nickname = $this->trimmed('nickname'); + try { + $nickname = Nickname::normalize($this->trimmed('nickname')); + } catch (NicknameException $e) { + $this->showForm($e->getMessage()); + } $fullname = $this->trimmed('fullname'); $homepage = $this->trimmed('homepage'); $description = $this->trimmed('description'); $location = $this->trimmed('location'); $aliasstring = $this->trimmed('aliases'); - if (!Validate::string($nickname, array('min_length' => 1, - 'max_length' => 64, - 'format' => NICKNAME_FMT))) { - // TRANS: Group create form validation error. - $this->showForm(_('Nickname must have only lowercase letters '. - 'and numbers and no spaces.')); - return; - } else if ($this->nicknameExists($nickname)) { + if ($this->nicknameExists($nickname)) { // TRANS: Group create form validation error. $this->showForm(_('Nickname already in use. Try another one.')); return; @@ -177,9 +174,7 @@ class NewgroupAction extends Action } foreach ($aliases as $alias) { - if (!Validate::string($alias, array('min_length' => 1, - 'max_length' => 64, - 'format' => NICKNAME_FMT))) { + if (!Nickname::isValid($alias)) { // TRANS: Group create form validation error. $this->showForm(sprintf(_('Invalid alias: "%s"'), $alias)); return; diff --git a/actions/profilesettings.php b/actions/profilesettings.php index e1a0f8b6d0..28b1d20f34 100644 --- a/actions/profilesettings.php +++ b/actions/profilesettings.php @@ -225,7 +225,13 @@ class ProfilesettingsAction extends AccountSettingsAction if (Event::handle('StartProfileSaveForm', array($this))) { - $nickname = $this->trimmed('nickname'); + try { + $nickname = Nickname::normalize($this->trimmed('nickname')); + } catch (NicknameException $e) { + $this->showForm($e->getMessage()); + return; + } + $fullname = $this->trimmed('fullname'); $homepage = $this->trimmed('homepage'); $bio = $this->trimmed('bio'); @@ -236,13 +242,7 @@ class ProfilesettingsAction extends AccountSettingsAction $tagstring = $this->trimmed('tags'); // Some validation - if (!Validate::string($nickname, array('min_length' => 1, - 'max_length' => 64, - 'format' => NICKNAME_FMT))) { - // TRANS: Validation error in form for profile settings. - $this->showForm(_('Nickname must have only lowercase letters and numbers and no spaces.')); - return; - } else if (!User::allowed_nickname($nickname)) { + if (!User::allowed_nickname($nickname)) { // TRANS: Validation error in form for profile settings. $this->showForm(_('Not a valid nickname.')); return; diff --git a/actions/register.php b/actions/register.php index 3ae3f56f60..5d91aef70e 100644 --- a/actions/register.php +++ b/actions/register.php @@ -198,7 +198,11 @@ class RegisterAction extends Action } // Input scrubbing - $nickname = common_canonical_nickname($nickname); + try { + $nickname = Nickname::normalize($nickname); + } catch (NicknameException $e) { + $this->showForm($e->getMessage()); + } $email = common_canonical_email($email); if (!$this->boolean('license')) { @@ -206,11 +210,6 @@ class RegisterAction extends Action 'agree to the license.')); } else if ($email && !Validate::email($email, common_config('email', 'check_domain'))) { $this->showForm(_('Not a valid email address.')); - } else if (!Validate::string($nickname, array('min_length' => 1, - 'max_length' => 64, - 'format' => NICKNAME_FMT))) { - $this->showForm(_('Nickname must have only lowercase letters '. - 'and numbers and no spaces.')); } else if ($this->nicknameExists($nickname)) { $this->showForm(_('Nickname already in use. Try another one.')); } else if (!User::allowed_nickname($nickname)) { diff --git a/classes/User.php b/classes/User.php index 964bc3e7f3..92180a9fbc 100644 --- a/classes/User.php +++ b/classes/User.php @@ -116,6 +116,16 @@ class User extends Memcached_DataObject return $result; } + /** + * Check whether the given nickname is potentially usable, or if it's + * excluded by any blacklists on this system. + * + * WARNING: INPUT IS NOT VALIDATED OR NORMALIZED. NON-NORMALIZED INPUT + * OR INVALID INPUT MAY LEAD TO FALSE RESULTS. + * + * @param string $nickname + * @return boolean true if clear, false if blacklisted + */ static function allowed_nickname($nickname) { // XXX: should already be validated for size, content, etc. diff --git a/lib/command.php b/lib/command.php index ae69f04a13..2a8075e7ba 100644 --- a/lib/command.php +++ b/lib/command.php @@ -139,7 +139,7 @@ class Command { $user = null; if (Event::handle('StartCommandGetUser', array($this, $arg, &$user))) { - $user = User::staticGet('nickname', $arg); + $user = User::staticGet('nickname', Nickname::normalize($arg)); } Event::handle('EndCommandGetUser', array($this, $arg, &$user)); if (!$user){ diff --git a/lib/common.php b/lib/common.php index cd4fbfb15a..d891807185 100644 --- a/lib/common.php +++ b/lib/common.php @@ -117,6 +117,17 @@ require_once 'markdown.php'; // XXX: other formats here +/** + * Avoid the NICKNAME_FMT constant; use the Nickname class instead. + * + * Nickname::DISPLAY_FMT is more suitable for inserting into regexes; + * note that it includes the [] and repeating bits, so should be wrapped + * directly in a capture paren usually. + * + * For validation, use Nickname::validate() etc. + * + * @deprecated + */ define('NICKNAME_FMT', VALIDATE_NUM.VALIDATE_ALPHA_LOWER); require_once INSTALLDIR.'/lib/util.php'; diff --git a/lib/nickname.php b/lib/nickname.php new file mode 100644 index 0000000000..48269f3b9f --- /dev/null +++ b/lib/nickname.php @@ -0,0 +1,176 @@ +. + */ + +class Nickname +{ + /** + * Regex fragment for pulling an arbitrarily-formated nickname. + * + * Not guaranteed to be valid after normalization; run the string through + * Nickname::normalize() to get the canonical form, or Nickname::validate() + * if you just need to check if it's properly formatted. + * + * This and CANONICAL_FMT replace the old NICKNAME_FMT, but be aware + * that these should not be enclosed in []s. + */ + const DISPLAY_FMT = '[0-9a-zA-Z_]+'; + + /** + * Regex fragment for checking a canonical nickname. + * + * Any non-matching string is not a valid canonical/normalized nickname. + * Matching strings are valid and canonical form, but may still be + * unavailable for registration due to blacklisting et. + * + * Only the canonical forms should be stored as keys in the database; + * there are multiple possible denormalized forms for each valid + * canonical-form name. + * + * This and DISPLAY_FMT replace the old NICKNAME_FMT, but be aware + * that these should not be enclosed in []s. + */ + const CANONICAL_FMT = '[0-9a-z]{1,64}'; + + /** + * Maximum number of characters in a canonical-form nickname. + */ + const MAX_LEN = 64; + + /** + * Nice simple check of whether the given string is a valid input nickname, + * which can be normalized into an internally canonical form. + * + * Note that valid nicknames may be in use or reserved. + * + * @param string $str + * @return boolean + */ + public static function validate($str) + { + try { + self::normalize($str); + return true; + } catch (NicknameException $e) { + return false; + } + } + + /** + * Validate an input nickname string, and normalize it to its canonical form. + * The canonical form will be returned, or an exception thrown if invalid. + * + * @param string $str + * @return string Normalized canonical form of $str + * + * @throws NicknameException (base class) + * @throws NicknameInvalidException + * @throws NicknameEmptyException + * @throws NicknameTooLongException + */ + public static function normalize($str) + { + $str = trim($str); + $str = str_replace('_', '', $str); + $str = mb_strtolower($str); + + $len = mb_strlen($str); + if ($len < 1) { + throw new NicknameEmptyException(); + } else if ($len > self::MAX_LEN) { + throw new NicknameTooLongException(); + } + if (!self::isCanonical($str)) { + throw new NicknameInvalidException(); + } + + return $str; + } + + /** + * Is the given string a valid canonical nickname form? + * + * @param string $str + * @return boolean + */ + public static function isCanonical($str) + { + return preg_match('/^(?:' . self::CANONICAL_FMT . ')$/', $str); + } +} + +class NicknameException extends ClientException +{ + function __construct($msg=null, $code=400) + { + if ($msg === null) { + $msg = $this->defaultMessage(); + } + parent::__construct($msg, $code); + } + + /** + * Default localized message for this type of exception. + * @return string + */ + protected function defaultMessage() + { + return null; + } +} + +class NicknameInvalidException extends NicknameException { + /** + * Default localized message for this type of exception. + * @return string + */ + protected function defaultMessage() + { + // TRANS: Validation error in form for registration, profile and group settings, etc. + return _('Nickname must have only lowercase letters and numbers and no spaces.'); + } +} + +class NicknameEmptyException extends NicknameException +{ + /** + * Default localized message for this type of exception. + * @return string + */ + protected function defaultMessage() + { + // TRANS: Validation error in form for registration, profile and group settings, etc. + return _('Nickname cannot be empty.'); + } +} + +class NicknameTooLongException extends NicknameInvalidException +{ + /** + * Default localized message for this type of exception. + * @return string + */ + protected function defaultMessage() + { + // TRANS: Validation error in form for registration, profile and group settings, etc. + return sprintf(_m('Nickname cannot be more than %d character long.', + 'Nickname cannot be more than %d characters long.', + Nickname::MAX_LEN), + Nickname::MAX_LEN); + } +} diff --git a/lib/router.php b/lib/router.php index 47357ca085..7d675bb547 100644 --- a/lib/router.php +++ b/lib/router.php @@ -223,10 +223,10 @@ class Router $m->connect('notice/new', array('action' => 'newnotice')); $m->connect('notice/new?replyto=:replyto', array('action' => 'newnotice'), - array('replyto' => '[A-Za-z0-9_-]+')); + array('replyto' => Nickname::DISPLAY_FMT)); $m->connect('notice/new?replyto=:replyto&inreplyto=:inreplyto', array('action' => 'newnotice'), - array('replyto' => '[A-Za-z0-9_-]+'), + array('replyto' => Nickname::DISPLAY_FMT), array('inreplyto' => '[0-9]+')); $m->connect('notice/:notice/file', @@ -250,7 +250,7 @@ class Router array('id' => '[0-9]+')); $m->connect('message/new', array('action' => 'newmessage')); - $m->connect('message/new?to=:to', array('action' => 'newmessage'), array('to' => '[A-Za-z0-9_-]+')); + $m->connect('message/new?to=:to', array('action' => 'newmessage'), array('to' => Nickname::DISPLAY_FMT)); $m->connect('message/:message', array('action' => 'showmessage'), array('message' => '[0-9]+')); @@ -281,7 +281,7 @@ class Router foreach (array('edit', 'join', 'leave', 'delete') as $v) { $m->connect('group/:nickname/'.$v, array('action' => $v.'group'), - array('nickname' => '[a-zA-Z0-9]+')); + array('nickname' => Nickname::DISPLAY_FMT)); $m->connect('group/:id/id/'.$v, array('action' => $v.'group'), array('id' => '[0-9]+')); @@ -290,20 +290,20 @@ class Router foreach (array('members', 'logo', 'rss', 'designsettings') as $n) { $m->connect('group/:nickname/'.$n, array('action' => 'group'.$n), - array('nickname' => '[a-zA-Z0-9]+')); + array('nickname' => Nickname::DISPLAY_FMT)); } $m->connect('group/:nickname/foaf', array('action' => 'foafgroup'), - array('nickname' => '[a-zA-Z0-9]+')); + array('nickname' => Nickname::DISPLAY_FMT)); $m->connect('group/:nickname/blocked', array('action' => 'blockedfromgroup'), - array('nickname' => '[a-zA-Z0-9]+')); + array('nickname' => Nickname::DISPLAY_FMT)); $m->connect('group/:nickname/makeadmin', array('action' => 'makeadmin'), - array('nickname' => '[a-zA-Z0-9]+')); + array('nickname' => Nickname::DISPLAY_FMT)); $m->connect('group/:id/id', array('action' => 'groupbyid'), @@ -311,7 +311,7 @@ class Router $m->connect('group/:nickname', array('action' => 'showgroup'), - array('nickname' => '[a-zA-Z0-9]+')); + array('nickname' => Nickname::DISPLAY_FMT)); $m->connect('group/', array('action' => 'groups')); $m->connect('group', array('action' => 'groups')); @@ -332,7 +332,7 @@ class Router $m->connect('api/statuses/friends_timeline/:id.:format', array('action' => 'ApiTimelineFriends', - 'id' => '[a-zA-Z0-9]+', + 'id' => Nickname::DISPLAY_FMT, 'format' => '(xml|json|rss|atom)')); $m->connect('api/statuses/home_timeline.:format', @@ -341,7 +341,7 @@ class Router $m->connect('api/statuses/home_timeline/:id.:format', array('action' => 'ApiTimelineHome', - 'id' => '[a-zA-Z0-9]+', + 'id' => Nickname::DISPLAY_FMT, 'format' => '(xml|json|rss|atom)')); $m->connect('api/statuses/user_timeline.:format', @@ -350,7 +350,7 @@ class Router $m->connect('api/statuses/user_timeline/:id.:format', array('action' => 'ApiTimelineUser', - 'id' => '[a-zA-Z0-9]+', + 'id' => Nickname::DISPLAY_FMT, 'format' => '(xml|json|rss|atom)')); $m->connect('api/statuses/mentions.:format', @@ -359,7 +359,7 @@ class Router $m->connect('api/statuses/mentions/:id.:format', array('action' => 'ApiTimelineMentions', - 'id' => '[a-zA-Z0-9]+', + 'id' => Nickname::DISPLAY_FMT, 'format' => '(xml|json|rss|atom)')); $m->connect('api/statuses/replies.:format', @@ -368,7 +368,7 @@ class Router $m->connect('api/statuses/replies/:id.:format', array('action' => 'ApiTimelineMentions', - 'id' => '[a-zA-Z0-9]+', + 'id' => Nickname::DISPLAY_FMT, 'format' => '(xml|json|rss|atom)')); $m->connect('api/statuses/retweeted_by_me.:format', @@ -389,7 +389,7 @@ class Router $m->connect('api/statuses/friends/:id.:format', array('action' => 'ApiUserFriends', - 'id' => '[a-zA-Z0-9]+', + 'id' => Nickname::DISPLAY_FMT, 'format' => '(xml|json)')); $m->connect('api/statuses/followers.:format', @@ -398,7 +398,7 @@ class Router $m->connect('api/statuses/followers/:id.:format', array('action' => 'ApiUserFollowers', - 'id' => '[a-zA-Z0-9]+', + 'id' => Nickname::DISPLAY_FMT, 'format' => '(xml|json)')); $m->connect('api/statuses/show.:format', @@ -441,7 +441,7 @@ class Router $m->connect('api/users/show/:id.:format', array('action' => 'ApiUserShow', - 'id' => '[a-zA-Z0-9]+', + 'id' => Nickname::DISPLAY_FMT, 'format' => '(xml|json)')); // direct messages @@ -479,12 +479,12 @@ class Router $m->connect('api/friendships/create/:id.:format', array('action' => 'ApiFriendshipsCreate', - 'id' => '[a-zA-Z0-9]+', + 'id' => Nickname::DISPLAY_FMT, 'format' => '(xml|json)')); $m->connect('api/friendships/destroy/:id.:format', array('action' => 'ApiFriendshipsDestroy', - 'id' => '[a-zA-Z0-9]+', + 'id' => Nickname::DISPLAY_FMT, 'format' => '(xml|json)')); // Social graph @@ -541,17 +541,17 @@ class Router $m->connect('api/favorites/:id.:format', array('action' => 'ApiTimelineFavorites', - 'id' => '[a-zA-Z0-9]+', + 'id' => Nickname::DISPLAY_FMT, 'format' => '(xml|json|rss|atom)')); $m->connect('api/favorites/create/:id.:format', array('action' => 'ApiFavoriteCreate', - 'id' => '[a-zA-Z0-9]+', + 'id' => Nickname::DISPLAY_FMT, 'format' => '(xml|json)')); $m->connect('api/favorites/destroy/:id.:format', array('action' => 'ApiFavoriteDestroy', - 'id' => '[a-zA-Z0-9]+', + 'id' => Nickname::DISPLAY_FMT, 'format' => '(xml|json)')); // blocks @@ -561,7 +561,7 @@ class Router $m->connect('api/blocks/create/:id.:format', array('action' => 'ApiBlockCreate', - 'id' => '[a-zA-Z0-9]+', + 'id' => Nickname::DISPLAY_FMT, 'format' => '(xml|json)')); $m->connect('api/blocks/destroy.:format', @@ -570,7 +570,7 @@ class Router $m->connect('api/blocks/destroy/:id.:format', array('action' => 'ApiBlockDestroy', - 'id' => '[a-zA-Z0-9]+', + 'id' => Nickname::DISPLAY_FMT, 'format' => '(xml|json)')); // help @@ -606,7 +606,7 @@ class Router $m->connect('api/statusnet/groups/timeline/:id.:format', array('action' => 'ApiTimelineGroup', - 'id' => '[a-zA-Z0-9]+', + 'id' => Nickname::DISPLAY_FMT, 'format' => '(xml|json|rss|atom)')); $m->connect('api/statusnet/groups/show.:format', @@ -615,12 +615,12 @@ class Router $m->connect('api/statusnet/groups/show/:id.:format', array('action' => 'ApiGroupShow', - 'id' => '[a-zA-Z0-9]+', + 'id' => Nickname::DISPLAY_FMT, 'format' => '(xml|json)')); $m->connect('api/statusnet/groups/join.:format', array('action' => 'ApiGroupJoin', - 'id' => '[a-zA-Z0-9]+', + 'id' => Nickname::DISPLAY_FMT, 'format' => '(xml|json)')); $m->connect('api/statusnet/groups/join/:id.:format', @@ -629,7 +629,7 @@ class Router $m->connect('api/statusnet/groups/leave.:format', array('action' => 'ApiGroupLeave', - 'id' => '[a-zA-Z0-9]+', + 'id' => Nickname::DISPLAY_FMT, 'format' => '(xml|json)')); $m->connect('api/statusnet/groups/leave/:id.:format', @@ -646,7 +646,7 @@ class Router $m->connect('api/statusnet/groups/list/:id.:format', array('action' => 'ApiGroupList', - 'id' => '[a-zA-Z0-9]+', + 'id' => Nickname::DISPLAY_FMT, 'format' => '(xml|json|rss|atom)')); $m->connect('api/statusnet/groups/list_all.:format', @@ -659,7 +659,7 @@ class Router $m->connect('api/statusnet/groups/membership/:id.:format', array('action' => 'ApiGroupMembership', - 'id' => '[a-zA-Z0-9]+', + 'id' => Nickname::DISPLAY_FMT, 'format' => '(xml|json)')); $m->connect('api/statusnet/groups/create.:format', @@ -692,7 +692,7 @@ class Router $m->connect('api/statusnet/app/service/:id.xml', array('action' => 'ApiAtomService', - 'id' => '[a-zA-Z0-9]+')); + 'id' => Nickname::DISPLAY_FMT)); $m->connect('api/statusnet/app/service.xml', array('action' => 'ApiAtomService')); @@ -789,54 +789,54 @@ class Router 'replies', 'inbox', 'outbox', 'microsummary', 'hcard') as $a) { $m->connect(':nickname/'.$a, array('action' => $a), - array('nickname' => '[a-zA-Z0-9]{1,64}')); + array('nickname' => Nickname::DISPLAY_FMT)); } foreach (array('subscriptions', 'subscribers') as $a) { $m->connect(':nickname/'.$a.'/:tag', array('action' => $a), array('tag' => '[a-zA-Z0-9]+', - 'nickname' => '[a-zA-Z0-9]{1,64}')); + 'nickname' => Nickname::DISPLAY_FMT)); } foreach (array('rss', 'groups') as $a) { $m->connect(':nickname/'.$a, array('action' => 'user'.$a), - array('nickname' => '[a-zA-Z0-9]{1,64}')); + array('nickname' => Nickname::DISPLAY_FMT)); } foreach (array('all', 'replies', 'favorites') as $a) { $m->connect(':nickname/'.$a.'/rss', array('action' => $a.'rss'), - array('nickname' => '[a-zA-Z0-9]{1,64}')); + array('nickname' => Nickname::DISPLAY_FMT)); } $m->connect(':nickname/favorites', array('action' => 'showfavorites'), - array('nickname' => '[a-zA-Z0-9]{1,64}')); + array('nickname' => Nickname::DISPLAY_FMT)); $m->connect(':nickname/avatar/:size', array('action' => 'avatarbynickname'), array('size' => '(original|96|48|24)', - 'nickname' => '[a-zA-Z0-9]{1,64}')); + 'nickname' => Nickname::DISPLAY_FMT)); $m->connect(':nickname/tag/:tag/rss', array('action' => 'userrss'), - array('nickname' => '[a-zA-Z0-9]{1,64}'), + array('nickname' => Nickname::DISPLAY_FMT), array('tag' => '[\pL\pN_\-\.]{1,64}')); $m->connect(':nickname/tag/:tag', array('action' => 'showstream'), - array('nickname' => '[a-zA-Z0-9]{1,64}'), + array('nickname' => Nickname::DISPLAY_FMT), array('tag' => '[\pL\pN_\-\.]{1,64}')); $m->connect(':nickname/rsd.xml', array('action' => 'rsd'), - array('nickname' => '[a-zA-Z0-9]{1,64}')); + array('nickname' => Nickname::DISPLAY_FMT)); $m->connect(':nickname', array('action' => 'showstream'), - array('nickname' => '[a-zA-Z0-9]{1,64}')); + array('nickname' => Nickname::DISPLAY_FMT)); } // user stuff diff --git a/lib/util.php b/lib/util.php index ce5da1cd81..42762b22fb 100644 --- a/lib/util.php +++ b/lib/util.php @@ -517,14 +517,29 @@ function common_user_cache_hash($user=false) } } -// get canonical version of nickname for comparison +/** + * get canonical version of nickname for comparison + * + * @param string $nickname + * @return string + * + * @throws NicknameException on invalid input + * @deprecated call Nickname::normalize() directly. + */ function common_canonical_nickname($nickname) { - // XXX: UTF-8 canonicalization (like combining chars) - return strtolower($nickname); + return Nickname::normalize($nickname); } -// get canonical version of email for comparison +/** + * get canonical version of email for comparison + * + * @fixme actually normalize + * @fixme reject invalid input + * + * @param string $email + * @return string + */ function common_canonical_email($email) { // XXX: canonicalize UTF-8 @@ -532,15 +547,33 @@ function common_canonical_email($email) return $email; } +/** + * Partial notice markup rendering step: build links to !group references. + * + * @param string $text partially rendered HTML + * @param Notice $notice in whose context we're working + * @return string partially rendered HTML + */ function common_render_content($text, $notice) { $r = common_render_text($text); $id = $notice->profile_id; $r = common_linkify_mentions($r, $notice); - $r = preg_replace('/(^|[\s\.\,\:\;]+)!([A-Za-z0-9]{1,64})/e', "'\\1!'.common_group_link($id, '\\2')", $r); + $r = preg_replace('/(^|[\s\.\,\:\;]+)!(' . Nickname::DISPLAY_FMT . ')/e', + "'\\1!'.common_group_link($id, '\\2')", $r); return $r; } +/** + * Finds @-mentions within the partially-rendered text section and + * turns them into live links. + * + * Should generally not be called except from common_render_content(). + * + * @param string $text partially-rendered HTML + * @param Notice $notice in-progress or complete Notice object for context + * @return string partially-rendered HTML + */ function common_linkify_mentions($text, $notice) { $mentions = common_find_mentions($text, $notice); @@ -597,6 +630,21 @@ function common_linkify_mention($mention) return $output; } +/** + * Find @-mentions in the given text, using the given notice object as context. + * References will be resolved with common_relative_profile() against the user + * who posted the notice. + * + * Note the return data format is internal, to be used for building links and + * such. Should not be used directly; rather, call common_linkify_mentions(). + * + * @param string $text + * @param Notice $notice notice in whose context we're building links + * + * @return array + * + * @access private + */ function common_find_mentions($text, $notice) { $mentions = array(); @@ -631,20 +679,15 @@ function common_find_mentions($text, $notice) } } - preg_match_all('/^T ([A-Z0-9]{1,64}) /', - $text, - $tmatches, - PREG_OFFSET_CAPTURE); - - preg_match_all('/(?:^|\s+)@(['.NICKNAME_FMT.']{1,64})/', - $text, - $atmatches, - PREG_OFFSET_CAPTURE); - - $matches = array_merge($tmatches[1], $atmatches[1]); + $matches = common_find_mentions_raw($text); foreach ($matches as $match) { - $nickname = common_canonical_nickname($match[0]); + try { + $nickname = Nickname::normalize($match[0]); + } catch (NicknameException $e) { + // Bogus match? Drop it. + continue; + } // Try to get a profile for this nickname. // Start with conversation context, then go to @@ -710,6 +753,31 @@ function common_find_mentions($text, $notice) return $mentions; } +/** + * Does the actual regex pulls to find @-mentions in text. + * Should generally not be called directly; for use in common_find_mentions. + * + * @param string $text + * @return array of PCRE match arrays + */ +function common_find_mentions_raw($text) +{ + $tmatches = array(); + preg_match_all('/^T (' . Nickname::DISPLAY_FMT . ') /', + $text, + $tmatches, + PREG_OFFSET_CAPTURE); + + $atmatches = array(); + preg_match_all('/(?:^|\s+)@(' . Nickname::DISPLAY_FMT . ')\b/', + $text, + $atmatches, + PREG_OFFSET_CAPTURE); + + $matches = array_merge($tmatches[1], $atmatches[1]); + return $matches; +} + function common_render_text($text) { $r = htmlspecialchars($text); @@ -1004,6 +1072,13 @@ function common_valid_profile_tag($str) return preg_match('/^[A-Za-z0-9_\-\.]{1,64}$/', $str); } +/** + * + * @param $sender_id + * @param $nickname + * @return + * @access private + */ function common_group_link($sender_id, $nickname) { $sender = Profile::staticGet($sender_id); @@ -1026,13 +1101,37 @@ function common_group_link($sender_id, $nickname) } } +/** + * Resolve an ambiguous profile nickname reference, checking in following order: + * - profiles that $sender subscribes to + * - profiles that subscribe to $sender + * - local user profiles + * + * WARNING: does not validate or normalize $nickname -- MUST BE PRE-VALIDATED + * OR THERE MAY BE A RISK OF SQL INJECTION ATTACKS. THIS FUNCTION DOES NOT + * ESCAPE SQL. + * + * @fixme validate input + * @fixme escape SQL + * @fixme fix or remove mystery third parameter + * @fixme is $sender a User or Profile? + * + * @param $sender the user or profile in whose context we're looking + * @param string $nickname validated nickname of + * @param $dt unused mystery parameter; in Notice reply-to handling a timestamp is passed. + * + * @return Profile or null + */ function common_relative_profile($sender, $nickname, $dt=null) { + // Will throw exception on invalid input. + $nickname = Nickname::normalize($nickname); + // Try to find profiles this profile is subscribed to that have this nickname $recipient = new Profile(); // XXX: use a join instead of a subquery - $recipient->whereAdd('EXISTS (SELECT subscribed from subscription where subscriber = '.$sender->id.' and subscribed = id)', 'AND'); - $recipient->whereAdd("nickname = '" . trim($nickname) . "'", 'AND'); + $recipient->whereAdd('EXISTS (SELECT subscribed from subscription where subscriber = '.intval($sender->id).' and subscribed = id)', 'AND'); + $recipient->whereAdd("nickname = '" . $recipient->escape($nickname) . "'", 'AND'); if ($recipient->find(true)) { // XXX: should probably differentiate between profiles with // the same name by date of most recent update @@ -1041,8 +1140,8 @@ function common_relative_profile($sender, $nickname, $dt=null) // Try to find profiles that listen to this profile and that have this nickname $recipient = new Profile(); // XXX: use a join instead of a subquery - $recipient->whereAdd('EXISTS (SELECT subscriber from subscription where subscribed = '.$sender->id.' and subscriber = id)', 'AND'); - $recipient->whereAdd("nickname = '" . trim($nickname) . "'", 'AND'); + $recipient->whereAdd('EXISTS (SELECT subscriber from subscription where subscribed = '.intval($sender->id).' and subscriber = id)', 'AND'); + $recipient->whereAdd("nickname = '" . $recipient->escape($nickname) . "'", 'AND'); if ($recipient->find(true)) { // XXX: should probably differentiate between profiles with // the same name by date of most recent update diff --git a/plugins/Facebook/FBConnectAuth.php b/plugins/Facebook/FBConnectAuth.php index d6d3786261..84d51578f1 100644 --- a/plugins/Facebook/FBConnectAuth.php +++ b/plugins/Facebook/FBConnectAuth.php @@ -257,13 +257,10 @@ class FBConnectauthAction extends Action } } - $nickname = $this->trimmed('newname'); - - if (!Validate::string($nickname, array('min_length' => 1, - 'max_length' => 64, - 'format' => NICKNAME_FMT))) { - $this->showForm(_m('Nickname must have only lowercase letters and numbers and no spaces.')); - return; + try { + $nickname = Nickname::normalize($this->trimmed('newname')); + } catch (NicknameException $e) { + $this->showForm($e->getMessage()); } if (!User::allowed_nickname($nickname)) { @@ -447,9 +444,7 @@ class FBConnectauthAction extends Action function isNewNickname($str) { - if (!Validate::string($str, array('min_length' => 1, - 'max_length' => 64, - 'format' => NICKNAME_FMT))) { + if (!Nickname::isValid($str)) { return false; } if (!User::allowed_nickname($str)) { diff --git a/plugins/FacebookBridge/actions/facebookfinishlogin.php b/plugins/FacebookBridge/actions/facebookfinishlogin.php index 2174c5ad4a..349acd7e22 100644 --- a/plugins/FacebookBridge/actions/facebookfinishlogin.php +++ b/plugins/FacebookBridge/actions/facebookfinishlogin.php @@ -324,12 +324,10 @@ class FacebookfinishloginAction extends Action } } - $nickname = $this->trimmed('newname'); - - if (!Validate::string($nickname, array('min_length' => 1, - 'max_length' => 64, - 'format' => NICKNAME_FMT))) { - $this->showForm(_m('Nickname must have only lowercase letters and numbers and no spaces.')); + try { + $nickname = Nickname::normalize($this->trimmed('newname')); + } catch (NicknameException $e) { + $this->showForm($e->getMessage()); return; } @@ -639,16 +637,7 @@ class FacebookfinishloginAction extends Action */ function isNewNickname($str) { - if ( - !Validate::string( - $str, - array( - 'min_length' => 1, - 'max_length' => 64, - 'format' => NICKNAME_FMT - ) - ) - ) { + if (!Nickname::isValid($str)) { return false; } diff --git a/plugins/Mapstraction/MapstractionPlugin.php b/plugins/Mapstraction/MapstractionPlugin.php index d5261d8bc7..5ad25763e7 100644 --- a/plugins/Mapstraction/MapstractionPlugin.php +++ b/plugins/Mapstraction/MapstractionPlugin.php @@ -67,10 +67,10 @@ class MapstractionPlugin extends Plugin { $m->connect(':nickname/all/map', array('action' => 'allmap'), - array('nickname' => '['.NICKNAME_FMT.']{1,64}')); + array('nickname' => Nickname::DISPLAY_FMT)); $m->connect(':nickname/map', array('action' => 'usermap'), - array('nickname' => '['.NICKNAME_FMT.']{1,64}')); + array('nickname' => Nickname::DISPLAY_FMT)); return true; } diff --git a/plugins/Mapstraction/map.php b/plugins/Mapstraction/map.php index 50ff82b67e..dbba4edd0c 100644 --- a/plugins/Mapstraction/map.php +++ b/plugins/Mapstraction/map.php @@ -53,7 +53,7 @@ class MapAction extends OwnerDesignAction parent::prepare($args); $nickname_arg = $this->arg('nickname'); - $nickname = common_canonical_nickname($nickname_arg); + $nickname = Nickname::normalize($nickname_arg); // Permanent redirect on non-canonical nickname diff --git a/plugins/OpenID/finishopenidlogin.php b/plugins/OpenID/finishopenidlogin.php index 01dd61edb1..86dd1c669b 100644 --- a/plugins/OpenID/finishopenidlogin.php +++ b/plugins/OpenID/finishopenidlogin.php @@ -272,13 +272,10 @@ class FinishopenidloginAction extends Action } } - $nickname = $this->trimmed('newname'); - - if (!Validate::string($nickname, array('min_length' => 1, - 'max_length' => 64, - 'format' => NICKNAME_FMT))) { - // TRANS: OpenID plugin message. The entered new user name did not conform to the requirements. - $this->showForm(_m('Nickname must have only lowercase letters and numbers and no spaces.')); + try { + $nickname = Nickname::validate($this->trimmed('newname')); + } catch (NicknameException $e) { + $this->showForm($e->getMessage()); return; } @@ -463,9 +460,7 @@ class FinishopenidloginAction extends Action function isNewNickname($str) { - if (!Validate::string($str, array('min_length' => 1, - 'max_length' => 64, - 'format' => NICKNAME_FMT))) { + if (!Nickname::isValid($str)) { return false; } if (!User::allowed_nickname($str)) { diff --git a/plugins/TwitterBridge/twitterauthorization.php b/plugins/TwitterBridge/twitterauthorization.php index 931a037230..bbe41bd438 100644 --- a/plugins/TwitterBridge/twitterauthorization.php +++ b/plugins/TwitterBridge/twitterauthorization.php @@ -441,12 +441,10 @@ class TwitterauthorizationAction extends Action } } - $nickname = $this->trimmed('newname'); - - if (!Validate::string($nickname, array('min_length' => 1, - 'max_length' => 64, - 'format' => NICKNAME_FMT))) { - $this->showForm(_m('Nickname must have only lowercase letters and numbers and no spaces.')); + try { + $nickname = Nickname::normalize($this->trimmed('newname')); + } catch (NicknameException $e) { + $this->showForm($e->getMessage()); return; } @@ -619,9 +617,7 @@ class TwitterauthorizationAction extends Action function isNewNickname($str) { - if (!Validate::string($str, array('min_length' => 1, - 'max_length' => 64, - 'format' => NICKNAME_FMT))) { + if (!Nickname::isValid($str)) { return false; } if (!User::allowed_nickname($str)) { diff --git a/tests/NicknameTest.php b/tests/NicknameTest.php new file mode 100644 index 0000000000..f49aeba602 --- /dev/null +++ b/tests/NicknameTest.php @@ -0,0 +1,111 @@ +assertTrue($exception && $exception instanceof $expectedException, + "invalid input '$input' expected to fail with $expectedException, " . + "got " . get_class($exception) . ': ' . $exception->getMessage()); + } else { + $this->assertTrue($normalized == false, + "invalid input '$input' expected to fail"); + } + } else { + $msg = "normalized input nickname '$input' expected to normalize to '$expected', got "; + if ($exception) { + $msg .= get_class($exception) . ': ' . $exception->getMessage(); + } else { + $msg .= "'$normalized'"; + } + $this->assertEquals($expected, $normalized, $msg); + } + } + + /** + * Test on the regex matching used in common_find_mentions + * (testing on the full notice rendering is difficult as it needs + * to be able to pull from global state) + * + * @dataProvider provider + */ + public function testAtReply($input, $expected, $expectedException=null) + { + if ($expected == false) { + // nothing to do + } else { + $text = "@{$input} awesome! :)"; + $matches = common_find_mentions_raw($text); + $this->assertEquals(1, count($matches)); + $this->assertEquals($expected, Nickname::normalize($matches[0][0])); + } + } + + static public function provider() + { + return array( + array('evan', 'evan'), + + // Case and underscore variants + array('Evan', 'evan'), + array('EVAN', 'evan'), + array('ev_an', 'evan'), + array('E__V_an', 'evan'), + array('evan1', 'evan1'), + array('evan_1', 'evan1'), + array('0x20', '0x20'), + array('1234', '1234'), // should this be allowed though? :) + array('12__34', '1234'), + + // Some (currently) invalid chars... + array('^#@&^#@', false, 'NicknameInvalidException'), // all invalid :D + array('ev.an', false, 'NicknameInvalidException'), + array('ev/an', false, 'NicknameInvalidException'), + array('ev an', false, 'NicknameInvalidException'), + array('ev-an', false, 'NicknameInvalidException'), + + // Non-ASCII letters; currently not allowed, in future + // we'll add them at least with conversion to ASCII. + // Not much use until we have storage of display names, + // though. + array('évan', false, 'NicknameInvalidException'), // so far... + array('Évan', false, 'NicknameInvalidException'), // so far... + + // Length checks + array('', false, 'NicknameEmptyException'), + array('___', false, 'NicknameEmptyException'), + array('eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee', 'eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee'), // 64 chars + array('eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee_', 'eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee'), // the _ will be trimmed off, remaining valid + array('eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee', false, 'NicknameTooLongException'), // 65 chars -- too long + ); + } +}