From 7440dc2145ef21986073a212d5f892dda2de6ee8 Mon Sep 17 00:00:00 2001 From: Joshua Judson Rosen Date: Wed, 30 Apr 2014 00:23:23 -0400 Subject: [PATCH] Prevent spurious refusals of legitimate notices posted to users via Salmon. Make the logic match the intent described in the comments. The intent is clearly "accept notices whenever (A or B or C)", but the logic implemented was more like "not ((not A) or (not B) or (not C))", which is a basical boolean algebra fail (each of those ORs need to become ANDs for double-negation to work). The practical implication was that, for example, writing a reply to someone else's notice and including an @-reference to _another_ user on another site to bring them into the discussion would fail to deliver the notice to the new user because their server would basically say `oh no, you can't message this user from someone else's thread' because an earlier check for the `A' or `C' parts of `(A or B or C)' prevents `B' from being checked. cf.: , which was refused by the nhcrossing.com server because it didn't know about , even though it would have passed the later `notice contains a reference to a local user' check if not for an exception being prematurely thrown. The whole idea of reporting `which specific check FAILED' in an `if ANY SUCCEEDS' analysis is just bogus, so nix all of the distinct ClientExceptions--a single `ALL FAILED' exception is the only one that makes sense. --- plugins/OStatus/actions/usersalmon.php | 34 ++++++++++++-------------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/plugins/OStatus/actions/usersalmon.php b/plugins/OStatus/actions/usersalmon.php index 383b42b147..3d742ba993 100644 --- a/plugins/OStatus/actions/usersalmon.php +++ b/plugins/OStatus/actions/usersalmon.php @@ -74,29 +74,27 @@ class UsersalmonAction extends SalmonAction } // Notice must either be a) in reply to a notice by this user - // or b) to the attention of this user - // or c) in reply to a notice to the attention of this user + // or b) in reply to a notice to the attention of this user + // or c) to the attention of this user $context = $this->activity->context; + $notice = false; if (!empty($context->replyToID)) { $notice = Notice::getKV('uri', $context->replyToID); - if (empty($notice)) { - // TRANS: Client exception. - throw new ClientException(_m('In reply to unknown notice.')); - } - if ($notice->profile_id != $this->user->id && - !in_array($this->user->id, $notice->getReplies())) { - // TRANS: Client exception. - throw new ClientException(_m('In reply to a notice not by this user and not mentioning this user.')); - } - } else if (!empty($context->attention)) { - if (!array_key_exists($this->user->getUri(), $context->attention) && - !array_key_exists(common_profile_url($this->user->nickname), $context->attention)) { - common_log(LOG_ERR, $this->user->getUri() . "not in attention list (".implode(',', array_keys($context->attention)).')'); - // TRANS: Client exception. - throw new ClientException(_m('To the attention of user(s), not including this one.')); - } + } + + if (!empty($notice) && + ($notice->profile_id == $this->user->id || + array_key_exists($this->user->id, $notice->getReplies()))) + { + // In reply to a notice either from or mentioning this user. + } else if (!empty($context->attention) && + (array_key_exists($this->user->uri, $context->attention) || + array_key_exists($common_profile_url($this->user->nickname), + $context->attention))) + { + // To the attention of this user. } else { // TRANS: Client exception. throw new ClientException(_m('Not to anyone in reply to anything.')); -- 2.39.5