From f1af583e3e37ffb5384276222d0c1cfc1e12dd90 Mon Sep 17 00:00:00 2001 From: Evan Prodromou Date: Thu, 18 Mar 2010 07:55:14 -0500 Subject: catch exceptions in OStatus updates, log, and continue --- plugins/OStatus/scripts/updateostatus.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'plugins/OStatus') diff --git a/plugins/OStatus/scripts/updateostatus.php b/plugins/OStatus/scripts/updateostatus.php index d553a7d62..1ef1ee829 100644 --- a/plugins/OStatus/scripts/updateostatus.php +++ b/plugins/OStatus/scripts/updateostatus.php @@ -56,7 +56,12 @@ try { $user = new User(); if ($user->find()) { while ($user->fetch()) { - updateOStatus($user); + try { + updateOStatus($user); + } catch (Exception $e) { + common_log(LOG_NOTICE, "Couldn't convert OMB subscriptions ". + "for {$user->nickname} to OStatus: " . $e->getMessage()); + } } } } else { @@ -118,8 +123,7 @@ function updateOStatus($user) if (!have_option('q', 'quiet')) { echo "fail.\n"; } - continue; - common_log(LOG_WARNING, "Couldn't convert OMB subscription (" . $up->nickname . ", " . $rp->nickname . + common_log(LOG_NOTICE, "Couldn't convert OMB subscription (" . $up->nickname . ", " . $rp->nickname . ") to OStatus: " . $e->getMessage()); continue; } -- cgit v1.2.3-54-g00ecf From a764603e733ba102c605e09d7279a59ee0d57e22 Mon Sep 17 00:00:00 2001 From: Evan Prodromou Date: Thu, 18 Mar 2010 08:04:22 -0500 Subject: reverse order from delete, insert to insert, delete on ostatus conversion --- plugins/OStatus/scripts/updateostatus.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'plugins/OStatus') diff --git a/plugins/OStatus/scripts/updateostatus.php b/plugins/OStatus/scripts/updateostatus.php index 1ef1ee829..315b49273 100644 --- a/plugins/OStatus/scripts/updateostatus.php +++ b/plugins/OStatus/scripts/updateostatus.php @@ -112,8 +112,8 @@ function updateOStatus($user) if (!have_option('q', 'quiet')) { echo "Converting..."; } - Subscription::cancel($up, $rp); Subscription::start($up, $op->localProfile()); + Subscription::cancel($up, $rp); if (!have_option('q', 'quiet')) { echo "done.\n"; } -- cgit v1.2.3-54-g00ecf From f1b3d84b7d55cc7c466982a95815ba1ddf130418 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 18 Mar 2010 13:13:57 -0700 Subject: Fix some remote subscription regressions from f21f78364a9cbde2ca535a3983b384707ad097ae Ostatus_profile::ensureProfile() has been renamed to Ostatus_profile::ensureProfileURL() --- plugins/OStatus/OStatusPlugin.php | 4 ++-- plugins/OStatus/classes/Ostatus_profile.php | 2 +- plugins/OStatus/scripts/updateostatus.php | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) (limited to 'plugins/OStatus') diff --git a/plugins/OStatus/OStatusPlugin.php b/plugins/OStatus/OStatusPlugin.php index b472ae242..58f373e45 100644 --- a/plugins/OStatus/OStatusPlugin.php +++ b/plugins/OStatus/OStatusPlugin.php @@ -290,7 +290,7 @@ class OStatusPlugin extends Plugin $url = "$scheme://$target"; $this->log(LOG_INFO, "Checking profile address '$url'"); try { - $oprofile = Ostatus_profile::ensureProfile($url); + $oprofile = Ostatus_profile::ensureProfileURL($url); if ($oprofile && !$oprofile->isGroup()) { $profile = $oprofile->localProfile(); $matches[$pos] = array('mentioned' => array($profile), @@ -392,7 +392,7 @@ class OStatusPlugin extends Plugin foreach ($urls as $url) { try { - return Ostatus_profile::ensureProfile($url); + return Ostatus_profile::ensureProfileURL($url); } catch (Exception $e) { common_log(LOG_ERR, 'Profile lookup failed for ' . $arg . ': ' . $e->getMessage()); diff --git a/plugins/OStatus/classes/Ostatus_profile.php b/plugins/OStatus/classes/Ostatus_profile.php index 73f5d2322..f8fda4162 100644 --- a/plugins/OStatus/classes/Ostatus_profile.php +++ b/plugins/OStatus/classes/Ostatus_profile.php @@ -1464,7 +1464,7 @@ class Ostatus_profile extends Memcached_DataObject if (array_key_exists('profileurl', $hints)) { try { common_log(LOG_INFO, "Discovery on acct:$addr with profile URL $profileUrl"); - $oprofile = self::ensureProfile($hints['profileurl'], $hints); + $oprofile = self::ensureProfileURL($hints['profileurl'], $hints); self::cacheSet(sprintf('ostatus_profile:webfinger:%s', $addr), $oprofile->uri); return $oprofile; } catch (Exception $e) { diff --git a/plugins/OStatus/scripts/updateostatus.php b/plugins/OStatus/scripts/updateostatus.php index 315b49273..622ded56a 100644 --- a/plugins/OStatus/scripts/updateostatus.php +++ b/plugins/OStatus/scripts/updateostatus.php @@ -103,7 +103,7 @@ function updateOStatus($user) echo "Checking {$rp->nickname}..."; } - $op = Ostatus_profile::ensureProfile($rp->profileurl); + $op = Ostatus_profile::ensureProfileURL($rp->profileurl); if (empty($op)) { echo "can't convert.\n"; -- cgit v1.2.3-54-g00ecf From cac9d23498808a60dfb890aef437606e98106a6d Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 18 Mar 2010 14:26:32 -0700 Subject: Fix for xmpp/sms notification options appearing to be disabled on new subscriptions. Base problem is that our caching-on-insert interferes with relying on column default values; the cached object is missing those fields, so they appear to be empty (null) when the object is retrieved from cache. Now explicitly setting them when inserting subscriptions, and cleaned up some code that had alternate code paths. May also have made auto-subscription work for remote OStatus subscribers, but can't test until magic sigs are working again. --- classes/Subscription.php | 56 +++++++++++++++-------------- classes/User.php | 27 +++----------- plugins/OStatus/actions/ostatussub.php | 2 +- plugins/OStatus/classes/Ostatus_profile.php | 46 ------------------------ 4 files changed, 35 insertions(+), 96 deletions(-) (limited to 'plugins/OStatus') diff --git a/classes/Subscription.php b/classes/Subscription.php index 5ac95f922..97c44a2e4 100644 --- a/classes/Subscription.php +++ b/classes/Subscription.php @@ -75,20 +75,7 @@ class Subscription extends Memcached_DataObject } if (Event::handle('StartSubscribe', array($subscriber, $other))) { - - $sub = new Subscription(); - - $sub->subscriber = $subscriber->id; - $sub->subscribed = $other->id; - $sub->created = common_sql_now(); - - $result = $sub->insert(); - - if (!$result) { - common_log_db_error($sub, 'INSERT', __FILE__); - throw new Exception(_('Could not save subscription.')); - } - + $sub = self::saveNew($subscriber->id, $other->id); $sub->notify(); self::blow('user:notices_with_friends:%d', $subscriber->id); @@ -103,20 +90,11 @@ class Subscription extends Memcached_DataObject !self::exists($other, $subscriber) && !$subscriber->hasBlocked($other)) { - $auto = new Subscription(); - - $auto->subscriber = $other->id; - $auto->subscribed = $subscriber->id; - $auto->created = common_sql_now(); - - $result = $auto->insert(); - - if (!$result) { - common_log_db_error($auto, 'INSERT', __FILE__); - throw new Exception(_('Could not save subscription.')); + try { + self::start($other, $subscriber); + } catch (Exception $e) { + common_log(LOG_ERR, "Exception during autosubscribe of {$other->nickname} to profile {$subscriber->id}: {$e->getMessage()}"); } - - $auto->notify(); } Event::handle('EndSubscribe', array($subscriber, $other)); @@ -125,6 +103,30 @@ class Subscription extends Memcached_DataObject return true; } + /** + * Low-level subscription save. + * Outside callers should use Subscription::start() + */ + protected function saveNew($subscriber_id, $other_id) + { + $sub = new Subscription(); + + $sub->subscriber = $subscriber_id; + $sub->subscribed = $other_id; + $sub->jabber = 1; + $sub->sms = 1; + $sub->created = common_sql_now(); + + $result = $sub->insert(); + + if (!$result) { + common_log_db_error($sub, 'INSERT', __FILE__); + throw new Exception(_('Could not save subscription.')); + } + + return $sub; + } + function notify() { # XXX: add other notifications (Jabber, SMS) here diff --git a/classes/User.php b/classes/User.php index 16c256535..8ad2ec63d 100644 --- a/classes/User.php +++ b/classes/User.php @@ -153,19 +153,12 @@ class User extends Memcached_DataObject return Sms_carrier::staticGet('id', $this->carrier); } + /** + * @deprecated use Subscription::start($sub, $other); + */ function subscribeTo($other) { - $sub = new Subscription(); - $sub->subscriber = $this->id; - $sub->subscribed = $other->id; - - $sub->created = common_sql_now(); // current time - - if (!$sub->insert()) { - return false; - } - - return true; + return Subscription::start($this->getProfile(), $other); } function hasBlocked($other) @@ -346,17 +339,7 @@ class User extends Memcached_DataObject common_log(LOG_WARNING, sprintf("Default user %s does not exist.", $defnick), __FILE__); } else { - $defsub = new Subscription(); - $defsub->subscriber = $user->id; - $defsub->subscribed = $defuser->id; - $defsub->created = $user->created; - - $result = $defsub->insert(); - - if (!$result) { - common_log_db_error($defsub, 'INSERT', __FILE__); - return false; - } + Subscription::start($user, $defuser); } } diff --git a/plugins/OStatus/actions/ostatussub.php b/plugins/OStatus/actions/ostatussub.php index 07081c2c6..994af6e95 100644 --- a/plugins/OStatus/actions/ostatussub.php +++ b/plugins/OStatus/actions/ostatussub.php @@ -299,7 +299,7 @@ class OStatusSubAction extends Action if ($user->isSubscribed($local)) { // TRANS: OStatus remote subscription dialog error. $this->showForm(_m('Already subscribed!')); - } elseif ($this->oprofile->subscribeLocalToRemote($user)) { + } elseif (Subscription::start($user, $local)) { $this->success(); } else { // TRANS: OStatus remote subscription dialog error. diff --git a/plugins/OStatus/classes/Ostatus_profile.php b/plugins/OStatus/classes/Ostatus_profile.php index f8fda4162..90a8d0ef4 100644 --- a/plugins/OStatus/classes/Ostatus_profile.php +++ b/plugins/OStatus/classes/Ostatus_profile.php @@ -194,52 +194,6 @@ class Ostatus_profile extends Memcached_DataObject } } - /** - * Subscribe a local user to this remote user. - * PuSH subscription will be started if necessary, and we'll - * send a Salmon notification to the remote server if available - * notifying them of the sub. - * - * @param User $user - * @return boolean success - * @throws FeedException - */ - public function subscribeLocalToRemote(User $user) - { - if ($this->isGroup()) { - throw new ServerException("Can't subscribe to a remote group"); - } - - if ($this->subscribe()) { - if ($user->subscribeTo($this->localProfile())) { - $this->notify($user->getProfile(), ActivityVerb::FOLLOW, $this); - return true; - } - } - return false; - } - - /** - * Mark this remote profile as subscribing to the given local user, - * and send appropriate notifications to the user. - * - * This will generally be in response to a subscription notification - * from a foreign site to our local Salmon response channel. - * - * @param User $user - * @return boolean success - */ - public function subscribeRemoteToLocal(User $user) - { - if ($this->isGroup()) { - throw new ServerException("Remote groups can't subscribe to local users"); - } - - Subscription::start($this->localProfile(), $user->getProfile()); - - return true; - } - /** * Send a subscription request to the hub for this feed. * The hub will later send us a confirmation POST to /main/push/callback. -- cgit v1.2.3-54-g00ecf From c8e3d08a8fb139003ae425e8b80296215f071b25 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 18 Mar 2010 15:11:25 -0700 Subject: Fix notice warning about unused var -- was renamed during refactoring. --- plugins/OStatus/classes/Ostatus_profile.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'plugins/OStatus') diff --git a/plugins/OStatus/classes/Ostatus_profile.php b/plugins/OStatus/classes/Ostatus_profile.php index 90a8d0ef4..e77c8f7e9 100644 --- a/plugins/OStatus/classes/Ostatus_profile.php +++ b/plugins/OStatus/classes/Ostatus_profile.php @@ -1403,7 +1403,7 @@ class Ostatus_profile extends Memcached_DataObject if (array_key_exists('feedurl', $hints)) { try { - common_log(LOG_INFO, "Discovery on acct:$addr with feed URL $feedUrl"); + common_log(LOG_INFO, "Discovery on acct:$addr with feed URL " . $hints['feedurl']); $oprofile = self::ensureFeedURL($hints['feedurl'], $hints); self::cacheSet(sprintf('ostatus_profile:webfinger:%s', $addr), $oprofile->uri); return $oprofile; -- cgit v1.2.3-54-g00ecf From 1301877dfe89c57c182246c0d7ba0ff6335fd17b Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 18 Mar 2010 17:08:19 -0700 Subject: OStatus discover fixes: * Subscription::start was sometimes passing users instead of profiles to hooks, which broke OStatus subscription notifications; now normalizing to profiles for processing. * H-card parsing would trigger a lot of PHP warnings and notices in hKit. Now suppressing warnings and notices for the duration of the call to keep them out of output when display_errors is on. * H-card parsing would trigger a PHP fatal error if the source page was not well-formed XML and Tidy was not present on the system. Switched normalization to use the PHP DOM module which is always present, as we have no need for Tidy's extra features here. * Trying to fetch avatars from Google profiles failed and triggered a PHP warning due to the relative URL not being resolved during h-card parsing. Now passing profile page URL into hKit by sneaking a tag in while we normalize the HTML source. * Profile pages without a "Link" header could trigger PHP notices due to a bad NULL -> array(NULL) conversion in LinkHeader::getLink(). Now checking that there was a return value before converting single return value into array. --- classes/Subscription.php | 8 +++ lib/activity.php | 4 +- plugins/OStatus/lib/discoveryhints.php | 91 ++++++++++++++++++++++------------ plugins/OStatus/lib/linkheader.php | 24 ++++----- 4 files changed, 81 insertions(+), 46 deletions(-) (limited to 'plugins/OStatus') diff --git a/classes/Subscription.php b/classes/Subscription.php index 97c44a2e4..60c12cccc 100644 --- a/classes/Subscription.php +++ b/classes/Subscription.php @@ -62,6 +62,14 @@ class Subscription extends Memcached_DataObject static function start($subscriber, $other) { + // @fixme should we enforce this as profiles in callers instead? + if ($subscriber instanceof User) { + $subscriber = $subscriber->getProfile(); + } + if ($other instanceof User) { + $other = $other->getProfile(); + } + if (!$subscriber->hasRight(Right::SUBSCRIBE)) { throw new Exception(_('You have been banned from subscribing.')); } diff --git a/lib/activity.php b/lib/activity.php index d84eabf7c..c67d090f7 100644 --- a/lib/activity.php +++ b/lib/activity.php @@ -720,7 +720,7 @@ class ActivityObject } } - static function fromNotice($notice) + static function fromNotice(Notice $notice) { $object = new ActivityObject(); @@ -734,7 +734,7 @@ class ActivityObject return $object; } - static function fromProfile($profile) + static function fromProfile(Profile $profile) { $object = new ActivityObject(); diff --git a/plugins/OStatus/lib/discoveryhints.php b/plugins/OStatus/lib/discoveryhints.php index db13793dd..4da2ec0f1 100644 --- a/plugins/OStatus/lib/discoveryhints.php +++ b/plugins/OStatus/lib/discoveryhints.php @@ -65,17 +65,22 @@ class DiscoveryHints { { common_debug("starting tidy"); - $body = self::_tidy($body); + $body = self::_tidy($body, $url); common_debug("done with tidy"); set_include_path(get_include_path() . PATH_SEPARATOR . INSTALLDIR . '/plugins/OStatus/extlib/hkit/'); require_once('hkit.class.php'); - $h = new hKit; + // hKit code is not clean for notices and warnings + $old = error_reporting(); + error_reporting($old & ~E_NOTICE & ~E_WARNING); + $h = new hKit; $hcards = $h->getByString('hcard', $body); + error_reporting($old); + if (empty($hcards)) { return array(); } @@ -144,39 +149,61 @@ class DiscoveryHints { return $hints; } - private static function _tidy($body) + /** + * hKit needs well-formed XML for its parsing. + * We'll take the HTML body here and normalize it to XML. + * + * @param string $body HTML document source, possibly not-well-formed + * @param string $url source URL + * @return string well-formed XML document source + * @throws Exception if HTML parsing failed. + */ + private static function _tidy($body, $url) { - if (function_exists('tidy_parse_string')) { - common_debug("Tidying with extension"); - $text = tidy_parse_string($body); - $text = tidy_clean_repair($text); - return $body; - } else if ($fullpath = self::_findProgram('tidy')) { - common_debug("Tidying with program $fullpath"); - $tempfile = tempnam('/tmp', 'snht'); // statusnet hcard tidy - file_put_contents($tempfile, $source); - exec("$fullpath -utf8 -indent -asxhtml -numeric -bare -quiet $tempfile", $tidy); - unlink($tempfile); - return implode("\n", $tidy); - } else { - common_debug("Not tidying."); - return $body; + if (empty($body)) { + throw new Exception("Empty HTML could not be parsed."); } - } - - private static function _findProgram($name) - { - $path = $_ENV['PATH']; - - $parts = explode(':', $path); - - foreach ($parts as $part) { - $fullpath = $part . '/' . $name; - if (is_executable($fullpath)) { - return $fullpath; + $dom = new DOMDocument(); + + // Some HTML errors will trigger warnings, but still work. + $old = error_reporting(); + error_reporting($old & ~E_WARNING); + + $ok = $dom->loadHTML($body); + + error_reporting($old); + + if ($ok) { + // hKit doesn't give us a chance to pass the source URL for + // resolving relative links, such as the avatar photo on a + // Google profile. We'll slip it into a tag if there's + // not already one present. + $bases = $dom->getElementsByTagName('base'); + if ($bases && $bases->length >= 1) { + $base = $bases->item(0); + if ($base->hasAttribute('href')) { + $base->setAttribute('href', $url); + } + } else { + $base = $dom->createElement('base'); + $base->setAttribute('href', $url); + $heads = $dom->getElementsByTagName('head'); + if ($heads || $heads->length) { + $head = $heads->item(0); + } else { + $head = $dom->createElement('head'); + $root = $dom->documentRoot; + if ($root->firstChild) { + $root->insertBefore($head, $root->firstChild); + } else { + $root->appendChild($head); + } + } + $head->appendChild($base); } + return $dom->saveXML(); + } else { + throw new Exception("Invalid HTML could not be parsed."); } - - return null; } } diff --git a/plugins/OStatus/lib/linkheader.php b/plugins/OStatus/lib/linkheader.php index 2f6c66dc9..afcd66d26 100644 --- a/plugins/OStatus/lib/linkheader.php +++ b/plugins/OStatus/lib/linkheader.php @@ -43,21 +43,21 @@ class LinkHeader static function getLink($response, $rel=null, $type=null) { $headers = $response->getHeader('Link'); + if ($headers) { + // Can get an array or string, so try to simplify the path + if (!is_array($headers)) { + $headers = array($headers); + } - // Can get an array or string, so try to simplify the path - if (!is_array($headers)) { - $headers = array($headers); - } - - foreach ($headers as $header) { - $lh = new LinkHeader($header); + foreach ($headers as $header) { + $lh = new LinkHeader($header); - if ((is_null($rel) || $lh->rel == $rel) && - (is_null($type) || $lh->type == $type)) { - return $lh->href; + if ((is_null($rel) || $lh->rel == $rel) && + (is_null($type) || $lh->type == $type)) { + return $lh->href; + } } } - return null; } -} \ No newline at end of file +} -- cgit v1.2.3-54-g00ecf