r82212 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r82211‎ \| r82212 \| r82213 >
Date:	03:11, 16 February 2011
Author:	yaron
Status:	resolved (Comments)
Tags:
Comment:	Added handling for regular expressions
Modified paths:	/trunk/extensions/ReplaceText/SpecialReplaceText.php (modified) (history)

Diff [purge]

Index: trunk/extensions/ReplaceText/SpecialReplaceText.php
—	—	@@ -1,6 +1,7 @@
2		~~-<?php~~
	2	+<?php
3	3
4	4	class ReplaceText extends SpecialPage {
	5	+
5	6	/**
6	7	* Constructor
7	8	*/
—	—	@@ -42,6 +43,7 @@
43	44
44	45	$this->target = $wgRequest->getText( 'target' );
45	46	$this->replacement = $wgRequest->getText( 'replacement' );
	47	+ $this->use_regex = ( $wgRequest->getVal( 'use_regex' ) == 1 );
46	48	$this->category = $wgRequest->getText( 'category' );
47	49	$this->prefix = $wgRequest->getText( 'prefix' );
48	50	$this->edit_pages = ( $wgRequest->getVal( 'edit_pages' ) == 1 );
—	—	@@ -60,6 +62,7 @@
61	63	$replacement_params['user_id'] = $wgUser->getId();
62	64	$replacement_params['target_str'] = $this->target;
63	65	$replacement_params['replacement_str'] = $this->replacement;
	66	+ $replacement_params['use_regex'] = $this->use_regex;
64	67	$replacement_params['edit_summary'] = wfMsgForContent( 'replacetext_editsummary', $this->target, $this->replacement );
65	68	$replacement_params['create_redirect'] = false;
66	69	$replacement_params['watch_page'] = false;
—	—	@@ -85,7 +88,7 @@
86	89	}
87	90	Job::batchInsert( $jobs );
88	91
89		~~- $count = $wgLang->formatNum( count( $jobs ) );~~
	92	+ $count = $wgLang->formatNum( count( $jobs ) );
90	93	$wgOut->addWikiMsg( 'replacetext_success', "<tt><nowiki>{$this->target}</nowiki></tt>", "<tt><nowiki>{$this->replacement}</nowiki></tt>", $count );
91	94
92	95	// Link back
—	—	@@ -93,7 +96,6 @@
94	97	$wgOut->addHTML( $sk->makeKnownLinkObj( $this->getTitle(), wfMsgHtml( 'replacetext_return' ) ) );
95	98	return;
96	99	} elseif ( $wgRequest->getCheck( 'target' ) ) { // very long elseif, look for "end elseif"
97		-
98	100	// first, check that at least one namespace has been
99	101	// picked, and that either editing or moving pages
100	102	// has been selected
—	—	@@ -113,20 +115,24 @@
114	116
115	117	// if user is replacing text within pages...
116	118	if ( $this->edit_pages ) {
117		~~- $res = $this->doSearchQuery( $this->target, $this->selected_namespaces, $this->category, $this->prefix );~~
	119	+ $res = $this->doSearchQuery( $this->target, $this->selected_namespaces, $this->category, $this->prefix , $this->use_regex );
118	120	foreach ( $res as $row ) {
119	121	$title = Title::makeTitleSafe( $row->page_namespace, $row->page_title );
120		~~- $context = $this->extractContext( $row->old_text, $this->target );~~
	122	+ $context = $this->extractContext( $row->old_text, $this->target, $this->use_regex );
121	123	$titles_for_edit[] = array( $title, $context );
122	124	}
123	125	}
124	126	if ( $this->move_pages ) {
125		~~- $res = $this->getMatchingTitles( $this->target, $this->selected_namespaces, $this->category, $this->prefix );~~
	127	+ $res = $this->getMatchingTitles( $this->target, $this->selected_namespaces, $this->category, $this->prefix, $this->use_regex );
126	128	foreach ( $res as $row ) {
127	129	$title = Title::makeTitleSafe( $row->page_namespace, $row->page_title );
128	130	// see if this move can happen
129	131	$cur_page_name = str_replace( '_', ' ', $row->page_title );
130		~~- $new_page_name = str_replace( $this->target, $this->replacement, $cur_page_name );~~
	132	+ if ( $this->use_regex ) {
	133	+ $new_page_name = preg_replace( "/".$this->target."/U", $this->replacement, $cur_page_name );
	134	+ } else {
	135	+ $new_page_name = str_replace( $this->target, $this->replacement, $cur_page_name );
	136	+ }
131	137	$new_title = Title::makeTitleSafe( $row->page_namespace, $new_page_name );
132	138	$err = $title->isValidMoveOperation( $new_title );
133	139	if ( $title->userCan( 'move', true ) && !is_array( $err ) ) {
—	—	@@ -158,16 +164,16 @@
159	165	//FIXME: raw html message
160	166	$wgOut->addHTML( '<p>' . $sk->makeKnownLinkObj( $this->getTitle(), wfMsg( 'replacetext_return' ) ) . '</p>' );
161	167	} else {
162		~~- // show a warning message if the replacement string~~
163		~~- // is either blank or found elsewhere on the wiki~~
164		~~- // (since undoing the replacement would be~~
165		~~- // difficult in either case)~~
	168	+ // Show a warning message if the replacement
	169	+ // string is either blank or found elsewhere on
	170	+ // the wiki (since undoing the replacement
	171	+ // would be difficult in either case).
166	172	$warning_msg = null;
167	173
168	174	if ( $this->replacement === '' ) {
169	175	$warning_msg = wfMsg('replacetext_blankwarning');
170	176	} elseif ( count( $titles_for_edit ) > 0 ) {
171		~~- $res = $this->doSearchQuery( $this->replacement, $this->selected_namespaces, $this->category, $this->prefix );~~
	177	+ $res = $this->doSearchQuery( $this->replacement, $this->selected_namespaces, $this->category, $this->prefix, $this->use_regex );
172	178	$count = $res->numRows();
173	179	if ( $count > 0 ) {
174	180	$warning_msg = wfMsgExt( 'replacetext_warning', 'parsemag',
—	—	@@ -176,7 +182,7 @@
177	183	);
178	184	}
179	185	} elseif ( count( $titles_for_move ) > 0 ) {
180		~~- $res = $this->getMatchingTitles( $this->replacement, $this->selected_namespaces, $this->category, $this->prefix );~~
	186	+ $res = $this->getMatchingTitles( $this->replacement, $this->selected_namespaces, $this->category, $this->prefix, $this->use_regex );
181	187	$count = $res->numRows();
182	188	if ( $count > 0 ) {
183	189	$warning_msg = wfMsgExt( 'replacetext_warning', 'parsemag',
—	—	@@ -200,7 +206,7 @@
201	207	}
202	208
203	209	function showForm( $warning_msg = null ) {
204		~~- global $wgOut;~~
	210	+ global $wgOut;
205	211	$wgOut->addHTML(
206	212	Xml::openElement( 'form', array( 'id' => 'powersearch', 'action' => $this->getTitle()->getFullUrl(), 'method' => 'post' ) ) .
207	213	Xml::hidden( 'title', $this->getTitle()->getPrefixedText() ) .
—	—	@@ -223,8 +229,13 @@
224	230	$wgOut->addHTML( '</td><td>' );
225	231	$wgOut->addHTML( Xml::textarea( 'replacement', $this->replacement, 50, 2, array( 'style' => 'width: auto;' ) ) );
226	232	$wgOut->addHTML( '</td></tr></table>' );
	233	+ $wgOut->addHTML( Xml::tags( 'p', null,
	234	+ Xml::checkLabel( wfMsg( 'replacetext_useregex' ), 'use_regex', 'use_regex' ) ) . "\n" .
	235	+ Xml::element( 'p', array( 'style' => 'font-style: italic' ),
	236	+ wfMsg( 'replacetext_regexdocu' ) )
	237	+ );
227	238
228		~~- // the interface is heavily based on the one in Special:Search~~
	239	+ // The interface is heavily based on the one in Special:Search.
229	240	$search_label = wfMsg( 'powersearch-ns' );
230	241	$namespaces = SearchEngine::searchableNamespaces();
231	242	$tables = $this->namespaceTables( $namespaces );
—	—	@@ -233,10 +244,10 @@
234	245	"<fieldset id=\"mw-searchoptions\">\n" .
235	246	Xml::tags( 'h4', null, wfMsgExt( 'powersearch-ns', array( 'parseinline' ) ) )
236	247	);
237		~~- // the ability to select/unselect groups of namespaces in the~~
	248	+ // The ability to select/unselect groups of namespaces in the
238	249	// search interface exists only in some skins, like Vector -
239	250	// check for the presence of the 'powersearch-togglelabel'
240		~~- // message to see if we can use this functionality here~~
	251	+ // message to see if we can use this functionality here.
241	252	if ( !wfEmptyMsg( 'powersearch-togglelabel', wfMsg( 'powersearch-togglelabel' ) ) ) {
242	253	$wgOut->addHTML(
243	254	Xml::tags(
—	—	@@ -262,7 +273,6 @@
263	274	)
264	275	)
265	276	)
266		-
267	277	);
268	278	} // end if
269	279	$wgOut->addHTML(
—	—	@@ -334,7 +344,6 @@
335	345	return $tables;
336	346	}
337	347
338		-
339	348	function pageListForm( $titles_for_edit, $titles_for_move, $unmoveable_titles ) {
340	349	global $wgOut, $wgLang, $wgScript;
341	350
—	—	@@ -345,7 +354,8 @@
346	355	Xml::openElement( 'form', $formOpts ) .
347	356	Xml::hidden( 'title', $this->getTitle()->getPrefixedText() ) .
348	357	Xml::hidden( 'target', $this->target ) .
349		~~- Xml::hidden( 'replacement', $this->replacement )~~
	358	+ Xml::hidden( 'replacement', $this->replacement ) .
	359	+ Xml::hidden( 'use_regex', $this->use_regex )
350	360	);
351	361
352	362	$js = file_get_contents( dirname( __FILE__ ) . '/ReplaceText.js' );
—	—	@@ -389,7 +399,8 @@
390	400	Xml::hidden( 'replace', 1 )
391	401	);
392	402
393		~~- // only show "invert selections" link if there are more than five pages~~
	403	+ // Only show "invert selections" link if there are more than
	404	+ // five pages.
394	405	if ( count( $titles_for_edit ) + count( $titles_for_move ) > 5 ) {
395	406	$buttonOpts = array(
396	407	'type' => 'button',
—	—	@@ -415,18 +426,24 @@
416	427	}
417	428	}
418	429
419		-
420	430	/**
421	431	* Extract context and highlights search text
	432	+ *
	433	+ * TODO: The bolding needs to be fixed for regular expressions.
422	434	*/
423		~~- function extractContext( $text, $target ) {~~
	435	+ function extractContext( $text, $target, $use_regex = false ) {
424	436	global $wgLang;
	437	+
425	438	$cw = $this->user->getOption( 'contextchars', 40 );
426	439
427	440	// Get all indexes
428		~~- $targetq = preg_quote( $target, '/' );~~
429		~~- preg_match_all( "/$targetq/", $text, $matches, PREG_OFFSET_CAPTURE );~~
430		-
	441	+ if ( $use_regex ) {
	442	+ preg_match_all( "/$target/", $text, $matches, PREG_OFFSET_CAPTURE );
	443	+ } else {
	444	+ $targetq = preg_quote( $target, '/' );
	445	+ preg_match_all( "/$targetq/", $text, $matches, PREG_OFFSET_CAPTURE );
	446	+ }
	447	+
431	448	$poss = array();
432	449	foreach ( $matches[0] as $_ ) {
433	450	$poss[] = $_[1];
—	—	@@ -454,11 +471,16 @@
455	472	list( $index, $len, ) = $_;
456	473	$context .= self::convertWhiteSpaceToHTML( $wgLang->truncate( substr( $text, 0, $index ), - $cw ) );
457	474	$snippet = self::convertWhiteSpaceToHTML( substr( $text, $index, $len ) );
458		~~- $targetq = preg_quote( self::convertWhiteSpaceToHTML( $target ), '/' );~~
459		~~- $context .= preg_replace( "/$targetq/i", '<span class="searchmatch">\0</span>', $snippet );~~
	475	+ if ( $use_regex ) {
	476	+ $targetStr = "/$target/U";
	477	+ } else {
	478	+ $targetq = preg_quote( self::convertWhiteSpaceToHTML( $target ), '/' );
	479	+ $targetStr = "/$targetq/i";
	480	+ }
	481	+ $context .= preg_replace( $targetStr, '<span class="searchmatch">\0</span>', $snippet );
	482	+
460	483	$context .= self::convertWhiteSpaceToHTML( $wgLang->truncate( substr( $text, $index + $len ), $cw ) );
461	484	}
462		-
463	485	return $context;
464	486	}
465	487
—	—	@@ -471,27 +493,28 @@
472	494	return $msg;
473	495	}
474	496
475		~~- function getMatchingTitles( $str, $namespaces, $category, $prefix ) {~~
	497	+ function getMatchingTitles( $str, $namespaces, $category, $prefix, $use_regex = false ) {
476	498	$dbr = wfGetDB( DB_SLAVE );
477	499
478	500	$str = Title::newFromText( $str )->getDbKey();
479	501
480	502	$tables = array( 'page' );
481	503	$vars = array( 'page_title', 'page_namespace' );
482		~~- // anyString() method was added in MW 1.16~~
483		~~- if ( method_exists( $dbr, 'anyString' ) ) {~~
484		~~- $any = $dbr->anyString();~~
485		~~- $conds = array(~~
486		~~- 'page_title ' . $dbr->buildLike( $any, $str, $any ),~~
487		~~- "page_namespace IN ({$dbr->makeList( $namespaces )})",~~
488		~~- );~~
	504	+ if ( $use_regex ) {
	505	+ $comparisonCond = "page_title REGEXP '$str'";
489	506	} else {
490		~~- $include_ns = $dbr->makeList( $namespaces );~~
491		~~- $conds = array(~~
492		~~- "page_title LIKE '%$str%'",~~
493		~~- "page_namespace IN ($include_ns)",~~
494		~~- );~~
	507	+ // anyString() method was added in MW 1.16
	508	+ if ( method_exists( $dbr, 'anyString' ) ) {
	509	+ $any = $dbr->anyString();
	510	+ $comparisonCond = 'page_title ' . $dbr->buildLike( $any, $str, $any );
	511	+ } else {
	512	+ $comparisonCond = "page_title LIKE '%$str%'";
	513	+ }
495	514	}
	515	+ $conds = array(
	516	+ $comparisonCond,
	517	+ "page_namespace IN ({$dbr->makeList( $namespaces )})",
	518	+ );
496	519
497	520	$this->categoryCondition( $category, $tables, $conds );
498	521	$this->prefixCondition( $prefix, $conds );
—	—	@@ -500,30 +523,27 @@
501	524	return $dbr->select( $tables, $vars, $conds, __METHOD__ , $sort );
502	525	}
503	526
504		~~- function doSearchQuery( $search, $namespaces, $category, $prefix ) {~~
	527	+ function doSearchQuery( $search, $namespaces, $category, $prefix, $use_regex = false ) {
505	528	$dbr = wfGetDB( DB_SLAVE );
506		-
507	529	$tables = array( 'page', 'revision', 'text' );
508	530	$vars = array( 'page_id', 'page_namespace', 'page_title', 'old_text' );
509		~~- // anyString() method was added in MW 1.16~~
510		~~- if ( method_exists( $dbr, 'anyString' ) ) {~~
511		~~- $any = $dbr->anyString();~~
512		~~- $conds = array(~~
513		~~- 'old_text ' . $dbr->buildLike( $any, $search, $any ),~~
514		~~- "page_namespace IN ({$dbr->makeList( $namespaces )})",~~
515		~~- 'rev_id = page_latest',~~
516		~~- 'rev_text_id = old_id'~~
517		~~- );~~
	531	+ if ( $use_regex ) {
	532	+ $comparisonCond = "old_text REGEXP '$search'";
518	533	} else {
519		~~- $any = $dbr->anyString();~~
520		~~- $include_ns = $dbr->makeList( $namespaces );~~
521		~~- $conds = array(~~
522		~~- "old_text " . $dbr->buildLike( $any, $search, $any ),~~
523		~~- "page_namespace IN ($include_ns)",~~
524		~~- 'rev_id = page_latest',~~
525		~~- 'rev_text_id = old_id'~~
526		~~- );~~
	534	+ // anyString() method was added in MW 1.16
	535	+ if ( method_exists( $dbr, 'anyString' ) ) {
	536	+ $any = $dbr->anyString();
	537	+ $comparisonCond = 'old_text ' . $dbr->buildLike( $any, $search, $any );
	538	+ } else {
	539	+ $comparisonCond = "old_text LIKE '%$search%'";
	540	+ }
527	541	}
	542	+ $conds = array(
	543	+ $comparisonCond,
	544	+ "page_namespace IN ({$dbr->makeList( $namespaces )})",
	545	+ 'rev_id = page_latest',
	546	+ 'rev_text_id = old_id'
	547	+ );
528	548
529	549	$this->categoryCondition( $category, $tables, $conds );
530	550	$this->prefixCondition( $prefix, $conds );

Follow-up revisions

Revision	Commit summary	Author	Date
r82240	Partly revert r82212: Remove spurios character	raymond	13:22, 16 February 2011
r82276	Follow-up to r82212 - simplified and improved namespace check within SQL	yaron	19:04, 16 February 2011
r83707	Made some improvements to SQL, including use of addQuotes() - follow-up to r8...	yaron	18:39, 11 March 2011
r84054	Added escaping so that a search string containing a space can be used on page...	yaron	20:30, 15 March 2011

Comments

#Comment by Nikerabbit (talk | contribs) 07:36, 16 February 2011

Some kind of space character was inserted before <?php

$this->target is not escaped properly in the regex.

+"page_namespace IN ({$dbr->makeList( $namespaces )})",

Is better done 'page_namespace' => $namespaces,

Looks like this revision introduces SQL injection.

#Comment by Raymond (talk | contribs) 15:57, 15 March 2011

"Replace text in page titles, when possible" is broken when "Original text:" contains (space), -, _ (at least these charaters).

#Comment by Yaron Koren (talk | contribs) 20:35, 15 March 2011

Thanks for the note - I fixed handling for spaces. Underlines still don't work, but I don't think they should, really... hyphens/dashes, on the other hand, seemed to work fine even beforehand.

#Comment by Yaron Koren (talk | contribs) 10:16, 16 August 2011

Changing status from "fixme" back to "new" - I think the bugs found were all addressed.

Status & tagging log

16:11, 16 August 2011 😂 (talk | contribs) changed the tags for r82212 [removed: nodeploy]
16:11, 16 August 2011 😂 (talk | contribs) changed the status of r82212 [removed: new added: resolved]
10:16, 16 August 2011 Yaron Koren (talk | contribs) changed the status of r82212 [removed: fixme added: new]
23:25, 11 August 2011 😂 (talk | contribs) changed the tags for r82212 [added: nodeploy]
07:36, 16 February 2011 Nikerabbit (talk | contribs) changed the status of r82212 [removed: new added: fixme]