jenkins-bot has submitted this change and it was merged.
Change subject: Fix fatal error when using invalid target page title
......................................................................
Fix fatal error when using invalid target page title
Errors where not reported at all. So I fixed the bug by throwing
an exception on invalid target page title and inserted some error
reporting.
Bug: 50288
Change-Id: Ie7eeaba8d8c4c92daea8ae1a4945abfeeafdc7ab
---
M includes/SF_AutoeditAPI.php
M languages/SF_Messages.php
M specials/SF_FormEdit.php
3 files changed, 57 insertions(+), 33 deletions(-)
Approvals:
Siebrand: Looks good to me, but someone else must approve
Yaron Koren: Checked; Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/SF_AutoeditAPI.php b/includes/SF_AutoeditAPI.php
index 6faa350..8797183 100644
--- a/includes/SF_AutoeditAPI.php
+++ b/includes/SF_AutoeditAPI.php
@@ -124,7 +124,6 @@
/**
*
- * @param type $options
*/
function prepareAction() {
@@ -314,7 +313,14 @@
global $wgUser;
// Find existing target article if it exists, or create a new
one.
- $article = new Article( Title::newFromText( $this->mOptions[
'target' ] ) );
+ $targetTitle = Title::newFromText( $this->mOptions[ 'target' ]
);
+
+ // if the specified target title is invalid, give up
+ if ( !$targetTitle instanceof Title ) {
+ throw new MWException( wfMessage(
'sf_autoedit_invalidtargetspecified', $this->mOptions[ 'target' ] )->parse() );
+ }
+
+ $article = new Article( $targetTitle );
// set up a normal edit page
// we'll feed it our data to simulate a normal edit
@@ -437,7 +443,6 @@
$status = $editor->internalAttemptSave( $resultDetails, $bot );
- // FIXME: Throw MWError instead of returning true;
switch ( $status->value ) {
case EditPage::AS_HOOK_ERROR_EXPECTED: // A hook
function returned an error
case EditPage::AS_CONTENT_TOO_BIG: // Content too big
(> $wgMaxArticleSize)
@@ -449,7 +454,6 @@
case EditPage::AS_END: // WikiPage::doEdit() was
unsuccessfull
throw new MWException( wfMessage(
'sf_autoedit_fail', $this->mOptions[ 'target' ] )->parse() );
- return true; // fail
case EditPage::AS_HOOK_ERROR: // Article update aborted
by a hook function
@@ -511,7 +515,6 @@
}
throw new MWException( wfMessage(
'spamprotectionmatch', wfEscapeWikiText( $match ) )->parse() ); // FIXME:
Include better error message
- return true; // fail
case EditPage::AS_BLOCKED_PAGE_FOR_USER: // User is
blocked from editting editor page
throw new UserBlockedError( $wgUser->getBlock()
);
@@ -540,7 +543,6 @@
// Render the status object into
$editor->hookError
$editor->hookError = '<div class="error">' .
$status->getWikitext() . '</div>';
throw new MWException( $status->getHTML() );
- return true; // fail
}
}
@@ -610,8 +612,9 @@
*
* This parses the formula and replaces <unique number> tags
*
- * @global type $wgParser
- * @param type $targetNameFormula
+ * @param type $targetNameFormula
+ *
+ * @throws MWException
* @return type
*/
protected function generateTargetName( $targetNameFormula ) {
@@ -633,7 +636,7 @@
// if any formula stuff is still in the name after the parsing,
just remove it
// FIXME: This is wrong. If anything is still left, something
should have been present in the form and wasn't. An error should be raised.
- $targetName = StringUtils::delimiterReplace( '<', '>', '',
$targetName );
+ //$targetName = StringUtils::delimiterReplace( '<', '>', '',
$targetName );
// replace spaces back with underlines, in case a magic word or
parser
// function name contains underlines - hopefully this won't
cause
@@ -644,7 +647,7 @@
global $wgParser;
$targetName = $wgParser->transformMsg( $targetName,
ParserOptions::newFromUser( null ) );
- $title_number = '';
+ $titleNumber = '';
$isRandom = false;
$randomNumHasPadding = false;
$randomNumDigits = 6;
@@ -655,7 +658,7 @@
$isRandom = true;
$randomNumHasPadding = array_key_exists( 2,
$matches );
$randomNumDigits = ( array_key_exists( 3,
$matches ) ? $matches[ 3 ] : $randomNumDigits );
- $title_number = SFUtils::makeRandomNumber(
$randomNumDigits, $randomNumHasPadding );
+ $titleNumber = SFUtils::makeRandomNumber(
$randomNumDigits, $randomNumHasPadding );
} else if ( preg_match(
'/{num.*start[_]*=[_]*([^;]*).*}/', $targetName, $matches ) ) {
// get unique number start value
// from target name; if it's not
@@ -664,43 +667,47 @@
;
if ( count( $matches ) == 2 && is_numeric(
$matches[ 1 ] ) && $matches[ 1 ] >= 0 ) {
// the "start" value"
- $title_number = $matches[ 1 ];
+ $titleNumber = $matches[ 1 ];
}
} else if ( preg_match( '/^(_?{num.*}?)*$/',
$targetName, $matches ) ) {
// the target name contains only underscores
and number fields,
// i.e. would result in an empty title without
the number set
- $title_number = '1';
+ $titleNumber = '1';
} else {
- $title_number = '';
+ $titleNumber = '';
}
// set target title
- $target_title = Title::newFromText( preg_replace(
'/{num.*}/', $title_number, $targetName ) );
+ $targetTitle = Title::newFromText( preg_replace(
'/{num.*}/', $titleNumber, $targetName ) );
+ // if the specified target title is invalid, give up
+ if ( !$targetTitle instanceof Title ) {
+ throw new MWException( wfMessage(
'sf_autoedit_invalidtargetspecified', trim( preg_replace( '/<unique
number(.*)>/', $titleNumber, $targetNameFormula ) ) )->parse() );
+ }
// if title exists already cycle through numbers for
this tag until
// we find one that gives a nonexistent page title;
//
- // can not use $target_title->exists(); it does not use
+ // can not use $targetTitle->exists(); it does not use
// Title::GAID_FOR_UPDATE, which is needed to get
correct data from
- // cache; use $target_title->getArticleID() instead
- while ( $target_title->getArticleID(
Title::GAID_FOR_UPDATE ) !== 0 ) {
+ // cache; use $targetTitle->getArticleID() instead
+ while ( $targetTitle->getArticleID(
Title::GAID_FOR_UPDATE ) !== 0 ) {
if ( $isRandom ) {
- $title_number =
SFUtils::makeRandomNumber( $randomNumDigits, $randomNumHasPadding );
+ $titleNumber =
SFUtils::makeRandomNumber( $randomNumDigits, $randomNumHasPadding );
}
// if title number is blank, change it to 2;
otherwise,
// increment it, and if necessary pad it with
leading 0s as well
- elseif ( $title_number == "" ) {
- $title_number = 2;
+ elseif ( $titleNumber == "" ) {
+ $titleNumber = 2;
} else {
- $title_number = str_pad( $title_number
+ 1, strlen( $title_number ), '0', STR_PAD_LEFT );
+ $titleNumber = str_pad( $titleNumber +
1, strlen( $titleNumber ), '0', STR_PAD_LEFT );
}
- $target_title = Title::newFromText(
preg_replace( '/{num.*}/', $title_number, $targetName ) );
+ $targetTitle = Title::newFromText(
preg_replace( '/{num.*}/', $titleNumber, $targetName ) );
}
- $targetName = $target_title->getPrefixedText();
+ $targetName = $targetTitle->getPrefixedText();
}
return $targetName;
@@ -1063,7 +1070,11 @@
/**
* Add error message to the ApiResult
+ *
* @param string $msg
+ * @param int $errorLevel
+ *
+ * @return string
*/
private function logMessage( $msg, $errorLevel = self::ERROR ) {
diff --git a/languages/SF_Messages.php b/languages/SF_Messages.php
index 8ca9229..45deada 100644
--- a/languages/SF_Messages.php
+++ b/languages/SF_Messages.php
@@ -160,9 +160,9 @@
'sf_autoedit_anoneditwarning' => 'Warning: You are not logged
in. Your IP address will be recorded in this page\'s edit history.',
'sf_autoedit_success' => 'Successfully modified [[$1]]
using form $2.',
'sf_autoedit_fail' => 'Modifying [[$1]] failed.',
- 'sf_autoedit_notargetspecified' => "No target page specified.",
- 'sf_autoedit_invalidtargetspecified' => 'The specified target page $1
is invalid.',
- 'sf_autoedit_invalidform' => '$1 is not a valid form.',
+ 'sf_autoedit_notargetspecified' => 'No target page specified.',
+ 'sf_autoedit_invalidtargetspecified' => "The specified target page
'''$1''' is invalid.",
+ 'sf_autoedit_invalidform' => "'''$1''' is not a valid form.",
'sf_autoedit_redirectlimitexeeded' => 'The maximum redirect limit for
form $1 was exceeded.',
'sf_autoedit_invalidredirecttarget' => '$1 is an invalid redirect
target for form $2.',
'sf_autoedit_invalidpreloadspecified'=> 'The specified preload page $1
is invalid.',
diff --git a/specials/SF_FormEdit.php b/specials/SF_FormEdit.php
index 49e62c2..626aa6e 100644
--- a/specials/SF_FormEdit.php
+++ b/specials/SF_FormEdit.php
@@ -62,7 +62,7 @@
return $text;
}
- static function printForm( &$form_name, &$targetName, $alt_forms =
array( ) ) {
+ static function printForm( &$form_name, &$targetName, $alt_forms =
array( ), $error ) {
global $wgOut, $wgRequest;
if ( method_exists( 'ApiMain', 'getContext' ) ) {
@@ -95,9 +95,23 @@
$module->execute();
+ $text = '';
+
// if action was successful and action was a Save, return
- if ( $module->getStatus() === 200 && $module->getAction() ===
SFAutoeditAPI::ACTION_SAVE ) {
- return;
+ if ( $module->getStatus() === 200 ) {
+ if ( $module->getAction() ===
SFAutoeditAPI::ACTION_SAVE ) {
+ return;
+ }
+ } else {
+
+ $resultData = $module->getResultData();
+
+ if ( array_key_exists( 'errors', $resultData ) ) {
+
+ foreach ($resultData['errors'] as $error) {
+ $text .= Html::rawElement( 'p', array(
'class' => 'error' ), $error['message'] ) . "\n";
+ }
+ }
}
// override the default title for this page if a title was
specified in the form
@@ -123,15 +137,14 @@
} else {
$pageTitle = wfMessage(
'sf_formedit_createtitle', $result[ 'form' ], $targetName )->text();
}
- } elseif ( $result[ 'form' ] == '' ) {
+ } elseif ( $result[ 'form' ] == '' ) { //FIXME: This looks
weird; a simple else should be enough, right?
// display error message if the form is not specified
in the URL
$pageTitle = wfMessage( 'formedit' )->text();
- $text = Html::element( 'p', array( 'class' => 'error'
), wfMessage( 'sf_formedit_badurl' )->text() ) . "\n";
+ $text .= Html::element( 'p', array( 'class' => 'error'
), wfMessage( 'sf_formedit_badurl' )->text() ) . "\n";
$wgOut->addHTML( $text );
}
$wgOut->setPageTitle( $pageTitle );
- $text = '';
if ( count( $alt_forms ) > 0 ) {
$text .= '<div class="infoMessage">' . wfMessage(
'sf_formedit_altforms' )->escaped() . ' ';
$text .= self::printAltFormsList( $alt_forms,
$targetName );
--
To view, visit https://gerrit.wikimedia.org/r/87441
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie7eeaba8d8c4c92daea8ae1a4945abfeeafdc7ab
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/SemanticForms
Gerrit-Branch: master
Gerrit-Owner: Foxtrott <s7ep...@gmail.com>
Gerrit-Reviewer: Siebrand <siebr...@wikimedia.org>
Gerrit-Reviewer: Yaron Koren <yaro...@gmail.com>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
