Page MenuHomePhabricator

ParserFunctions + Variables: tighter integration.
Closed, DeclinedPublicFeature

Description

Author: van.de.bugger

Description:
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.

  1. New error message introduced: "Unexpected variable".
  1. 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.

  1. 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.


Version: unspecified
Severity: enhancement

Attached:

Details

Reference
bz30039

Event Timeline

bzimport raised the priority of this task from to Low.Nov 21 2014, 11:49 PM
bzimport added a project: ParserFunctions.
bzimport set Reference to bz30039.
bzimport added a subscriber: Unknown Object (MLST).
bzimport created this task.Jul 24 2011, 6:33 PM

happy.melon.wiki wrote:

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.

van.de.bugger wrote:

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.

john wrote:

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.

van.de.bugger wrote:

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.

van.de.bugger wrote:

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.

YaganZ added a subscriber: YaganZ.Aug 25 2016, 12:19 AM

Oh, even six years later I want to note that Variables can double parsing speed.

This is unlikely to be resolved any time soon, as it would require either Hooks in ExprParser at really specific places - neither good style nor convenient, a patch providing them is very unlikely to get merged, or a possibility to replace ExprParser with an inheriting custom parser, which seems really complicated.

Furthermore, I'm not even sure this is a good idea at all, as this can lead to unexpected behavior of the parser becausec of unconventional variable names. No matter what, it would make ExprParser much more fragile.

MGChecker lowered the priority of this task from Low to Lowest.Apr 30 2018, 6:21 AM
MGChecker closed this task as Declined.Aug 9 2018, 12:21 AM

I agree with declining this; there's just too much potential for unwanted Variables expansion caused by lazy/careless naming of Variables, or by intentionally malicious ParserFunction input. To prevent this, you'd have to have some feature for clearly marking what's meant to be a variable name and what's meant to just be text, and at that point you're back to the present situation.

Oh, even six years later I want to note that Variables can double parsing speed.

Can you clarify what you meant here? Are you saying that Variables can speed up page parsing, or that it can cause parsing to take twice as long?

I agree with declining this; there's just too much potential for unwanted Variables expansion caused by lazy/careless naming of Variables, or by intentionally malicious ParserFunction input. To prevent this, you'd have to have some feature for clearly marking what's meant to be a variable name and what's meant to just be text, and at that point you're back to the present situation.

Indeed. Additionally, it's highly improbable that we would get alterations to the ParserFunctions extension merged, that would be required to do this task.

Oh, even six years later I want to note that Variables can double parsing speed.

Can you clarify what you meant here? Are you saying that Variables can speed up page parsing, or that it can cause parsing to take twice as long?

Variables can speed up page parsing. Imagine templates that use page names and multiple switches with around 1000 entries to get some data, for example about template colors, and imagine these templates over 100 times on a single page. If you just have to query this information once, you are much faster.

MGChecker changed the subtype of this task from "Task" to "Feature Request".Mar 1 2019, 11:37 PM