Page MenuHomePhabricator

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

Description

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

Example:

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

Both should trigger error obviously.


Version: unspecified
Severity: major

Details

Reference
bz54904

Event Timeline

bzimport raised the priority of this task from to Normal.
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?

https://www.mediawiki.org/wiki/Help:Extension:ParserFunctions#.23expr

"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

https://gerrit.wikimedia.org/r/406485

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

Reason:
See T56904, as well as what was written before.

https://gerrit.wikimedia.org/r/406485

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.