jenkins-bot has submitted this change and it was merged. Change subject: Refactor API code and fix invalid input handling ......................................................................
Refactor API code and fix invalid input handling Simplify the code and add additional checks for invalid input. Change-Id: Ic1fe978730af7715c72f58cd7af46ab753e614e8 --- M ApiFlowThank.php M ApiRevThank.php M ApiThank.php 3 files changed, 23 insertions(+), 23 deletions(-) Approvals: Addshore: Looks good to me, approved jenkins-bot: Verified diff --git a/ApiFlowThank.php b/ApiFlowThank.php index 6838d33..7c86b7e 100644 --- a/ApiFlowThank.php +++ b/ApiFlowThank.php @@ -24,17 +24,22 @@ $this->dieOnBadUser( $user ); $params = $this->extractRequestParams(); - $postId = UUID::create( $params['postid'] ); - if ( $this->userAlreadySentThanksForId( $user, $postId ) ) { - $this->markResultSuccess(); - return; + try { + $postId = UUID::create( $params['postid'] ); + } catch ( FlowException $e ) { + $this->dieUsage( 'Post ID is invalid', 'invalidpostid' ); } $data = $this->getFlowData( $postId ); $recipient = $this->getRecipientFromPost( $data['post'] ); $this->dieOnBadRecipient( $user, $recipient ); + + if ( $this->userAlreadySentThanksForId( $user, $postId ) ) { + $this->markResultSuccess( $recipient->getName() ); + return; + } $rootPost = $data['root']; $workflowId = $rootPost->getPostId(); @@ -87,8 +92,7 @@ * @returns User */ private function getRecipientFromPost( PostRevision $post ) { - $uid = $post->getCreatorId(); - $recipient = User::newFromId( $uid ); + $recipient = User::newFromId( $post->getCreatorId() ); if ( !$recipient->loadFromId() ) { $this->dieUsage( 'Recipient is invalid', 'invalidrecipient' ); } @@ -102,12 +106,6 @@ private function getPageTitleFromRootPost( PostRevision $rootPost ) { $workflow = Container::get( 'storage' )->get( 'Workflow', $rootPost->getPostId() ); return $workflow->getArticleTitle(); - } - - private function markResultSuccess() { - $this->getResult()->addValue( null, 'result', array( - 'success' => 1, - ) ); } /** @@ -139,7 +137,7 @@ // Mark the thank in session to prevent duplicates (Bug 46690). $user->getRequest()->setSessionData( "flow-thanked-{$postId->getAlphadecimal()}", true ); // Set success message. - $this->markResultSuccess(); + $this->markResultSuccess( $recipient->getName() ); // Log it if we're supposed to log it. if ( $wgThanksLogging ) { $this->logThanks( $user, $recipient ); diff --git a/ApiRevThank.php b/ApiRevThank.php index 827cdd9..c1b1caf 100644 --- a/ApiRevThank.php +++ b/ApiRevThank.php @@ -16,7 +16,7 @@ $revision = $this->getRevisionFromParams( $params ); if ( $this->userAlreadySentThanksForRevision( $user, $revision ) ) { - $this->markResultSuccess( $revision ); + $this->markResultSuccess( $revision->getUserText() ); } else { $recipient = $this->getUserFromRevision( $revision ); $this->dieOnBadRecipient( $user, $recipient ); @@ -35,7 +35,9 @@ private function getRevisionFromParams( $params ) { $revision = Revision::newFromId( $params['rev'] ); - if ( !$revision ) { + + // Revision ID 1 means an invalid argument was passed in. + if ( !$revision || $revision->getId() === 1 ) { $this->dieUsage( 'Revision ID is not valid', 'invalidrevision' ); } elseif ( $revision->isDeleted( Revision::DELETED_TEXT ) ) { $this->dieUsage( 'Revision has been deleted', 'revdeleted' ); @@ -70,13 +72,6 @@ return User::newFromId( $recipient ); } - private function markResultSuccess( Revision $revision ) { - $this->getResult()->addValue( null, 'result', array( - 'success' => 1, - 'recipient' => $revision->getUserText( Revision::FOR_PUBLIC ) - ) ); - } - private function sendThanks( User $user, Revision $revision, User $recipient, $source ) { global $wgThanksLogging; $title = $this->getTitleFromRevision( $revision ); @@ -96,7 +91,7 @@ // Mark the thank in session to prevent duplicates (Bug 46690) $user->getRequest()->setSessionData( "thanks-thanked-{$revision->getId()}", true ); // Set success message - $this->markResultSuccess( $revision ); + $this->markResultSuccess( $recipient->getName() ); // Log it if we're supposed to log it if ( $wgThanksLogging ) { $this->logThanks( $user, $recipient ); diff --git a/ApiThank.php b/ApiThank.php index 7c3a904..65d6fde 100644 --- a/ApiThank.php +++ b/ApiThank.php @@ -32,6 +32,13 @@ } } + protected function markResultSuccess( $recipientName ) { + $this->getResult()->addValue( null, 'result', array( + 'success' => 1, + 'recipient' => $recipientName, + ) ); + } + protected function logThanks( User $user, User $recipient ) { $logEntry = new ManualLogEntry( 'thanks', 'thank' ); $logEntry->setPerformer( $user ); -- To view, visit https://gerrit.wikimedia.org/r/125940 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ic1fe978730af7715c72f58cd7af46ab753e614e8 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Thanks Gerrit-Branch: master Gerrit-Owner: Wctaiwan <wctai...@gmail.com> Gerrit-Reviewer: Addshore <addshorew...@gmail.com> Gerrit-Reviewer: Legoktm <legoktm.wikipe...@gmail.com> Gerrit-Reviewer: Spage <sp...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits