From aca5ff1b23152abf01551c384c1b1d43ecc40b6f Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Mon, 12 Jan 2015 02:23:23 +0100 Subject: [PATCH] Found some unreachable code in Favorite The portion after StartAtomPubNewActivity would never be reached since Favorite handles that activity through ActivityHandlerPlugin nowadays. So I cleaned it up and followed a couple of paths, making stuff prettier. --- lib/activityhandlerplugin.php | 12 +- lib/mail.php | 1 + .../Favorite/actions/atompubfavoritefeed.php | 128 +++--------------- plugins/Favorite/classes/Fave.php | 7 +- plugins/Favorite/lib/favcommand.php | 13 +- 5 files changed, 38 insertions(+), 123 deletions(-) diff --git a/lib/activityhandlerplugin.php b/lib/activityhandlerplugin.php index 6a148d9dd5..22f794e14b 100644 --- a/lib/activityhandlerplugin.php +++ b/lib/activityhandlerplugin.php @@ -430,12 +430,13 @@ abstract class ActivityHandlerPlugin extends Plugin * Handle object posted via AtomPub * * @param Activity &$activity Activity that was posted - * @param User $user User that posted it + * @param Profile $scoped Profile of user posting * @param Notice &$notice Resulting notice * * @return boolean hook value */ - function onStartAtomPubNewActivity(Activity &$activity, $user, &$notice) + // FIXME: Make sure we can really do strong Notice typing with a $notice===null without having =null here + public function onStartAtomPubNewActivity(Activity &$activity, Profile $scoped, Notice &$notice) { if (!$this->isMyActivity($activity)) { return true; @@ -443,10 +444,9 @@ abstract class ActivityHandlerPlugin extends Plugin $options = array('source' => 'atompub'); - // $user->getProfile() is a Profile - $notice = $this->saveNoticeFromActivity($activity, - $user->getProfile(), - $options); + $notice = $this->saveNoticeFromActivity($activity, $scoped, $options); + + Event::handle('EndAtomPubNewActivity', array($activity, $scoped, $notice)); return false; } diff --git a/lib/mail.php b/lib/mail.php index 9c3342a8cc..8b30eba609 100644 --- a/lib/mail.php +++ b/lib/mail.php @@ -675,6 +675,7 @@ function mail_notify_message($message, $from=null, $to=null) */ function mail_notify_fave(User $rcpt, Profile $sender, Notice $notice) { + // This test is actually "if the sender is sandboxed" if (!$sender->hasRight(Right::EMAILONFAVE)) { return; } diff --git a/plugins/Favorite/actions/atompubfavoritefeed.php b/plugins/Favorite/actions/atompubfavoritefeed.php index cfc79ec0f0..b8dae707b3 100644 --- a/plugins/Favorite/actions/atompubfavoritefeed.php +++ b/plugins/Favorite/actions/atompubfavoritefeed.php @@ -28,11 +28,7 @@ * @link http://status.net/ */ -if (!defined('STATUSNET')) { - // This check helps protect against security problems; - // your code file can't be executed directly from the web. - exit(1); -} +if (!defined('GNUSOCIAL') && !defined('STATUSNET')) { exit(1); } /** * Feed of ActivityStreams 'favorite' actions @@ -49,20 +45,13 @@ class AtompubfavoritefeedAction extends ApiAuthAction private $_profile = null; private $_faves = null; - /** - * For initializing members of the class. - * - * @param array $argarray misc. arguments - * - * @return boolean true - */ - function prepare($argarray) + protected function prepare(array $args=array()) { - parent::prepare($argarray); + parent::prepare($args); $this->_profile = Profile::getKV('id', $this->trimmed('profile')); - if (empty($this->_profile)) { + if (!$this->_profile instanceof Profile) { // TRANS: Client exception thrown when requesting a favorite feed for a non-existing profile. throw new ClientException(_('No such profile.'), 404); } @@ -77,16 +66,9 @@ class AtompubfavoritefeedAction extends ApiAuthAction return true; } - /** - * Handler method - * - * @param array $argarray is ignored since it's now passed in in prepare() - * - * @return void - */ - function handle($argarray=null) + protected function handle() { - parent::handle($argarray); + parent::handle(); switch ($_SERVER['REQUEST_METHOD']) { case 'HEAD': @@ -99,10 +81,9 @@ class AtompubfavoritefeedAction extends ApiAuthAction default: // TRANS: Client exception thrown when using an unsupported HTTP method. throw new ClientException(_('HTTP method not supported.'), 405); - return; } - return; + return true; } /** @@ -209,11 +190,10 @@ class AtompubfavoritefeedAction extends ApiAuthAction { // XXX: Refactor this; all the same for atompub - if (empty($this->auth_user) || - $this->auth_user->id != $this->_profile->id) { + if (!$this->scoped instanceof Profile || + $this->scoped->id != $this->_profile->id) { // TRANS: Client exception thrown when trying to set a favorite for another user. - throw new ClientException(_("Cannot add someone else's". - " subscription."), 403); + throw new ClientException(_("Cannot add someone else's subscription."), 403); } $xml = file_get_contents('php://input'); @@ -224,69 +204,24 @@ class AtompubfavoritefeedAction extends ApiAuthAction $dom->documentElement->localName != 'entry') { // TRANS: Client error displayed when not using an Atom entry. throw new ClientException(_('Atom post must be an Atom entry.')); - return; } $activity = new Activity($dom->documentElement); + $notice = null; - $fave = null; + // Favorite plugin handles these as ActivityHandlerPlugin through Notice->saveActivity + // which in turn uses "StoreActivityObject" event. + Event::handle('StartAtomPubNewActivity', array(&$activity, $this->scoped, &$notice)); + assert($notice instanceof Notice); - if (Event::handle('StartAtomPubNewActivity', array(&$activity))) { + $act = $notice->asActivity(); - if ($activity->verb != ActivityVerb::FAVORITE) { - // TRANS: Client exception thrown when trying use an incorrect activity verb for the Atom pub method. - throw new ClientException(_('Can only handle favorite activities.')); - return; - } + header('Content-Type: application/atom+xml; charset=utf-8'); + header('Content-Location: ' . $act->selfLink); - $note = $activity->objects[0]; - - if (!in_array($note->type, array(ActivityObject::NOTE, - ActivityObject::BLOGENTRY, - ActivityObject::STATUS))) { - // TRANS: Client exception thrown when trying favorite an object that is not a notice. - throw new ClientException(_('Can only fave notices.')); - return; - } - - $notice = Notice::getKV('uri', $note->id); - - if (empty($notice)) { - // XXX: import from listed URL or something - // TRANS: Client exception thrown when trying favorite a notice without content. - throw new ClientException(_('Unknown notice.')); - } - - $old = Fave::pkeyGet(array('user_id' => $this->auth_user->id, - 'notice_id' => $notice->id)); - - if (!empty($old)) { - // TRANS: Client exception thrown when trying favorite an already favorited notice. - throw new ClientException(_('Already a favorite.')); - } - - $profile = $this->auth_user->getProfile(); - - $fave = Fave::addNew($profile, $notice); - - if ($fave instanceof Fave) { - Fave::blowCacheForProfileId($this->_profile->id); - $this->notify($fave, $notice, $this->auth_user); - } - - Event::handle('EndAtomPubNewActivity', array($activity, $fave)); - } - - if (!empty($fave)) { - $act = $fave->asActivity(); - - header('Content-Type: application/atom+xml; charset=utf-8'); - header('Content-Location: ' . $act->selfLink); - - $this->startXML(); - $this->raw($act->asString(true, true, true)); - $this->endXML(); - } + $this->startXML(); + $this->raw($act->asString(true, true, true)); + $this->endXML(); } /** @@ -348,25 +283,4 @@ class AtompubfavoritefeedAction extends ApiAuthAction return true; } } - - /** - * Notify the author of the favorite that the user likes their notice - * - * @param Favorite $fave the favorite in question - * @param Notice $notice the notice that's been faved - * @param User $user the user doing the favoriting - * - * @return void - */ - function notify($fave, $notice, $user) - { - $other = User::getKV('id', $notice->profile_id); - if ($other && $other->id != $user->id && !empty($other->email)) { - require_once INSTALLDIR.'/lib/mail.php'; - - mail_notify_fave($other, $user->getProfile(), $notice); - // XXX: notify by IM - // XXX: notify by SMS - } - } } diff --git a/plugins/Favorite/classes/Fave.php b/plugins/Favorite/classes/Fave.php index 7a18bbfd44..6acfb38014 100644 --- a/plugins/Favorite/classes/Fave.php +++ b/plugins/Favorite/classes/Fave.php @@ -44,7 +44,8 @@ class Fave extends Managed_DataObject * * @param Profile $profile the local or remote user who likes * @param Notice $notice the notice that is liked - * @return mixed false on failure, or Fave record on success + * @return Fave record on success + * @throws Exception on failure */ static function addNew(Profile $profile, Notice $notice) { @@ -66,7 +67,7 @@ class Fave extends Managed_DataObject $fave->insert(); } catch (ServerException $e) { common_log_db_error($fave, 'INSERT', __FILE__); - return false; + throw $e; } self::blowCacheForProfileId($fave->user_id); self::blowCacheForNoticeId($fave->notice_id); @@ -158,7 +159,7 @@ class Fave extends Managed_DataObject return $act; } - static function existsForProfile($notice, Profile $scoped) + static function existsForProfile(Notice $notice, Profile $scoped) { $fave = self::pkeyGet(array('user_id'=>$scoped->id, 'notice_id'=>$notice->id)); diff --git a/plugins/Favorite/lib/favcommand.php b/plugins/Favorite/lib/favcommand.php index 94658b8a53..d89b7f3421 100644 --- a/plugins/Favorite/lib/favcommand.php +++ b/plugins/Favorite/lib/favcommand.php @@ -25,13 +25,12 @@ class FavCommand extends Command return; } - $fave = Fave::addNew($this->user->getProfile(), $notice); - - if (!$fave) { - // TRANS: Error message text shown when a favorite could not be set. - $channel->error($this->user, _('Could not create favorite.')); - return; - } + try { + $fave = Fave::addNew($this->user->getProfile(), $notice); + } catch (Exception $e) { + $channel->error($this->user, $e->getMessage()); + return; + } // @fixme favorite notification should be triggered // at a lower level