1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-26 08:42:41 +01:00

Fix two very, very minor correctness issues in Slowvote

Summary:
See <https://hackerone.com/reports/492525> and <https://hackerone.com/reports/489531>. I previously awarded a bounty for <https://hackerone.com/reports/434116> so Slowvote is getting "researched" a lot.

  - Prevent users from undoing their vote by submitting the form with nothing selected.
  - Prevent users from racing between the `delete()` and `save()` to vote for multiple options in a plurality poll.

Test Plan:
  - Clicked the vote button with nothing selected in plurality and approval polls, got an error now.
  - Added a `sleep(5)` between `delete()` and `save()`. Submitted different plurality votes in different windows. Before: votes raced, invalid end state. After: votes waited on the lock, arrived in a valid end state.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D20125
This commit is contained in:
epriestley 2019-02-07 09:26:07 -08:00
parent f2236eb061
commit 26081594e2

View file

@ -37,6 +37,19 @@ final class PhabricatorSlowvoteVoteController
$method = $poll->getMethod(); $method = $poll->getMethod();
$is_plurality = ($method == PhabricatorSlowvotePoll::METHOD_PLURALITY); $is_plurality = ($method == PhabricatorSlowvotePoll::METHOD_PLURALITY);
if (!$votes) {
if ($is_plurality) {
$message = pht('You must vote for something.');
} else {
$message = pht('You must vote for at least one option.');
}
return $this->newDialog()
->setTitle(pht('Stand For Something'))
->appendParagraph($message)
->addCancelButton($poll->getURI());
}
if ($is_plurality && count($votes) > 1) { if ($is_plurality && count($votes) > 1) {
throw new Exception( throw new Exception(
pht('In this poll, you may only vote for one option.')); pht('In this poll, you may only vote for one option.'));
@ -52,14 +65,27 @@ final class PhabricatorSlowvoteVoteController
} }
} }
$poll->openTransaction();
$poll->beginReadLocking();
$poll->reload();
$old_votes = id(new PhabricatorSlowvoteChoice())->loadAllWhere(
'pollID = %d AND authorPHID = %s',
$poll->getID(),
$viewer->getPHID());
$old_votes = mpull($old_votes, null, 'getOptionID');
foreach ($old_votes as $old_vote) { foreach ($old_votes as $old_vote) {
if (!idx($votes, $old_vote->getOptionID(), false)) { if (idx($votes, $old_vote->getOptionID())) {
$old_vote->delete(); continue;
} }
$old_vote->delete();
} }
foreach ($votes as $vote) { foreach ($votes as $vote) {
if (idx($old_votes, $vote, false)) { if (idx($old_votes, $vote)) {
continue; continue;
} }
@ -70,6 +96,9 @@ final class PhabricatorSlowvoteVoteController
->save(); ->save();
} }
$poll->endReadLocking();
$poll->saveTransaction();
return id(new AphrontRedirectResponse()) return id(new AphrontRedirectResponse())
->setURI($poll->getURI()); ->setURI($poll->getURI());
} }