Wikimedia Bugzilla is closed!

Created attachment 8406 [details]
Patch

Rather than committing and waiting for review, seemed sensible to create a patch and put here for you to look at Tim, being an area of code I'm not really familiar with

I'd rather see wfMessage being fixed and wfMsg* slowly converted to deprecated wrappers around wfMessage.

(In reply to comment #1)
> Created attachment 8406 [details]
> Patch
> 
> Rather than committing and waiting for review, seemed sensible to create a
> patch and put here for you to look at Tim, being an area of code I'm not really
> familiar with

That's the general idea, but MessageCache::parse() lacks protection against re-entrant calls. You need something like:

if ( $this->mInParser ) {
   return htmlspecialchars( $in );
}
$this->mInParser = true;
$this->mParser->parse(...);
$this->mInParser = false;

Both MessageCache::transform() and MessageCache::parse() should be protected using the same member variable, since they both access the same parser object. You could test it by making a couple of tag hook extensions that call $wgMessageCache->parse().

<foo>
<bar>
[[test]]
</bar>
</foo>

It's easy enough to make a tag hook extension in a few lines of code in LocalSettings.php, you don't need to make a separate file or anything.

Please also patch Message::parseText() to call MessageCache::singleton()->parse(), so that Niklas's idea can easily be done later.

Comment on attachment 8406 [details]
Patch

>Index: GlobalFunctions.php
>===================================================================
>--- GlobalFunctions.php	(revision 86120)
>+++ GlobalFunctions.php	(working copy)
>@@ -716,10 +716,12 @@
>  * @return string
>  */
> function wfMsgWikiHtml( $key ) {
>-	global $wgOut;
>+	global $wgMessageCache;
> 	$args = func_get_args();
> 	array_shift( $args );
>-	return wfMsgReplaceArgs( $wgOut->parse( wfMsgGetKey( $key, true ), /* can't be set to false */ true ), $args );
>+	return wfMsgReplaceArgs(
>+		$wgMessageCache->parse( wfMsgGetKey( $key, true ), null, /* can't be set to false */ true ),
>+		$args );
> }
> 
> /**
>@@ -741,7 +743,7 @@
>  * Behavior for conflicting options (e.g., parse+parseinline) is undefined.
>  */
> function wfMsgExt( $key, $options ) {
>-	global $wgOut;
>+	global $wgMessageCache;
> 
> 	$args = func_get_args();
> 	array_shift( $args );
>@@ -781,9 +783,9 @@
> 	}
> 
> 	if( in_array( 'parse', $options, true ) ) {
>-		$string = $wgOut->parse( $string, true, !$forContent, $langCodeObj );
>+		$string = $wgMessageCache->parse( $string, null, true, !$forContent, $langCodeObj );
> 	} elseif ( in_array( 'parseinline', $options, true ) ) {
>-		$string = $wgOut->parse( $string, true, !$forContent, $langCodeObj );
>+		$string = $wgMessageCache->parse( $string, null, true, !$forContent, $langCodeObj );
> 		$m = array();
> 		if( preg_match( '/^<p>(.*)\n?<\/p>\n?$/sU', $string, $m ) ) {
> 			$string = $m[1];
>Index: MessageCache.php
>===================================================================
>--- MessageCache.php	(revision 86120)
>+++ MessageCache.php	(working copy)
>@@ -102,6 +102,8 @@
> 
> 	/**
> 	 * ParserOptions is lazy initialised.
>+	 *
>+	 * @return ParserOptions
> 	 */
> 	function getParserOptions() {
> 		if ( !$this->mParserOptions ) {
>@@ -220,6 +222,8 @@
> 
> 	/**
> 	 * Set the cache to $cache, if it is valid. Otherwise set the cache to false.
>+	 *
>+	 * @return bool
> 	 */
> 	function setCache( $cache, $code ) {
> 		if ( isset( $cache['VERSION'] ) && $cache['VERSION'] == MSG_CACHE_VERSION ) {
>@@ -734,6 +738,21 @@
> 			return $message;
> 		}
> 
>+		$parser = $this->getParser();
>+		if ( $parser ) {
>+			$popts = $this->getParserOptions();
>+			$popts->setInterfaceMessage( $interface );
>+			$popts->setTargetLanguage( $language );
>+			$popts->setUserLang( $language );
>+			$message = $parser->transformMsg( $message, $popts, $title );
>+		}
>+		return $message;
>+	}
>+
>+	/**
>+	 * @return Parser
>+	 */
>+	function getParser() {
> 		global $wgParser, $wgParserConf;
> 		if ( !$this->mParser && isset( $wgParser ) ) {
> 			# Do some initialisation so that we don't have to do it twice
>@@ -746,16 +765,28 @@
> 			} else {
> 				$this->mParser = clone $wgParser;
> 			}
>-			#wfDebug( __METHOD__ . ": following contents triggered transform: $message\n" );
> 		}
>-		if ( $this->mParser ) {
>-			$popts = $this->getParserOptions();
>-			$popts->setInterfaceMessage( $interface );
>+		return $this->mParser;
>+	}
>+
>+	/**
>+	 * @param $text string
>+	 * @param $title
>+	 * @param $linestart bool
>+	 * @return ParserOutput
>+	 */
>+	public function parse( $text, $title = null, $linestart = true, $interface = false, $language = null  ) {
>+		$parser = $this->getParser();
>+		$popts = $this->getParserOptions();
>+
>+		if ( $interface ) {
>+			$popts->setInterfaceMessage( true );
>+		}
>+		if ( $language !== null ) {
> 			$popts->setTargetLanguage( $language );
>-			$popts->setUserLang( $language );
>-			$message = $this->mParser->transformMsg( $message, $popts, $title );
> 		}
>-		return $message;
>+
>+		return $parser->parse( $text, $title, $popts, $linestart );
> 	}
> 
> 	function disable() {
>Index: OutputPage.php
>===================================================================
>--- OutputPage.php	(revision 86120)
>+++ OutputPage.php	(working copy)
>@@ -792,7 +792,7 @@
> 	 * @param $t Title object
> 	 */
> 	public function setTitle( $t ) {
>-		$this->getContext()->setTitle($t);
>+		$this->getContext()->setTitle( $t );
> 	}
> 
> 	/**

r86304

*** Bug 14959 has been marked as a duplicate of this bug. ***