SubMirror now works again against old PuSH

There was a problem with (specifically at least) PuSHpress for
Wordpress. A previous attempt to perform a DB transaction backfired
because the remote side could connect to the callback before our
commit had gone through.

I take full responsibility for introducing the bug in the first place :)
This commit is contained in:
Mikael Nordfeldth 2014-03-03 00:01:13 +01:00
parent 83f50d25c0
commit dc0ae2785d
2 changed files with 7 additions and 5 deletions

View File

@ -52,7 +52,7 @@ class PushCallbackAction extends Action
} }
$feedsub = FeedSub::getKV('id', $feedid); $feedsub = FeedSub::getKV('id', $feedid);
if (!$feedsub) { if (!$feedsub instanceof FeedSub) {
// TRANS: Server exception. %s is a feed ID. // TRANS: Server exception. %s is a feed ID.
throw new ServerException(sprintf(_m('Unknown PuSH feed id %s'),$feedid), 400); throw new ServerException(sprintf(_m('Unknown PuSH feed id %s'),$feedid), 400);
} }

View File

@ -261,11 +261,16 @@ class FeedSub extends Managed_DataObject
} }
/** /**
* Setting to subscribe means it is _waiting_ to become active. This
* cannot be done in a transaction because there is a chance that the
* remote script we're calling (as in the case of PuSHpress) performs
* the lookup _while_ we're POSTing data, which means the transaction
* never completes (PushcallbackAction gets an 'inactive' state).
*
* @return boolean true on successful sub/unsub, false on failure * @return boolean true on successful sub/unsub, false on failure
*/ */
protected function doSubscribe($mode) protected function doSubscribe($mode)
{ {
$this->query('BEGIN');
$orig = clone($this); $orig = clone($this);
if ($mode == 'subscribe') { if ($mode == 'subscribe') {
$this->secret = common_random_hexstr(32); $this->secret = common_random_hexstr(32);
@ -302,7 +307,6 @@ class FeedSub extends Managed_DataObject
$response = $client->post($hub, $headers, $post); $response = $client->post($hub, $headers, $post);
$status = $response->getStatus(); $status = $response->getStatus();
if ($status == 202) { if ($status == 202) {
$this->query('COMMIT');
common_log(LOG_INFO, __METHOD__ . ': sub req ok, awaiting verification callback'); common_log(LOG_INFO, __METHOD__ . ': sub req ok, awaiting verification callback');
return true; return true;
} else if ($status >= 200 && $status < 300) { } else if ($status >= 200 && $status < 300) {
@ -310,9 +314,7 @@ class FeedSub extends Managed_DataObject
} else { } else {
common_log(LOG_ERR, __METHOD__ . ": sub req failed with HTTP $status: " . $response->getBody()); common_log(LOG_ERR, __METHOD__ . ": sub req failed with HTTP $status: " . $response->getBody());
} }
$this->query('ROLLBACK');
} catch (Exception $e) { } catch (Exception $e) {
$this->query('ROLLBACK');
// wtf! // wtf!
common_log(LOG_ERR, __METHOD__ . ": error \"{$e->getMessage()}\" hitting hub $this->huburi subscribing to $this->uri"); common_log(LOG_ERR, __METHOD__ . ": error \"{$e->getMessage()}\" hitting hub $this->huburi subscribing to $this->uri");