From 2744bdcdb76e5d8affa7c54a583effadb7b1430d Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Wed, 19 Apr 2017 11:37:43 +0200 Subject: [PATCH 1/8] Empty resource would throw exception The "+ Remote" link on your profile page broke because of exception. --- plugins/OStatus/actions/ostatussub.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plugins/OStatus/actions/ostatussub.php b/plugins/OStatus/actions/ostatussub.php index a8039ae565..7531bb6886 100644 --- a/plugins/OStatus/actions/ostatussub.php +++ b/plugins/OStatus/actions/ostatussub.php @@ -242,7 +242,11 @@ class OStatusSubAction extends Action function pullRemoteProfile() { $validate = new Validate(); - $this->profile_uri = Discovery::normalize($this->trimmed('profile')); + try { + $this->profile_uri = Discovery::normalize($this->trimmed('profile')); + } catch (Exception $e) { + $this->profile_uri = null; + } try { if (Discovery::isAcct($this->profile_uri) && $validate->email(mb_substr($this->profile_uri, 5))) { $this->oprofile = Ostatus_profile::ensureWebfinger($this->profile_uri); From 3453521c9c235d8e99d959469aa5c08ca5d6e5fb Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Wed, 19 Apr 2017 11:41:34 +0200 Subject: [PATCH 2/8] Less frightening interface on remote subscription Instead of an error message in a red box about being unable to find the profile, you get the title "Remote subscription" and no error message. --- plugins/OStatus/actions/ostatussub.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugins/OStatus/actions/ostatussub.php b/plugins/OStatus/actions/ostatussub.php index 7531bb6886..919737ba28 100644 --- a/plugins/OStatus/actions/ostatussub.php +++ b/plugins/OStatus/actions/ostatussub.php @@ -245,8 +245,9 @@ class OStatusSubAction extends Action try { $this->profile_uri = Discovery::normalize($this->trimmed('profile')); } catch (Exception $e) { - $this->profile_uri = null; + return false; } + try { if (Discovery::isAcct($this->profile_uri) && $validate->email(mb_substr($this->profile_uri, 5))) { $this->oprofile = Ostatus_profile::ensureWebfinger($this->profile_uri); @@ -391,7 +392,7 @@ class OStatusSubAction extends Action function title() { // TRANS: Page title for OStatus remote subscription form. - return _m('Confirm'); + return !empty($this->profile_uri) ? _m('Confirm') : _m('Remote subscription'); } /** From 64b72a3c9b8c9ee2d8716a3271834293d1e863f8 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Sat, 22 Apr 2017 10:51:03 +0200 Subject: [PATCH 3/8] New domain regexp for WebFinger matching. --- plugins/OStatus/OStatusPlugin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/OStatus/OStatusPlugin.php b/plugins/OStatus/OStatusPlugin.php index eaf09bf6fd..09a25495df 100644 --- a/plugins/OStatus/OStatusPlugin.php +++ b/plugins/OStatus/OStatusPlugin.php @@ -287,7 +287,7 @@ class OStatusPlugin extends Plugin $wmatches = array(); // Webfinger matches: @user@example.com or even @user--one.george_orwell@1984.biz - if (preg_match_all('!(?:^|\s+)@((?:\w+[\w\-\_\.]?)*(?:[\w\-\_\.]*\w+)@(?:\w+\-?\w+\.)*\w+(?:\w+\-\w+)*\.\w+)!', + if (preg_match_all('/(?:^|\s+)@((?:\w+[\w\-\_\.]?)*(?:[\w\-\_\.]*\w+)@(?:(?!-)[A-Za-z0-9\-]{1,63}(? Date: Sat, 22 Apr 2017 10:55:24 +0200 Subject: [PATCH 4/8] A bit more instructive debugging --- plugins/LRDD/lib/discovery.php | 2 ++ plugins/OStatus/classes/Ostatus_profile.php | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/plugins/LRDD/lib/discovery.php b/plugins/LRDD/lib/discovery.php index 4049113408..c8cf3277e2 100644 --- a/plugins/LRDD/lib/discovery.php +++ b/plugins/LRDD/lib/discovery.php @@ -93,6 +93,8 @@ class Discovery // Normalize the incoming $id to make sure we have a uri $uri = self::normalize($id); + common_debug(sprintf('Performing discovery for "%s" (normalized "%s")', $id, $uri)); + foreach ($this->methods as $class) { try { $xrd = new XML_XRD(); diff --git a/plugins/OStatus/classes/Ostatus_profile.php b/plugins/OStatus/classes/Ostatus_profile.php index 5de311107c..bddec92690 100644 --- a/plugins/OStatus/classes/Ostatus_profile.php +++ b/plugins/OStatus/classes/Ostatus_profile.php @@ -1571,12 +1571,14 @@ class Ostatus_profile extends Managed_DataObject if (is_null($uri)) { // Negative cache entry // TRANS: Exception. - throw new Exception(_m('Not a valid webfinger address.')); + throw new Exception(_m('Not a valid webfinger address (via cache).')); } $oprofile = Ostatus_profile::getKV('uri', $uri); if ($oprofile instanceof Ostatus_profile) { return $oprofile; } + common_log(LOG_ERR, sprintf(__METHOD__ . ': Webfinger address cache inconsistent with database, did not find Ostatus_profile uri==%s', $uri)); + self::cacheSet(sprintf('ostatus_profile:webfinger:%s', $addr), false); } // Try looking it up From eefbfe746f7b4be7cc27cff868f9d1158b1c8dfd Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Sat, 22 Apr 2017 10:58:14 +0200 Subject: [PATCH 5/8] Split up OStatusPlugin preg functions so they can be reused cherry-pick-merge --- plugins/OStatus/OStatusPlugin.php | 155 ++++++++++++++++++------------ 1 file changed, 91 insertions(+), 64 deletions(-) diff --git a/plugins/OStatus/OStatusPlugin.php b/plugins/OStatus/OStatusPlugin.php index 09a25495df..ddee3e9f9c 100644 --- a/plugins/OStatus/OStatusPlugin.php +++ b/plugins/OStatus/OStatusPlugin.php @@ -272,6 +272,46 @@ class OStatusPlugin extends Plugin return true; } + /** + * Webfinger matches: @user@example.com or even @user--one.george_orwell@1984.biz + * + * @return array The matching IDs (without @ or acct:) and each respective position in the given string. + */ + static function extractWebfingerIds($text) + { + $wmatches = array(); + $result = preg_match_all('/(?:^|\s+)@((?:\w+[\w\-\_\.]?)*(?:[\w\-\_\.]*\w+)@(?:(?!-)[A-Za-z0-9\-]{1,63}(?log(LOG_INFO, "Checking webfinger '$target'"); - $profile = null; - try { - $oprofile = Ostatus_profile::ensureWebfinger($target); - if (!$oprofile instanceof Ostatus_profile || !$oprofile->isPerson()) { - continue; - } - $profile = $oprofile->localProfile(); - } catch (OStatusShadowException $e) { - // This means we got a local user in the webfinger lookup - $profile = $e->profile; - } catch (Exception $e) { - $this->log(LOG_ERR, "Webfinger check failed: " . $e->getMessage()); + foreach (self::extractWebfingerIds($text) as $wmatch) { + list($target, $pos) = $wmatch; + $this->log(LOG_INFO, "Checking webfinger '$target'"); + $profile = null; + try { + $oprofile = Ostatus_profile::ensureWebfinger($target); + if (!$oprofile instanceof Ostatus_profile || !$oprofile->isPerson()) { continue; } - - assert($profile instanceof Profile); - - $text = !empty($profile->nickname) && mb_strlen($profile->nickname) < mb_strlen($target) - ? $profile->getNickname() // TODO: we could do getFancyName() or getFullname() here - : $target; - $url = $profile->getUri(); - if (!common_valid_http_url($url)) { - $url = $profile->getUrl(); - } - $matches[$pos] = array('mentioned' => array($profile), - 'type' => 'mention', - 'text' => $text, - 'position' => $pos, - 'length' => mb_strlen($target), - 'url' => $url); + $profile = $oprofile->localProfile(); + } catch (OStatusShadowException $e) { + // This means we got a local user in the webfinger lookup + $profile = $e->profile; + } catch (Exception $e) { + $this->log(LOG_ERR, "Webfinger check failed: " . $e->getMessage()); + continue; } + + assert($profile instanceof Profile); + + $text = !empty($profile->nickname) && mb_strlen($profile->nickname) < mb_strlen($target) + ? $profile->getNickname() // TODO: we could do getBestName() or getFullname() here + : $target; + $url = $profile->getUri(); + if (!common_valid_http_url($url)) { + $url = $profile->getUrl(); + } + $matches[$pos] = array('mentioned' => array($profile), + 'type' => 'mention', + 'text' => $text, + 'position' => $pos, + 'length' => mb_strlen($target), + 'url' => $url); } - // Profile matches: @example.com/mublog/user - if (preg_match_all('!(?:^|\s+)@((?:\w+\.)*\w+(?:\w+\-\w+)*\.\w+(?:/\w+)*)!', - $text, - $wmatches, - PREG_OFFSET_CAPTURE)) { - foreach ($wmatches[1] as $wmatch) { - list($target, $pos) = $wmatch; - $schemes = array('http', 'https'); - foreach ($schemes as $scheme) { - $url = "$scheme://$target"; - $this->log(LOG_INFO, "Checking profile address '$url'"); - try { - $oprofile = Ostatus_profile::ensureProfileURL($url); - if ($oprofile instanceof Ostatus_profile && !$oprofile->isGroup()) { - $profile = $oprofile->localProfile(); - $text = !empty($profile->nickname) && mb_strlen($profile->nickname) < mb_strlen($target) ? - $profile->nickname : $target; - $matches[$pos] = array('mentioned' => array($profile), - 'type' => 'mention', - 'text' => $text, - 'position' => $pos, - 'length' => mb_strlen($target), - 'url' => $profile->getUrl()); - break; - } - } catch (Exception $e) { - $this->log(LOG_ERR, "Profile check failed: " . $e->getMessage()); + foreach (self::extractUrlMentions($text) as $wmatch) { + list($target, $pos) = $wmatch; + $schemes = array('http', 'https'); + foreach ($schemes as $scheme) { + $url = "$scheme://$target"; + $this->log(LOG_INFO, "Checking profile address '$url'"); + try { + $oprofile = Ostatus_profile::ensureProfileURL($url); + if ($oprofile instanceof Ostatus_profile && !$oprofile->isGroup()) { + $profile = $oprofile->localProfile(); + $text = !empty($profile->nickname) && mb_strlen($profile->nickname) < mb_strlen($target) ? + $profile->nickname : $target; + $matches[$pos] = array('mentioned' => array($profile), + 'type' => 'mention', + 'text' => $text, + 'position' => $pos, + 'length' => mb_strlen($target), + 'url' => $profile->getUrl()); + break; } + } catch (Exception $e) { + $this->log(LOG_ERR, "Profile check failed: " . $e->getMessage()); } } } From 5e7a7701b94ee63927750064a39b188d9e17164a Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Sat, 22 Apr 2017 11:07:38 +0200 Subject: [PATCH 6/8] Domain name regular expression into lib/framework.php cherry-pick-merge --- lib/framework.php | 11 +++++++++++ plugins/OStatus/OStatusPlugin.php | 4 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/framework.php b/lib/framework.php index 620730370f..29f1d694d4 100644 --- a/lib/framework.php +++ b/lib/framework.php @@ -57,6 +57,17 @@ define('NOTICE_INBOX_SOURCE_FORWARD', 4); define('NOTICE_INBOX_SOURCE_PROFILE_TAG', 5); define('NOTICE_INBOX_SOURCE_GATEWAY', -1); +/** + * StatusNet had this string as valid path characters: '\pN\pL\,\!\(\)\.\:\-\_\+\/\=\&\;\%\~\*\$\'\@' + * Some of those characters can be troublesome when auto-linking plain text. Such as "http://some.com/)" + * URL encoding should be used whenever a weird character is used, the following strings are not definitive. + */ +define('URL_REGEX_VALID_PATH_CHARS', '\pN\pL\,\!\.\:\-\_\+\/\@\=\;\%\~\*'); +define('URL_REGEX_VALID_QSTRING_CHARS', URL_REGEX_VALID_PATH_CHARS . '\&'); +define('URL_REGEX_VALID_FRAGMENT_CHARS', URL_REGEX_VALID_QSTRING_CHARS . '\?\#'); +define('URL_REGEX_EXCLUDED_END_CHARS', '\?\.\,\!\#\:\''); // don't include these if they are directly after a URL +define('URL_REGEX_DOMAIN_NAME', '(?:(?!-)[A-Za-z0-9\-]{1,63}(? Date: Sat, 22 Apr 2017 11:15:55 +0200 Subject: [PATCH 7/8] Fix URL mention regular expression in OStatusPlugin --- plugins/OStatus/OStatusPlugin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/OStatus/OStatusPlugin.php b/plugins/OStatus/OStatusPlugin.php index 8a99620aef..053615bb06 100644 --- a/plugins/OStatus/OStatusPlugin.php +++ b/plugins/OStatus/OStatusPlugin.php @@ -300,7 +300,7 @@ class OStatusPlugin extends Plugin static function extractUrlMentions($text) { $wmatches = array(); - $result = preg_match_all('!(?:^|\s+)@'.URL_REGEX_DOMAIN_NAME.'(?:/\w+)*)!', + $result = preg_match_all('/(?:^|\s+)@('.URL_REGEX_DOMAIN_NAME.'(?:\/\w+)*)/', $text, $wmatches, PREG_OFFSET_CAPTURE); From ee29b23bd40fb5f3716d76667fbfe3e4452ef39e Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Sat, 22 Apr 2017 11:45:24 +0200 Subject: [PATCH 8/8] Fix URL mention regular expression FOR REALZ --- plugins/OStatus/OStatusPlugin.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugins/OStatus/OStatusPlugin.php b/plugins/OStatus/OStatusPlugin.php index 053615bb06..98d036c87d 100644 --- a/plugins/OStatus/OStatusPlugin.php +++ b/plugins/OStatus/OStatusPlugin.php @@ -300,7 +300,9 @@ class OStatusPlugin extends Plugin static function extractUrlMentions($text) { $wmatches = array(); - $result = preg_match_all('/(?:^|\s+)@('.URL_REGEX_DOMAIN_NAME.'(?:\/\w+)*)/', + // In the regexp below we need to match / _before_ URL_REGEX_VALID_PATH_CHARS because it otherwise gets merged + // with the TLD before (but / is in URL_REGEX_VALID_PATH_CHARS anyway, it's just its positioning that is important) + $result = preg_match_all('/(?:^|\s+)@('.URL_REGEX_DOMAIN_NAME.'(?:\/['.URL_REGEX_VALID_PATH_CHARS.']*)*)/', $text, $wmatches, PREG_OFFSET_CAPTURE);