Anomie has uploaded a new change for review.
https://gerrit.wikimedia.org/r/236807
Change subject: ApiResult: Fix size checking
......................................................................
ApiResult: Fix size checking
Two bugs here:
* Setting NO_SIZE_CHECK also bypassed validation
* ApiResult::valueSize() didn't handle ApiSerializable, which is fixed
by defining that the value needs to be passed through
ApiResult::validateValue() first.
Bug: T111796
Change-Id: I7c00d8ee53364a26f8f63f82a4d83b92baf5383e
---
M includes/api/ApiResult.php
M tests/phpunit/includes/api/ApiResultTest.php
2 files changed, 41 insertions(+), 6 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/07/236807/1
diff --git a/includes/api/ApiResult.php b/includes/api/ApiResult.php
index 014ca85..608e9e1 100644
--- a/includes/api/ApiResult.php
+++ b/includes/api/ApiResult.php
@@ -288,7 +288,7 @@
* @param int $flags Zero or more OR-ed flags like OVERRIDE |
ADD_ON_TOP.
*/
public static function setValue( array &$arr, $name, $value, $flags = 0
) {
- if ( !( $flags & ApiResult::NO_VALIDATE ) ) {
+ if ( ( $flags & ApiResult::NO_VALIDATE ) !==
ApiResult::NO_VALIDATE ) {
$value = self::validateValue( $value );
}
@@ -402,6 +402,11 @@
$arr = &$this->path( $path, ( $flags & ApiResult::ADD_ON_TOP )
? 'prepend' : 'append' );
if ( $this->checkingSize && !( $flags &
ApiResult::NO_SIZE_CHECK ) ) {
+ // self::valueSize needs the validated value. Then flag
+ // to not re-validate later.
+ $value = self::validateValue( $value );
+ $flags |= ApiResult::NO_VALIDATE;
+
$newsize = $this->size + self::valueSize( $value );
if ( $this->maxSize !== false && $newsize >
$this->maxSize ) {
/// @todo Add i18n message when replacing calls
to ->setWarning()
@@ -1079,14 +1084,12 @@
* or the sum of the strlen()s of the elements if the item is an array.
* @note Once the deprecated public self::size is removed, we can rename
* this back to a less awkward name.
- * @param mixed $value
+ * @param mixed $value Validated value (see self::validateValue())
* @return int
*/
private static function valueSize( $value ) {
$s = 0;
- if ( is_array( $value ) ||
- is_object( $value ) && !is_callable( array( $value,
'__toString' ) )
- ) {
+ if ( is_array( $value ) ) {
foreach ( $value as $k => $v ) {
if ( !self::isMetadataKey( $s ) ) {
$s += self::valueSize( $v );
@@ -1488,7 +1491,7 @@
*/
public static function size( $value ) {
wfDeprecated( __METHOD__, '1.25' );
- return self::valueSize( $value );
+ return self::valueSize( self::validateValue( $value ) );
}
/**
diff --git a/tests/phpunit/includes/api/ApiResultTest.php
b/tests/phpunit/includes/api/ApiResultTest.php
index affb0fa..cff2328 100644
--- a/tests/phpunit/includes/api/ApiResultTest.php
+++ b/tests/phpunit/includes/api/ApiResultTest.php
@@ -181,6 +181,19 @@
);
}
+ ApiResult::setValue( $arr, null, NAN, ApiResult::NO_VALIDATE );
+
+ try {
+ ApiResult::setValue( $arr, null, NAN,
ApiResult::NO_SIZE_CHECK );
+ $this->fail( 'Expected exception not thrown' );
+ } catch ( InvalidArgumentException $ex ) {
+ $this->assertSame(
+ 'Cannot add non-finite floats to ApiResult',
+ $ex->getMessage(),
+ 'Expected exception'
+ );
+ }
+
$arr = array();
$result2 = new ApiResult( 8388608 );
$result2->addValue( null, 'foo', 'bar' );
@@ -408,6 +421,19 @@
);
}
+ $result->addValue( null, null, NAN, ApiResult::NO_VALIDATE );
+
+ try {
+ $result->addValue( null, null, NAN,
ApiResult::NO_SIZE_CHECK );
+ $this->fail( 'Expected exception not thrown' );
+ } catch ( InvalidArgumentException $ex ) {
+ $this->assertSame(
+ 'Cannot add non-finite floats to ApiResult',
+ $ex->getMessage(),
+ 'Expected exception'
+ );
+ }
+
$result->reset();
$result->addParsedLimit( 'foo', 12 );
$this->assertSame( array(
@@ -444,6 +470,12 @@
$result->removeValue( null, 'foo' );
$this->assertTrue( $result->addValue( null, 'foo', '1' ) );
+ $result = new ApiResult( 10 );
+ $obj = new ApiResultTestSerializableObject( 'ok' );
+ $obj->foobar = 'foobaz';
+ $this->assertTrue( $result->addValue( null, 'foo', $obj ) );
+ $this->assertSame( 2, $result->getSize() );
+
$result = new ApiResult( 8388608 );
$result2 = new ApiResult( 8388608 );
$result2->addValue( null, 'foo', 'bar' );
--
To view, visit https://gerrit.wikimedia.org/r/236807
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7c00d8ee53364a26f8f63f82a4d83b92baf5383e
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Anomie <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
