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 <[email protected]>
Gerrit-Reviewer: Addshore <[email protected]>
Gerrit-Reviewer: Legoktm <[email protected]>
Gerrit-Reviewer: Spage <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
