OStatus: fix up some recent regressions in subscription setup; fix state checks and verification token, and avatar save on setup. Needs updates for new atom code next...

plugins/OStatus/actions/pushhub.php

@@ -44,7 +44,7 @@ class PushHubAction extends Action
         // PHP converts '.'s in incoming var names to '_'s.
         // It also merges multiple values, which'll break hub.verify and hub.topic for publishing
         // @fixme handle multiple args
-        $arg = str_replace('.', '_', $arg);
+        $arg = str_replace('hub.', 'hub_', $arg);
         return parent::arg($arg, $def);
     }
 
@@ -96,7 +96,11 @@ class PushHubAction extends Action
         $sub = new HubSub();
         $sub->topic = $feed;
         $sub->callback = $callback;
+        $sub->verify_token = $this->arg('hub.verify_token', null);
         $sub->secret = $this->arg('hub.secret', null);
+        if (strlen($sub->secret) > 200) {
+            throw new ClientException("hub.secret must be no longer than 200 chars", 400);
+        }
         $sub->setLease(intval($this->arg('hub.lease_seconds')));
 
         // @fixme check for feeds we don't manage

plugins/OStatus/classes/Ostatus_profile.php

@@ -262,7 +262,7 @@ class Ostatus_profile extends Memcached_DataObject
         $avatar = $munger->getAvatar();
         if ($avatar) {
             try {
-                $this->updateAvatar($avatar);
+                $profile->updateAvatar($avatar);
             } catch (Exception $e) {
                 common_log(LOG_ERR, "Exception setting OStatus avatar: " .
                                     $e->getMessage());
@@ -283,8 +283,10 @@ class Ostatus_profile extends Memcached_DataObject
         // ripped from oauthstore.php (for old OMB client)
         $temp_filename = tempnam(sys_get_temp_dir(), 'listener_avatar');
         copy($url, $temp_filename);
-        $imagefile = new ImageFile($profile->id, $temp_filename);
+        
-        $filename = Avatar::filename($profile->id,
+        // @fixme should we be using different ids?
+        $imagefile = new ImageFile($this->id, $temp_filename);
+        $filename = Avatar::filename($this->id,
                                      image_type_to_extension($imagefile->type),
                                      null,
                                      common_timestamp());
@@ -376,17 +378,56 @@ class Ostatus_profile extends Memcached_DataObject
      * The hub will later send us a confirmation POST to /main/push/callback.
      *
      * @return bool true on success, false on failure
+     * @throws ServerException if feed state is not valid
      */
     public function subscribe($mode='subscribe')
     {
-        if (common_config('feedsub', 'nohub')) {
+        if ($this->sub_state != '') {
-            // Fake it! We're just testing remote feeds w/o hubs.
+            throw new ServerException("Attempting to start PuSH subscription to feed in state $this->sub_state");
-            return true;
         }
-        // @fixme use the verification token
+        if (empty($this->huburi)) {
-        #$token = md5(mt_rand() . ':' . $this->feeduri);
+            if (common_config('feedsub', 'nohub')) {
-        #$this->verify_token = $token;
+                // Fake it! We're just testing remote feeds w/o hubs.
-        #$this->update(); // @fixme
+                return true;
+            } else {
+                throw new ServerException("Attempting to start PuSH subscription for feed with no hub");
+            }
+        }
+
+        return $this->doSubscribe('subscribe');
+    }
+
+    /**
+     * Send a PuSH unsubscription request to the hub for this feed.
+     * The hub will later send us a confirmation POST to /main/push/callback.
+     *
+     * @return bool true on success, false on failure
+     * @throws ServerException if feed state is not valid
+     */
+    public function unsubscribe() {
+        if ($this->sub_state != 'active') {
+            throw new ServerException("Attempting to end PuSH subscription to feed in state $this->sub_state");
+        }
+        if (empty($this->huburi)) {
+            if (common_config('feedsub', 'nohub')) {
+                // Fake it! We're just testing remote feeds w/o hubs.
+                return true;
+            } else {
+                throw new ServerException("Attempting to end PuSH subscription for feed with no hub");
+            }
+        }
+
+        return $this->doSubscribe('unsubscribe');
+    }
+
+    protected function doSubscribe($mode)
+    {
+        $orig = clone($this);
+        $this->verify_token = md5(mt_rand() . ':' . $this->feeduri);
+        $this->sub_state = $mode;
+        $this->update($orig);
+        unset($orig);
+
         try {
             $callback = common_local_url('pushcallback', array('feed' => $this->id));
             $headers = array('Content-Type: application/x-www-form-urlencoded');
@@ -416,6 +457,13 @@ class Ostatus_profile extends Memcached_DataObject
         } catch (Exception $e) {
             // wtf!
             common_log(LOG_ERR, __METHOD__ . ": error \"{$e->getMessage()}\" hitting hub $this->huburi subscribing to $this->feeduri");
+
+            $orig = clone($this);
+            $this->verify_token = null;
+            $this->sub_state = null;
+            $this->update($orig);
+            unset($orig);
+
             return false;
         }
     }
@@ -460,16 +508,6 @@ class Ostatus_profile extends Memcached_DataObject
         return $this->update($original);
     }
 
-    /**
-     * Send a PuSH unsubscription request to the hub for this feed.
-     * The hub will later send us a confirmation POST to /main/push/callback.
-     *
-     * @return bool true on success, false on failure
-     */
-    public function unsubscribe() {
-        return $this->subscribe('unsubscribe');
-    }
-
     /**
      * Send an Activity Streams notification to the remote Salmon endpoint,
      * if so configured.
@@ -568,6 +606,11 @@ class Ostatus_profile extends Memcached_DataObject
     {
         common_log(LOG_INFO, __METHOD__ . ": packet for \"$this->feeduri\"! $hmac $xml");
 
+        if ($this->sub_state != 'active') {
+            common_log(LOG_ERR, __METHOD__ . ": ignoring PuSH for inactive feed $this->feeduri (in state '$this->sub_state')");
+            return;
+        }
+
         if ($this->secret) {
             if (preg_match('/^sha1=([0-9a-fA-F]{40})$/', $hmac, $matches)) {
                 $their_hmac = strtolower($matches[1]);

plugins/OStatus/lib/feedmunger.php

@@ -258,11 +258,12 @@ class FeedMunger
     {
         // hack hack hack
         // should get profile for this entry's author...
-        $remote = Ostatus_profile::staticGet('feeduri', $this->getSelfLink());
+        $feeduri = $this->getSelfLink();
-        if ($feed) {
+        $remote = Ostatus_profile::staticGet('feeduri', $feeduri);
-            return $feed->profile_id;
+        if ($remote) {
+            return $remote->profile_id;
         } else {
-            throw new Exception("Can't find feed profile");
+            throw new Exception("Can't find feed profile for $feeduri");
         }
     }