Last modified: 2014-09-23 19:57:33 UTC
Created attachment 8822 [details] Patch for ParserFunctions. Hi, Currently ParserFunctions have no notion about variables, which resulting in lots of typing and lots of braces, for example: {{ #ifexpr: {{ #var: a }} + {{ #var: a }} > 4 | ... }} I always wanted #expr and #ifexpr functions recognize variables so I can write {{ #ifexpr: a + a > 4 | ... }} Attached patch implements this integration between ParserFunctions and Variables. Changes summary: 1. When if a word is a not predefined expression word (like `pi' or `trunc'), it is checked whether Varaibles extension enabled and whether such a variable exists. If variable exists, its value used. 2. New error message introduced: "Unexpected variable". 3. Second parameter added to ExprParser::doExpression, parser — it is required by Variables (though it is not actually used). For compatibility with older code, parameter is optional. 4. Variables extension is not required, if Variables extension is not enabled, ParserFunctions does not recognize variables but does not issue errors. I have access to SVN. If the patch is generally ok, I can commit it. Thanks, Van.
In installations where ParserFunctions is installed but Variables is not, $wgExtVariables is undefined, and so referencing it is a register_globals vulnerability. The ParserFunctions extension should not contain references to the Variables extension in the same way that core code should not contain references to extensions. You should include hooks in ParserFunctions and hook functions in Variables which handle the actual processing (and could happily be used by other extensions). It's fine to tweak the code flow in ParserFunctions to ensure you can pass the right things to the hook.
Sorry, I am not an experienced PHP developer, so I did not get everything you wrote. As I understand, vulnerability is only possible if Variables extension is not installed AND register_globals is ON. register_globals is OFF by default and I saw a lot of warnings that it should remain be OFF. However, I do not object to introduce a hook if you point me an example what it is. Another thing I though about is fully emulating Variables in ParserFunctions. Variables is trivial extension and is in public domain, so it is possible to just incorporate Variables into ParserFunctions.
Some information about hooks: http://www.mediawiki.org/wiki/Hooks Basically you need a wfRunHooks call somewhere that lets you modify the behaviour of the ParserFunctions call from Variables.
Sorry, did not catch. Manual:Hooks says: > Hooks allow custom code to be executed when one of many defined events (like saving an article or a user logging in) occur. In case of ParserFunctions/Variables integration, I do not see events. In this case one extension (namely, ParserFunctions) directly uses service provided by another extension (namely, Variables). Happy-melon said: > The ParserFunctions extension should not contain references to the Variables extension in the same way that core code should not contain references to extensions. ?? Extensions use services provided by core directly. Extensions use global variables defined in core, like $wgAutoloadClasses, $wgGroupPermissions, $wgContLang, etc. In this particular case, what is "core" and what is "extension"? (1) ParserFunctions extends Variables or (2) Variables extends ParserFunctions? In the first case I do not see any issue if ParserFunctions uses services provided by Variables by using global variable $wgExtVariables. In the seconds case ParserFunctions (namely, expression parser/evaluator) can be extended with plugins to introduce new functions, operators, names... It is an interesting idea -- extensions for extensions... It is suitable for big and complex extensions like SemanticMediaWiki, but ParserFunctions is rather simple extension... Do you really want make a monster from ParserFunctions? To me it is much simpler to move Variables functionality into ParserFunctions.
BTW, I just scanned extension sources checked out from svn. DynamicPageList extension uses Variables directly via $wgExtVariables, as well as another extension, Loops.
rm patch-need-review, this patch isn't going to be accepted. We should switch to Lua anyway instead of inviting new exciting ways to make wikitext even slower.
