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.: <http://status.hackerposse.com/notice/55846>, which was
refused by the nhcrossing.com server because it didn't know
about <http://sn.jonkman.ca/notice/93724>, 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.
This commit is contained in:
Joshua Judson Rosen 2014-04-30 00:23:23 -04:00 committed by Mikael Nordfeldth
parent 15c0568d1b
commit 7440dc2145

View File

@ -74,29 +74,27 @@ class UsersalmonAction extends SalmonAction
} }
// Notice must either be a) in reply to a notice by this user // Notice must either be a) in reply to a notice by this user
// or b) to the attention of this user // or b) in reply to a notice to the attention of this user
// or c) in reply to a notice to the attention of this user // or c) to the attention of this user
$context = $this->activity->context; $context = $this->activity->context;
$notice = false;
if (!empty($context->replyToID)) { if (!empty($context->replyToID)) {
$notice = Notice::getKV('uri', $context->replyToID); $notice = Notice::getKV('uri', $context->replyToID);
if (empty($notice)) { }
// TRANS: Client exception.
throw new ClientException(_m('In reply to unknown notice.')); if (!empty($notice) &&
} ($notice->profile_id == $this->user->id ||
if ($notice->profile_id != $this->user->id && array_key_exists($this->user->id, $notice->getReplies())))
!in_array($this->user->id, $notice->getReplies())) { {
// TRANS: Client exception. // In reply to a notice either from or mentioning this user.
throw new ClientException(_m('In reply to a notice not by this user and not mentioning this user.')); } else if (!empty($context->attention) &&
} (array_key_exists($this->user->uri, $context->attention) ||
} else if (!empty($context->attention)) { array_key_exists($common_profile_url($this->user->nickname),
if (!array_key_exists($this->user->getUri(), $context->attention) && $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)).')'); // To the attention of this user.
// TRANS: Client exception.
throw new ClientException(_m('To the attention of user(s), not including this one.'));
}
} else { } else {
// TRANS: Client exception. // TRANS: Client exception.
throw new ClientException(_m('Not to anyone in reply to anything.')); throw new ClientException(_m('Not to anyone in reply to anything.'));