Page MenuHomePhabricator

{{formatnum}} arguments 'R' and 'NOSEP' should match the entire string, not just part of the string
Closed, ResolvedPublic

Description

[This issue was discovered by https://de.wikipedia.org/wiki/Benutzer:Schnark]


Summary

{{formatnum}} interprets any string containing the letter 'R' as a flag for 'rawsuffix'.

Hence, {{formatnum:1,234.56|any string that has the capital letter 'R'}} is equivalent to {{formatnum:1,234.56|R}}. This also applies to {{formatnum:1,234.56|Random}}, {{formatnum:1,234.56|EVERYTHING}}, etc.. In all cases, 1234.56 will be generated as the output. Instead, the function should discard the argument and output 1,234.56.

The same also applies to the 'NOSEP' argument. For example, {{formatnum:1234.56|any argument that has the string 'NOSEP'}} will output 1234.56. It should output 1,234.56.


Explanation

  • In MessagesEn.php, 'rawsuffix' is defined as 'R'. Note that few languages overrides this magic word.
  • CoreParserFunctions.php defines a function called formatnum which calls "self::matchAgainstMagicword( 'rawsuffix', $arg )"
  • matchAgainstMagicword in turn calls the match function in MagicWord.php
  • MagicWord::match does "preg_match( $this->getRegex(), $text )". For rawsuffix, this will match any string that has the letter 'R'
  • The same situation arises for 'nocommafysuffix' and 'NOSEP'

Recommendation

  • Change the behavior to match the entire string 'R', not just any part of it.

Possible issues

  • This may break existing usage. For example, {{formatnum:1234.56|RAW}} is currently permissible, but should be invalid. This can be mitigated by adding 'RAW' as another magic word for 'rawsuffix' in MessagesEn.php

Version: 1.22.0
Severity: normal

Details

Reference
bz56199

Event Timeline

bzimport raised the priority of this task from to Normal.Nov 22 2014, 2:38 AM
bzimport added a project: MediaWiki-Parser.
bzimport set Reference to bz56199.
bzimport added a subscriber: Unknown Object (MLST).
gnosygnu created this task.Oct 26 2013, 4:21 AM

formatnum is part of core -> moving

There is MagicWord::matchStart which allows to reduce the side effect to a R at the begin.

Using MagicWord::matchStartAndRemove and check, if the rest is a empty string is also possible. Maybe someone knows a better way.

Change 100198 had a related patch set uploaded by Umherirrender:
Raw option of parser functions should match complete word

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

Change 100198 merged by jenkins-bot:
Raw option of parser functions should match complete word

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

successfully merged