Page MenuHomePhabricator

Segfault observed on parse for certain input text using the old parser
Closed, ResolvedPublic

Description

Author: emil

Description:
http://svn.wikimedia.org/viewvc/mediawiki?view=rev&revision=30144 should be applied to Parser_OldPP.php too. Below is the patch:

Index: includes/parser/Parser_OldPP.php

  • includes/parser/Parser_OldPP.php (revision 40727)

+++ includes/parser/Parser_OldPP.php (working copy)
@@ -578,6 +578,10 @@

        break;
default:
        if( isset( $this->mTagHooks[$tagName] ) ) {

+ # Workaround for PHP bug 35229 and similar
+ if ( !is_callable( $this->mTagHooks[$name] ) ) {
+ throw new MWException( "Tag hook for $name is not callable\n" );
+ }

        $output = call_user_func_array( $this->mTagHooks[$tagName],
                array( $content, $params, $this ) );
} else {

@@ -2999,6 +3003,11 @@

if ( $function ) {
        $funcArgs = array_map( 'trim', $args );
        $funcArgs = array_merge( array( &$this, trim( substr( $part1, $colonPos + 1 ) ) ), $funcArgs );

+
+ # Workaround for PHP bug 35229 and similar
+ if ( !is_callable( $this->mFunctionHooks[$function] ) ) {
+ throw new MWException( "Function hook for $function is not callable\n" );
+ }

$result = call_user_func_array( $this->mFunctionHooks[$function], $funcArgs );
$found = true;

Emil


Version: 1.13.x
Severity: major

Details

Reference
bz15563

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 10:18 PM
bzimport set Reference to bz15563.

emil wrote:

a patch for Parser_OldPP.php

Attached:

emil wrote:

this is a correct patch

the previous patch had a wrong variable name

Attached:

Note I'd like to kill the old parser on 1.14 dev trunk, but the patch should probably go on 1.13 branch.

(In reply to comment #3)

Note I'd like to kill the old parser on 1.14 dev trunk, but the patch should
probably go on 1.13 branch.

Actually kill it, or just not load it by default and keep it around like DatabaseFunctions?

It's already not loaded by default. DatabaseFunctions is just a collection of wrapper functions for a stable interface, it doesn't need any maintenance. Parser_OldPP is large and complex and easily broken by interface changes. So I think deleting it is quite reasonable.

Is there some reason Wikia is still using it?

emil wrote:

It is a problem with migrating so many wikis automatically to the new parser. Many of our communities actually don't understand http://meta.wikimedia.org/wiki/Migration_to_the_new_preprocessor and they don't know what they should change on their pages. They will just think that Wikia screwed up their stuff. So we are still not quite sure how to do it smoothly.

You could write a script to scan your database for HTML output changes, and post lists of affected articles.

emil wrote:

Yes, that's the plan. Initially I had an impression that it would give me too many false positives. See for example: http://muppet.wikia.com/wiki/Special:ParserDiffTest/Wait_Wait..._Don%27t_Tell_Me!

But then I discovered there is a "timing" parameter (http://muppet.wikia.com/wiki/Special:ParserDiffTest/Wait_Wait..._Don%27t_Tell_Me!?timing=1) which for some reason doesn't give the false positive. Additionally I can compare old parser vs. new parser performance across all Wikia articles :)