r51123 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r51122‎ \| r51123 \| r51124 >
Date:	17:08, 28 May 2009
Author:	robin
Status:	ok (Comments)
Tags:
Comment:	Update per review on r50188: * better function names * use getOption() instead of mOptions[] * specific messages instead of general 'go' and 'username', and fix escaping and parsing * fix security vulnerability in RC form
Modified paths:	/trunk/extensions/WikimediaIncubator/CreateAccountTestWiki.php (modified) (history) /trunk/extensions/WikimediaIncubator/IncubatorTest.php (modified) (history) /trunk/extensions/WikimediaIncubator/SpecialViewUserLang.php (modified) (history) /trunk/extensions/WikimediaIncubator/TestWikiRC.php (modified) (history) /trunk/extensions/WikimediaIncubator/WikimediaIncubator.i18n.php (modified) (history) /trunk/extensions/WikimediaIncubator/WikimediaIncubator.php (modified) (history)

Diff [purge]

Index: trunk/extensions/WikimediaIncubator/WikimediaIncubator.php
—	—	@@ -10,19 +10,18 @@
11	11	'path' => __FILE__,
12	12	'name' => 'Wikimedia Incubator',
13	13	'author' => 'SPQRobin',
14		~~- 'version' => '2.1.4',~~
	14	+ 'version' => '2.2.0',
15	15	'url' => 'http://www.mediawiki.org/wiki/Extension:WikimediaIncubator',
16	16	'description' => 'Test wiki features for Wikimedia Incubator',
17	17	'descriptionmsg' => 'wminc-desc',
18	18	);
19	19
20	20	/* Config */
21		~~-$wmincEnableAutoTestWiki = true;~~
22		~~-$wmincPref = 'incubatortestwiki'; /* Name of the preference */~~
23	21	$wgGroupPermissions['*']['viewuserlang'] = false;
24	22	$wgGroupPermissions['sysop']['viewuserlang'] = true;
25	23
26	24	/* General */
	25	+$wmincPref = 'incubatortestwiki'; // Name of the preference
27	26	$dir = dirname( __FILE__ ) . '/';
28	27	$wgExtensionMessagesFiles['WikimediaIncubator'] = $dir . 'WikimediaIncubator.i18n.php';
29	28
—	—	@@ -35,22 +34,21 @@
36	35
37	36	/* TestWiki preference */
38	37	$wgAutoloadClasses['IncubatorTest'] = $dir . 'IncubatorTest.php';
39		~~-$wgHooks['GetPreferences'][] = 'IncubatorTest::AddPreferences';~~
40		~~-$wgHooks['MagicWordwgVariableIDs'][] = 'IncubatorTest::MagicWordVariable';~~
41		~~-$wgHooks['LanguageGetMagic'][] = 'IncubatorTest::MagicWord';~~
42		~~-$wgHooks['ParserGetVariableValueSwitch'][] = 'IncubatorTest::MagicWordValue';~~
	38	+$wgHooks['GetPreferences'][] = 'IncubatorTest::onGetPreferences';
	39	+$wgHooks['MagicWordwgVariableIDs'][] = 'IncubatorTest::magicWordVariable';
	40	+$wgHooks['LanguageGetMagic'][] = 'IncubatorTest::magicWord';
	41	+$wgHooks['ParserGetVariableValueSwitch'][] = 'IncubatorTest::magicWordValue';
43	42
44	43	/* Edit page */
45		~~-$wgHooks['EditPage::showEditForm:initial'][] = 'IncubatorTest::EditPageCheckPrefix';~~
	44	+$wgHooks['EditPage::showEditForm:initial'][] = 'IncubatorTest::editPageCheckPrefix';
46	45
47	46	/* Recent Changes */
48	47	$wgAutoloadClasses['TestWikiRC'] = $dir . 'TestWikiRC.php';
49		~~-$wgHooks['SpecialRecentChangesQuery'][] = 'TestWikiRC::RcQuery';~~
50		~~-$wgHooks['SpecialRecentChangesPanel'][] = 'TestWikiRC::RcForm';~~
	48	+$wgHooks['SpecialRecentChangesQuery'][] = 'TestWikiRC::onRcQuery';
	49	+$wgHooks['SpecialRecentChangesPanel'][] = 'TestWikiRC::onRcForm';
51	50
52	51	/* Automatic pref on account creation */
53		~~-if ( $wmincEnableAutoTestWiki == true ) {~~
54		~~- $wgAutoloadClasses['AutoTestWiki'] = $dir . 'CreateAccountTestWiki.php';~~
55		~~- $wgHooks['UserCreateForm'][] = 'AutoTestWiki::UserCreateForm';~~
56		~~- $wgHooks['AddNewAccount'][] = 'AutoTestWiki::AddNewAccount';~~
57		-}
	52	+$wgAutoloadClasses['AutoTestWiki'] = $dir . 'CreateAccountTestWiki.php';
	53	+$wgHooks['UserCreateForm'][] = 'AutoTestWiki::onUserCreateForm';
	54	+$wgHooks['AddNewAccount'][] = 'AutoTestWiki::onAddNewAccount';
	55	+
Index: trunk/extensions/WikimediaIncubator/SpecialViewUserLang.php
—	—	@@ -54,9 +54,9 @@
55	55	$title = htmlspecialchars( $this->getTitle()->getPrefixedText() );
56	56	$action = htmlspecialchars( $wgScript );
57	57	$target = htmlspecialchars( $target );
58		~~- $ok = wfMsg( 'go' );~~
59		~~- $username = wfMsg( 'username' );~~
60		~~- $inputformtop = wfMsg( 'wminc-viewuserlang' );~~
	58	+ $ok = wfMsgHtml( 'go' );
	59	+ $username = wfMsgHtml( 'username' );
	60	+ $inputformtop = wfMsgHtml( 'wminc-viewuserlang' );
61	61
62	62	$wgOut->addHTML( <<<EOT
63	63	<fieldset>
—	—	@@ -84,7 +84,7 @@
85	85	global $wgOut, $wgLang, $wgLanguageNames, $wmincPref;
86	86	$user = User::newFromName( $target );
87	87	if ( $user == null \|\| $user->getId() == 0 ) {
88		~~- $wgOut->addWikiText( '<span class="error">' . wfMsg( 'wminc-viewuserlang-unexisting', $target ) . '</span>' );~~
	88	+ $wgOut->addWikiText( '<span class="error">' . wfMsgNoTrans( 'wminc-viewuserlang-unexisting', $target ) . '</span>' );
89	89	} else {
90	90	$name = $user->getName();
91	91	$wgOut->addWikiText( '*' . wfMsg( 'username' ) . ' [[User:' . $name . '\|' . $name . ']] (' .
—	—	@@ -97,9 +97,9 @@
98	98	$wgLanguageNames[$user->mOptions['language']] . ' (' . $user->mOptions['language'] . ')'
99	99	)
100	100	);
101		~~- if ( IncubatorTest::NormalPrefix() == true ) {~~
	101	+ if ( IncubatorTest::isNormalPrefix() == true ) {
102	102	$testwiki = '[[W' . $user->mOptions[$wmincPref . '-project'] . '/' . $user->mOptions[$wmincPref . '-code'] . ']]';
103		~~- } elseif ( IncubatorTest::DisplayPrefix() == 'inc' ) {~~
	103	+ } elseif ( IncubatorTest::displayPrefix() == 'inc' ) {
104	104	$testwiki = 'Incubator';
105	105	} else {
106	106	$testwiki = wfMsg( 'wminc-testwiki-none' );
Index: trunk/extensions/WikimediaIncubator/CreateAccountTestWiki.php
—	—	@@ -4,7 +4,7 @@
5	5	* This can be used to work with my toolserver project (http://toolserver.org/~robin/?tool=proposewiki), so users don't have to change their preferences (automatically is always better :p)
6	6	*/
7	7	class AutoTestWiki {
8		~~- function UserCreateForm( $template ) {~~
	8	+ function onUserCreateForm( $template ) {
9	9	global $wgRequest;
10	10	$projectvalue = strtolower( $wgRequest->getVal( 'testwikiproject', '' ) );
11	11	$codevalue = strtolower( $wgRequest->getVal( 'testwikicode', '' ) );
—	—	@@ -16,7 +16,7 @@
17	17	return true;
18	18	}
19	19
20		~~- function AddNewAccount( $user ) {~~
	20	+ function onAddNewAccount( $user ) {
21	21	global $wgRequest, $wmincPref;
22	22	$getprojectvalue = $wgRequest->getVal( 'testwiki-project' );
23	23	$getcodevalue = $wgRequest->getVal( 'testwiki-code' );
Index: trunk/extensions/WikimediaIncubator/IncubatorTest.php
—	—	@@ -5,7 +5,7 @@
6	6
7	7	class IncubatorTest
8	8	{
9		~~- static function AddPreferences( $user, &$preferences ) {~~
	9	+ static function onGetPreferences( $user, &$preferences ) {
10	10	global $wmincPref, $wmincPrefProject, $wmincPrefNone;
11	11
12	12	$preferences['language']['help-message'] = 'wminc-prefinfo-language';
—	—	@@ -35,7 +35,7 @@
36	36	return true;
37	37	}
38	38
39		~~- function CodeValidation( $input, $alldata ) {~~
	39	+ function codeValidation( $input, $alldata ) {
40	40	global $wmincPref;
41	41	// If the user selected a project that NEEDS a language code, but the user DID NOT enter a language code, give an error
42	42	if ( !in_array( $alldata[$wmincPref . '-project'], array( '', 'none', 'inc' ) ) && !$input ) {
—	—	@@ -45,79 +45,85 @@
46	46	}
47	47	}
48	48
49		~~- function NormalPrefix() {~~
	49	+ function isNormalPrefix() {
50	50	global $wgUser, $wmincPref;
51		~~- if ( in_array( $wgUser->mOptions[$wmincPref . '-project'], array( '', 'none', 'inc' ) ) ) {~~
	51	+ if ( in_array( $wgUser->getOption($wmincPref . '-project'), array( '', 'none', 'inc' ) ) ) {
52	52	return false; // false because this is NOT a normal prefix
53	53	} else {
54	54	return true; // true because this is a normal prefix
55	55	}
56	56	}
57		~~- function DisplayPrefix() {~~
	57	+ function displayPrefix() {
	58	+ // display the prefix of the user preference
58	59	global $wgUser, $wmincPref;
59		~~- if ( self::NormalPrefix() == true ) {~~
60		~~- return 'W' . $wgUser->mOptions[$wmincPref . '-project'] . '/' . $wgUser->mOptions[$wmincPref . '-code']; // return the prefix~~
	60	+ if ( self::isNormalPrefix() == true ) {
	61	+ return 'W' . $wgUser->getOption($wmincPref . '-project') . '/' . $wgUser->getOption($wmincPref . '-code'); // return the prefix
61	62	} else {
62		~~- return $wgUser->mOptions[$wmincPref . '-project']; // still provide the value~~
	63	+ return $wgUser->getOption($wmincPref . '-project'); // still provide the value
63	64	}
64	65	}
65	66
66		~~- function DisplayPrefixedTitle( $title, $namespace = '' ) {~~
	67	+ function displayPrefixedTitle( $title, $namespace = '' ) {
67	68	global $wgUser, $wmincPref;
68	69	$out = '';
69		~~- if ( self::NormalPrefix() ) {~~
	70	+ if ( self::isNormalPrefix() ) {
70	71	if ( $namespace ) { $out .= $namespace . ':'; }
71		~~- $out .= self::DisplayPrefix() . '/' . $title;~~
	72	+ $out .= self::displayPrefix() . '/' . $title;
72	73	} else {
73		~~- $out .= self::DisplayPrefix();~~
	74	+ $out .= self::displayPrefix();
74	75	}
75	76	return $out;
76	77	}
77	78
78		~~- function MagicWordVariable( &$magicWords ) {~~
	79	+ function magicWordVariable( &$magicWords ) {
79	80	$magicWords[] = 'usertestwiki';
80	81	return true;
81	82	}
82	83
83		~~- function MagicWord( &$magicWords, $langCode ) {~~
	84	+ function magicWord( &$magicWords, $langCode ) {
84	85	$magicWords['usertestwiki'] = array( 0, 'USERTESTWIKI' );
85	86	return true;
86	87	}
87	88
88		~~- function MagicWordValue( &$parser, &$cache, &$magicWordId, &$ret ) {~~
89		~~- $ret = self::DisplayPrefix();~~
	89	+ function magicWordValue( &$parser, &$cache, &$magicWordId, &$ret ) {
	90	+ if( !self::displayPrefix() ) {
	91	+ $ret = 'none';
	92	+ } else {
	93	+ $ret = self::displayPrefix();
	94	+ }
90	95	return true;
91	96	}
92	97
93		~~- function EditPageCheckPrefix( $editpage ) {~~
	98	+ function editPageCheckPrefix( $editpage ) {
94	99	// If user has "project" as test wiki preference, it isn't needed to check
95		~~- if ( self::DisplayPrefix() != 'inc' ) {~~
96		~~- global $wgTitle;~~
97		~~- $namespaces = array( NS_MAIN, NS_TALK, NS_TEMPLATE, NS_TEMPLATE_TALK, NS_CATEGORY, NS_CATEGORY_TALK );~~
98		~~- // If it is in one of the above namespace, check if the page title has a prefix~~
99		~~- if ( in_array( $wgTitle->getNamespace(), $namespaces ) && !preg_match( '/W[bnpqt]\/[a-z][a-z][a-z]?/', $wgTitle->getText() ) ) {~~
100		~~- global $wgOut;~~
101		~~- wfLoadExtensionMessages( 'WikimediaIncubator' );~~
102		~~- $warning = '<div id="wminc-warning"><span id="wm-warning-unprefixed">'~~
103		~~- . wfMsg( 'wminc-warning-unprefixed' )~~
	100	+ if ( self::displayPrefix() == 'inc' ) {
	101	+ return true;
	102	+ }
	103	+ global $wgTitle;
	104	+ $namespaces = array( NS_MAIN, NS_TALK, NS_TEMPLATE, NS_TEMPLATE_TALK, NS_CATEGORY, NS_CATEGORY_TALK );
	105	+ // If it is in one of the above namespace, check if the page title has a prefix
	106	+ if ( in_array( $wgTitle->getNamespace(), $namespaces ) && !preg_match( '/W[bnpqt]\/[a-z][a-z][a-z]?/', $wgTitle->getText() ) ) {
	107	+ global $wgOut;
	108	+ wfLoadExtensionMessages( 'WikimediaIncubator' );
	109	+ $warning = '<div id="wminc-warning"><span id="wm-warning-unprefixed">'
	110	+ . wfMsg( 'wminc-warning-unprefixed' )
	111	+ . '</span>';
	112	+ // If the user has a test wiki pref, suggest a page title with prefix
	113	+ if ( self::isNormalPrefix() ) {
	114	+ global $wgUser;
	115	+ $suggest = self::displayPrefixedTitle( $wgTitle->getText(), $wgTitle->getNsText() );
	116	+ if ( !$wgTitle->exists() ) { // Creating a page, so suggest to create a prefixed page
	117	+ $warning .= ' <span id="wminc-warning-suggest">'
	118	+ . wfMsg( 'wminc-warning-suggest', $suggest )
104	119	. '</span>';
105		~~- // If the user has a test wiki pref, suggest a page title with prefix~~
106		~~- if ( self::NormalPrefix() ) {~~
107		~~- global $wgUser;~~
108		~~- $suggest = self::DisplayPrefixedTitle( $wgTitle->getText(), $wgTitle->getNsText() );~~
109		~~- if ( !$wgTitle->exists() ) { // Creating a page, so suggest to create a prefixed page~~
110		~~- $warning .= ' <span id="wminc-warning-suggest">'~~
111		~~- . wfMsg( 'wminc-warning-suggest', $suggest )~~
112		~~- . '</span>';~~
113		~~- } elseif ( $wgUser->isAllowed( 'move' ) ) { // Page exists, so suggest to move~~
114		~~- $warning .= ' <span id="wminc-warning-suggest-move" class="plainlinks">'~~
115		~~- . wfMsg( 'wminc-warning-suggest-move', $suggest, urlencode( $suggest ), urlencode( $wgTitle ) )~~
116		~~- . '</span>';~~
117		~~- }~~
	120	+ } elseif ( $wgUser->isAllowed( 'move' ) ) { // Page exists, so suggest to move
	121	+ $warning .= ' <span id="wminc-warning-suggest-move" class="plainlinks">'
	122	+ . wfMsg( 'wminc-warning-suggest-move', $suggest, urlencode( $suggest ), urlencode( $wgTitle ) )
	123	+ . '</span>';
118	124	}
119		~~- $warning .= '</div>';~~
120		~~- $wgOut->addWikiText( $warning );~~
121		~~- }~~
	125	+ }
	126	+ $warning .= '</div>';
	127	+ $wgOut->addWikiText( $warning );
122	128	}
123	129	return true;
124	130	}
Index: trunk/extensions/WikimediaIncubator/TestWikiRC.php
—	—	@@ -4,10 +4,10 @@
5	5	*/
6	6
7	7	class TestWikiRC {
8		~~- static function RcQuery( &$conds, &$tables, &$join_conds, $opts ) {~~
	8	+ static function onRcQuery( &$conds, &$tables, &$join_conds, $opts ) {
9	9	global $wgUser, $wgRequest, $wmincPref;
10		~~- $projectvalue = strtolower( $wgRequest->getVal( 'rc-testwiki-project', $wgUser->mOptions[$wmincPref . '-project'] ) );~~
11		~~- $codevalue = strtolower( $wgRequest->getVal( 'rc-testwiki-code', $wgUser->mOptions[$wmincPref . '-code'] ) );~~
	10	+ $projectvalue = strtolower( $wgRequest->getVal( 'rc-testwiki-project', $wgUser->getOption($wmincPref . '-project') ) );
	11	+ $codevalue = strtolower( $wgRequest->getVal( 'rc-testwiki-code', $wgUser->getOption($wmincPref . '-code') ) );
12	12	$fullprefix = 'W' . $projectvalue . '/' . $codevalue;
13	13	$opts->add( 'rc-testwiki-project', false );
14	14	$opts->setValue( 'rc-testwiki-project', $projectvalue );
—	—	@@ -25,16 +25,17 @@
26	26	$dbr = wfGetDB( DB_SLAVE );
27	27	$namespaces = array( NS_MAIN, NS_TALK, NS_TEMPLATE, NS_TEMPLATE_TALK, NS_CATEGORY, NS_CATEGORY_TALK );
28	28	$conds[] = 'rc_namespace IN (' . $dbr->makeList( $namespaces ) . ')';
29		~~- $conds[] = 'rc_title like \'' . $fullprefix . '/%\' OR rc_title like \'' . $fullprefix . '\'';~~
	29	+ $conds[] = 'rc_title like ' . $dbr->addQuotes( $dbr->escapeLike( $fullprefix ) . '/%' ) .
	30	+ ' OR rc_title = ' . $dbr->addQuotes( $fullprefix );
30	31	return true;
31	32	}
32	33	}
33	34
34		~~- static function RcForm( &$items, $opts ) {~~
	35	+ static function onRcForm( &$items, $opts ) {
35	36	global $wgUser, $wgRequest, $wmincPref;
36	37	wfLoadExtensionMessages( 'WikimediaIncubator' );
37		~~- $projectvalue = $wgRequest->getVal( 'rc-testwiki-project', $wgUser->mOptions[$wmincPref . '-project'] );~~
38		~~- $langcodevalue = $wgRequest->getVal( 'rc-testwiki-code', $wgUser->mOptions[$wmincPref . '-code'] );~~
	38	+ $projectvalue = $wgRequest->getVal( 'rc-testwiki-project', $wgUser->getOption($wmincPref . '-project') );
	39	+ $langcodevalue = $wgRequest->getVal( 'rc-testwiki-code', $wgUser->getOption($wmincPref . '-code') );
39	40	$opts->consumeValue( 'rc-testwiki-project' );
40	41	$opts->consumeValue( 'rc-testwiki-code' );
41	42	$label = Xml::label( wfMsg( 'wminc-testwiki' ), 'rc-testwiki' );
Index: trunk/extensions/WikimediaIncubator/WikimediaIncubator.i18n.php
—	—	@@ -13,6 +13,8 @@
14	14	$messages['en'] = array(
15	15	'wminc-desc' => 'Test wiki system for Wikimedia Incubator',
16	16	'wminc-viewuserlang' => 'Look up user language and test wiki',
	17	+ 'wminc-viewuserlang-user' => 'Username:',
	18	+ 'wminc-viewuserlang-go' => 'Go',
17	19	'wminc-testwiki' => 'Test wiki:',
18	20	'wminc-testwiki-none' => 'None/All',
19	21	'wminc-prefinfo-language' => 'Your interface language - independent from your test wiki',

Follow-up revisions

Revision	Commit summary	Author	Date
r83459	actually use the messages (added in r51123)	robin	15:11, 7 March 2011

Past revisions this follows-up on

Revision	Commit summary	Author	Date
r50188	add WikimediaIncubator extension	robin	17:56, 4 May 2009

Comments

#Comment by Krinkle (talk | contribs) 16:28, 5 March 2011

+		$ok = wfMsgHtml( 'go' );
+		$username = wfMsgHtml( 'username' );
+		$inputformtop = wfMsgHtml( 'wminc-viewuserlang' );
(...)
+	'wminc-viewuserlang-user' => 'Username:',
+	'wminc-viewuserlang-go' => 'Go',

Later in that file $ok is still used and the "wminc-viewuserlang-go" message isn't used anywhere. "go" and "username" are re-used instead of extension-specific ones.

If a message is similar to another one, to make it easier for translators tag it with {{Identical| centralnamehere }}</nowki> in the /qqq entry. Next, define the English-message. Translators will then translate it in other languages very quickly since they can base it from the other ones. A central 'identical message' page for 'Go' already exists at TranslateWiki: [[translatewiki:Template:Identical/Go]]. So the 'qqq' entry for 'wminc-viewuserlang-go' would be <code><nowiki>{{Identical|Go}}

#Comment by SPQRobin (talk | contribs) 15:14, 7 March 2011

I added the messages but forgot to replace them in $ok and $username. Fixed in r83459.

And btw, the /qqq entries already include the template "Identical" and some information.

#Comment by Krinkle (talk | contribs) 17:07, 7 March 2011

Cool. Marking ok.

Status & tagging log

17:07, 7 March 2011 Krinkle (talk | contribs) changed the status of r51123 [removed: deferred added: ok]
02:14, 6 June 2009 Tim Starling (talk | contribs) changed the status of r51123 [removed: new added: deferred]