Page MenuHomePhabricator

{{#expr:}}/{{#ifexpr:}} incorrectly handles strings with multiple decimal points
Closed, DeclinedPublic


{{#expr:}}/{{#ifexpr:}} incorrectly handles strings with multiple decimal points


  • {{#expr:02.10.2013}} -> 2.1
  • {{#ifexpr:02.10.2013|t|f}} -> t

Both should trigger error obviously.

Version: unspecified
Severity: major


Related Gerrit Patches:
mediawiki/extensions/ParserFunctions : masterFix incorrect handling of strings with multiple decimal points

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 2:35 AM
bzimport added a project: ParserFunctions.
bzimport set Reference to bz54904.
bzimport added a subscriber: Unknown Object (MLST).
Danny_B created this task.Oct 2 2013, 11:41 PM
brion added a comment.Oct 3 2013, 3:38 PM

Why should they trigger errors, exactly? Is there an input format specification we can match behavior to?

"02.10.2013" is not valid mathematical expression -> that's why it should trigger error

02.10 is OK
10.2013 is obviously OK

but 2+ decimal points obviously can't be OK

Rammanojpotla added a subscriber: Rammanojpotla.EditedJan 28 2018, 9:27 AM

when I try solving {{#expr: 12.34 + 1.2.34 }} it gives me output as 13.54 instead of giving an error I think this is also wrong and must give an exception, correct me if I am wrong?

Change 406485 had a related patch set uploaded (by Rammanojpotla; owner: Rammanoj):
[mediawiki/extensions/ParserFunctions@master] Fix incorrect handling of strings with multiple decimal points

Code review in progress?

thiemowmde closed this task as Declined.Jun 12 2018, 6:03 PM
thiemowmde added a subscriber: thiemowmde.

This behavior is extensively used as a feature in (literally) millions of pages. Even if I agree the described behavior is confusing and should better result in an error, it is to late to change this now.

Change 406485 abandoned by Thiemo Kreuz (WMDE):
Fix incorrect handling of strings with multiple decimal points

See T56904, as well as what was written before.

Superyetkin reopened this task as Open.Jun 12 2018, 7:33 PM

I do not think it is too late for such a change. Affected pages can be fixed before any deployment action is taken.

thiemowmde closed this task as Declined.Jun 13 2018, 9:14 AM

If somebody is willing to write the code that is needed to find actual usages of this feature, talk to the affected communities (there are a few hundred the Wikimedia Foundation is responsible for), support them in migrating to other solutions (e.g. Lua), make them understand why the removal of this feature is necessary in the first place, handle the backlash when the communication team is not able to explain exactly that, give up, try again, … I honestly do not think this is worth anybodies time.

If you think the fact that {{#expr:1.2.3}} does not fail is a mistake, then just don't feed 1.2.3 into #expr. You can do something like {{#ifeq: {{#expr: {{{1}}} }} | {{{1}}} | ok | warning }} to detect such cases. You can switch to Lua (MediaWiki-extensions-Scribunto) altogether. You can run a bot, find and cleanup bad input values you don't want to feed to #expr.

I don't see what the benefit of #expr just failing would be. This ticket currently doesn't explain this, and why it's worth it.