Page MenuHomePhabricator

The extension doesn't succeed to include the bundled geshi library when a geshi folder is available in the path
Closed, ResolvedPublic

Description

[ PHP error ]

Fatal error: Call to undefined method GeSHi::enable_keyword_links() in /path/to/wiki/extensions/SyntaxHighlight_GeSHi/SyntaxHighlight_GeSHi.class.php on line 278

[ Bug analysis ]

The extension tries to include the local GeSHi library, if it's not already included in the configuration:

/**
 * Initialise messages and ensure the GeSHi class is loaded
 * @return bool
 */
private static function initialise() {
        if( !self::$initialised ) {
                if( !class_exists( 'GeSHi' ) ) {
                        require( 'geshi/geshi.php' );
                }
                self::$initialised = true;
        }
        return true;
}

I've a strange scenario where this doesn't work anymore[*]: when a up to date GeSHi library is available in include_path, it will try to include it instead of the

  • It worked before: I installed a wiki in May 2010 in this same environment with extensive usage of GeSHi (which is coherent with git blame output, it seems this line is from 2010-08-09).

[ Steps to reproduce ]

(1) Install the last GeSHi version in a library accessible in include_path

(2) Install the extension

(3) Edit a page and add a source block, then preview it.

[ Note about include_path ]

PHP include_path ordering isn't relevant here: you can have like .:/usr/local/share/pear:/var/wwwroot/.libs, but that won't help: in this context, . is the MediaWiki core, not the extension one.

[ Side issue. We should always include our copy of the library. ]

This last year, we accepted two code divergences between our bundled GeSHi library and the version upstream to fix bugs. Furthermore, we use a slowly upstream maintained and legacy version, not the last one.

I don't see any rationale to document "Install your copy of GeSHi elsewhere".

[ Suggested fix ]

(1) Code -> require( DIR . '/geshi/geshi.php' );

(2) Documentation -> ask to include GeSHi code in the extension in LocalSettings before the extension instead to edit the extension code with custom path

[ Alternative solution ]

If we really want to go on with this situation where we let people install elsewhere GeSHi, we have a performance problem: my suggested fix will include GeSHi for every page, including for requests not needing the GeSHi library.

We could also in this case create a wgGeshiPath setting, with a null value, include DIR . '/geshi/geshi.php' when it's null, $wgGeshiPath /geshi.php otherwise.


Version: master
Severity: critical

Details

Reference
bz44214

Event Timeline

bzimport raised the priority of this task from to Low.Nov 22 2014, 1:31 AM
bzimport added a project: SyntaxHighlight.
bzimport set Reference to bz44214.

[ Taking this bug ]

To be compatible with PHP 5.2, I won't use DIR in my fix.

Gerrit change 44992.

Patch got merged on March 06th. Assuming this is fixed.