r107876 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r107875‎ | r107876 | r107877 >
Date:11:02, 3 January 2012
Author:johnduhart
Status:ok (Comments)
Tags:reedy 
Comment:
Bug 33482 - Api incorrectly calls ApiBase::parseMultiValue if allowed values is given as an array

Simply means that if you have an array of acceptable values and you only accept one at a time, you can have pipes in the allowed values.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES-1.19 (modified) (history)
  • /trunk/phase3/includes/api/ApiBase.php (modified) (history)

Diff [purge]

Index: trunk/phase3/RELEASE-NOTES-1.19
——@@ -213,6 +213,8 @@
214214 calculated correctly with respect to timezone
215215 * (bug 32219) InstantCommons now fetches content from Wikimedia Commons using
216216 HTTPS when the local wiki is served over HTTPS
 217+* (bug 33482) - Api incorrectly calls ApiBase::parseMultiValue if allowed
 218+ values is given as an array
217219
218220 === API changes in 1.19 ===
219221 * (bug 19838) siprop=interwikimap can now use the interwiki cache.
Index: trunk/phase3/includes/api/ApiBase.php
——@@ -959,6 +959,11 @@
960960 }
961961
962962 if ( !$allowMultiple && count( $valuesList ) != 1 ) {
 963+ // Bug 33482 - Allow entries with | in them for non-multiple values
 964+ if ( in_array( $value, $allowedValues ) ) {
 965+ return $value;
 966+ }
 967+
963968 $possibleValues = is_array( $allowedValues ) ? "of '" . implode( "', '", $allowedValues ) . "'" : '';
964969 $this->dieUsage( "Only one $possibleValues is allowed for parameter '$valueName'", "multival_$valueName" );
965970 }

Comments

#Comment by Brion VIBBER (talk | contribs)   20:20, 3 January 2012

Bug is listed as 'calls parseMultiValue incorrectly' however this changed code is in parseMultiValue -- so it's obviously not fixing the problem that we shouldn't have gotten here in the first place. Something seems awry?

#Comment by Johnduhart (talk | contribs)   21:57, 3 January 2012

Talk to Niklas about it, this is what he needed and he confirmed that it worked. Probably just didn't know what he needed at the time.

#Comment by Johnduhart (talk | contribs)   16:48, 4 January 2012

Resetting to new for now.

#Comment by Catrope (talk | contribs)   16:50, 4 January 2012

The bug was a bit misnamed. Not calling parseMultiValue() was a possible solution to Niklas's problem, this is another solution, and it's valid. Setting back to OK.

(FTR, the problem was that parameters with PARAM_TYPE set to an array of allowed values but PARAM_ISMULTI set to false would not take values containing pipe characters properly.)

Status & tagging log