From d2e66641322a3297be6a3a6680d10ed7b6720c0e Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Fri, 19 Mar 2010 10:15:00 -0700 Subject: [PATCH 1/6] Validate OStatus avatar URL before fetching. --- plugins/OStatus/classes/Ostatus_profile.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/plugins/OStatus/classes/Ostatus_profile.php b/plugins/OStatus/classes/Ostatus_profile.php index 6ae8e4fd58..6145080fc7 100644 --- a/plugins/OStatus/classes/Ostatus_profile.php +++ b/plugins/OStatus/classes/Ostatus_profile.php @@ -781,8 +781,8 @@ class Ostatus_profile extends Memcached_DataObject } /** - * * Download and update given avatar image + * * @param string $url * @throws Exception in various failure cases */ @@ -792,6 +792,9 @@ class Ostatus_profile extends Memcached_DataObject // We've already got this one. return; } + if (!common_valid_http_url($url)) { + throw new ServerException(_m("Invalid avatar URL %s"), $url); + } if ($this->isGroup()) { $self = $this->localGroup(); From b97400bd6f49dfac71124a3243d1c27f49822f58 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Fri, 19 Mar 2010 11:17:56 -0700 Subject: [PATCH 2/6] clarify output on fixup-shadow.php --- plugins/OStatus/scripts/fixup-shadow.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/OStatus/scripts/fixup-shadow.php b/plugins/OStatus/scripts/fixup-shadow.php index 0171b77bc9..ec014c7878 100644 --- a/plugins/OStatus/scripts/fixup-shadow.php +++ b/plugins/OStatus/scripts/fixup-shadow.php @@ -50,7 +50,7 @@ $encGroup = str_replace($marker, '%', $encGroup); $sql = "SELECT * FROM ostatus_profile WHERE uri LIKE '%s' OR uri LIKE '%s'"; $oprofile->query(sprintf($sql, $encProfile, $encGroup)); -echo "Found $oprofile->N bogus ostatus_profile entries:\n"; +echo "Found $oprofile->N bogus ostatus_profile entries for local users and groups:\n"; while ($oprofile->fetch()) { echo "$oprofile->uri"; @@ -58,7 +58,7 @@ while ($oprofile->fetch()) { if ($dry) { echo " (unchanged)\n"; } else { - echo " deleting..."; + echo " removing bogus ostatus_profile entry..."; $evil = clone($oprofile); $evil->delete(); echo " ok\n"; From 5c314c22885f78a04684637cb8a0e4e745220bd9 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Mon, 15 Mar 2010 15:41:57 -0700 Subject: [PATCH 3/6] Drop result ID from data objects on clone(). This keeps the original object working if it was in the middle of a query loop, even if the cloned object falls out of scope and triggers its destructor. This bug was hitting a number of places where we had the pattern: $db->find(); while($dbo->fetch()) { $x = clone($dbo); // do anything with $x other than storing it in an array } The cloned object's destructor would trigger on the second run through the loop, freeing the database result set -- not really what we wanted. (Loops that stored the clones into an array were fine, since the clones stay in scope in the array longer than the original does.) Detaching the database result from the clone lets us work with its data without interfering with the rest of the query. In the unlikely even that somebody is making clones in the middle of a query, then trying to continue the query with the clone instead of the original object, well they're gonna be broken now. --- classes/Safe_DataObject.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/classes/Safe_DataObject.php b/classes/Safe_DataObject.php index 021f7b5064..08bc6846f4 100644 --- a/classes/Safe_DataObject.php +++ b/classes/Safe_DataObject.php @@ -42,6 +42,25 @@ class Safe_DataObject extends DB_DataObject } } + /** + * Magic function called at clone() time. + * + * We use this to drop connection with some global resources. + * This supports the fairly common pattern where individual + * items being read in a loop via a single object are cloned + * for individual processing, then fall out of scope when the + * loop comes around again. + * + * As that triggers the destructor, we want to make sure that + * the original object doesn't have its database result killed. + * It will still be freed properly when the original object + * gets destroyed. + */ + function __clone() + { + $this->_DB_resultid = false; + } + /** * Magic function called at serialize() time. * From 8a221228ebac156cbbb4693b06e25a0c59c858c3 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Fri, 19 Mar 2010 12:50:34 -0700 Subject: [PATCH 4/6] Fix typo in public tag cloud query setup which caused the cutoff to get skipped. --- actions/publictagcloud.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actions/publictagcloud.php b/actions/publictagcloud.php index 9993b2d3fd..70c356659a 100644 --- a/actions/publictagcloud.php +++ b/actions/publictagcloud.php @@ -109,7 +109,7 @@ class PublictagcloudAction extends Action $cutoff = sprintf("notice_tag.created > '%s'", common_sql_date(time() - common_config('tag', 'cutoff'))); $tags->selectAdd($calc . ' as weight'); - $tags->addWhere($cutoff); + $tags->whereAdd($cutoff); $tags->groupBy('tag'); $tags->orderBy('weight DESC'); From c84c4c6839c1791775cc698ad488bc23ed956d5b Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Fri, 19 Mar 2010 15:47:43 -0700 Subject: [PATCH 5/6] OStatus: be a little laxer about attempts to start/stop PuSH subscriptions that were left in an inconsistent state. Instead of aborting, we'll try to reconfirm the sub/unsub, which once confirmed will replace whatever the previous state was on the server side. --- plugins/OStatus/classes/FeedSub.php | 6 +++--- plugins/OStatus/classes/Ostatus_profile.php | 21 ++++++++++----------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/plugins/OStatus/classes/FeedSub.php b/plugins/OStatus/classes/FeedSub.php index 80ba37bc11..b10509dae6 100644 --- a/plugins/OStatus/classes/FeedSub.php +++ b/plugins/OStatus/classes/FeedSub.php @@ -61,7 +61,7 @@ class FeedSub extends Memcached_DataObject public $__table = 'feedsub'; public $id; - public $feeduri; + public $uri; // PuSH subscription data public $huburi; @@ -238,7 +238,7 @@ class FeedSub extends Memcached_DataObject public function subscribe($mode='subscribe') { if ($this->sub_state && $this->sub_state != 'inactive') { - throw new ServerException("Attempting to start PuSH subscription to feed in state $this->sub_state"); + common_log(LOG_WARNING, "Attempting to (re)start PuSH subscription to $this->uri in unexpected state $this->sub_state"); } if (empty($this->huburi)) { if (common_config('feedsub', 'nohub')) { @@ -261,7 +261,7 @@ class FeedSub extends Memcached_DataObject */ public function unsubscribe() { if ($this->sub_state != 'active') { - throw new ServerException("Attempting to end PuSH subscription to feed in state $this->sub_state"); + common_log(LOG_WARNING, "Attempting to (re)end PuSH subscription to $this->uri in unexpected state $this->sub_state"); } if (empty($this->huburi)) { if (common_config('feedsub', 'nohub')) { diff --git a/plugins/OStatus/classes/Ostatus_profile.php b/plugins/OStatus/classes/Ostatus_profile.php index e0e0223b8f..bff6ff209b 100644 --- a/plugins/OStatus/classes/Ostatus_profile.php +++ b/plugins/OStatus/classes/Ostatus_profile.php @@ -204,12 +204,13 @@ class Ostatus_profile extends Memcached_DataObject public function subscribe() { $feedsub = FeedSub::ensureFeed($this->feeduri); - if ($feedsub->sub_state == 'active' || $feedsub->sub_state == 'subscribe') { + if ($feedsub->sub_state == 'active') { + // Active subscription, we don't need to do anything. return true; - } else if ($feedsub->sub_state == '' || $feedsub->sub_state == 'inactive') { + } else { + // Inactive or we got left in an inconsistent state. + // Run a subscription request to make sure we're current! return $feedsub->subscribe(); - } else if ('unsubscribe') { - throw new FeedSubException("Unsub is pending, can't subscribe..."); } } @@ -222,15 +223,13 @@ class Ostatus_profile extends Memcached_DataObject */ public function unsubscribe() { $feedsub = FeedSub::staticGet('uri', $this->feeduri); - if (!$feedsub) { + if (!$feedsub || $feedsub->sub_state == '' || $feedsub->sub_state == 'inactive') { + // No active PuSH subscription, we can just leave it be. return true; - } - if ($feedsub->sub_state == 'active') { + } else { + // PuSH subscription is either active or in an indeterminate state. + // Send an unsubscribe. return $feedsub->unsubscribe(); - } else if ($feedsub->sub_state == '' || $feedsub->sub_state == 'inactive' || $feedsub->sub_state == 'unsubscribe') { - return true; - } else if ($feedsub->sub_state == 'subscribe') { - throw new FeedSubException("Feed is awaiting subscription, can't unsub..."); } } From db0cf50f658a91d0d0a019256e6e85d73b7a3ff6 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Fri, 19 Mar 2010 15:54:16 -0700 Subject: [PATCH 6/6] Avoid notices for accessing undefined array indices in hcard processing --- plugins/OStatus/lib/discoveryhints.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/OStatus/lib/discoveryhints.php b/plugins/OStatus/lib/discoveryhints.php index 9102788e6f..80cfbbf15e 100644 --- a/plugins/OStatus/lib/discoveryhints.php +++ b/plugins/OStatus/lib/discoveryhints.php @@ -102,7 +102,7 @@ class DiscoveryHints { if (array_key_exists('url', $hcard)) { if (is_string($hcard['url'])) { $hints['homepage'] = $hcard['url']; - } else if (is_array($hcard['url'])) { + } else if (is_array($hcard['url']) && !empty($hcard['url'])) { // HACK get the last one; that's how our hcards look $hints['homepage'] = $hcard['url'][count($hcard['url'])-1]; } @@ -231,7 +231,7 @@ class DiscoveryHints { // If it's got a scheme, use it - if ($parts['scheme'] != '') { + if (!empty($parts['scheme'])) { return $rel; }