API: ApiResult must validate even when using numeric auto-indexes
authorBrad Jorsch <bjorsch@wikimedia.org>
Wed, 29 Apr 2015 14:39:25 +0000 (10:39 -0400)
committerBrad Jorsch <bjorsch@wikimedia.org>
Wed, 29 Apr 2015 14:43:45 +0000 (10:43 -0400)
Bug: T97490
Change-Id: I5301a615a992b090000a59f86e13b9f78dcd5aec

includes/api/ApiResult.php
tests/phpunit/includes/api/ApiResultTest.php

index 345cf43..044b6e0 100644 (file)
@@ -276,6 +276,10 @@ class ApiResult implements ApiSerializable {
         * @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 ) ) {
+                       $value = self::validateValue( $value );
+               }
+
                if ( $name === null ) {
                        if ( $flags & ApiResult::ADD_ON_TOP ) {
                                array_unshift( $arr, $value );
@@ -285,10 +289,6 @@ class ApiResult implements ApiSerializable {
                        return;
                }
 
-               if ( !( $flags & ApiResult::NO_VALIDATE ) ) {
-                       $value = self::validateValue( $value );
-               }
-
                $exists = isset( $arr[$name] );
                if ( !$exists || ( $flags & ApiResult::OVERRIDE ) ) {
                        if ( !$exists && ( $flags & ApiResult::ADD_ON_TOP ) ) {
index f2fcb4a..f0d8455 100644 (file)
@@ -106,6 +106,16 @@ class ApiResultTest extends MediaWikiTestCase {
                                'Expected exception'
                        );
                }
+               try {
+                       ApiResult::setValue( $arr, null, $fh );
+                       $this->fail( 'Expected exception not thrown' );
+               } catch ( InvalidArgumentException $ex ) {
+                       $this->assertSame(
+                               'Cannot add resource(stream) to ApiResult',
+                               $ex->getMessage(),
+                               'Expected exception'
+                       );
+               }
                try {
                        $obj->file = $fh;
                        ApiResult::setValue( $arr, 'sub', $obj );
@@ -117,6 +127,17 @@ class ApiResultTest extends MediaWikiTestCase {
                                'Expected exception'
                        );
                }
+               try {
+                       $obj->file = $fh;
+                       ApiResult::setValue( $arr, null, $obj );
+                       $this->fail( 'Expected exception not thrown' );
+               } catch ( InvalidArgumentException $ex ) {
+                       $this->assertSame(
+                               'Cannot add resource(stream) to ApiResult',
+                               $ex->getMessage(),
+                               'Expected exception'
+                       );
+               }
                fclose( $fh );
 
                try {
@@ -129,6 +150,16 @@ class ApiResultTest extends MediaWikiTestCase {
                                'Expected exception'
                        );
                }
+               try {
+                       ApiResult::setValue( $arr, null, INF );
+                       $this->fail( 'Expected exception not thrown' );
+               } catch ( InvalidArgumentException $ex ) {
+                       $this->assertSame(
+                               'Cannot add non-finite floats to ApiResult',
+                               $ex->getMessage(),
+                               'Expected exception'
+                       );
+               }
                try {
                        ApiResult::setValue( $arr, 'nan', NAN );
                        $this->fail( 'Expected exception not thrown' );
@@ -139,6 +170,16 @@ class ApiResultTest extends MediaWikiTestCase {
                                'Expected exception'
                        );
                }
+               try {
+                       ApiResult::setValue( $arr, null, NAN );
+                       $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 );
@@ -155,10 +196,14 @@ class ApiResultTest extends MediaWikiTestCase {
                ApiResult::setValue( $arr, 'foo', "foo\x80bar" );
                ApiResult::setValue( $arr, 'bar', "a\xcc\x81" );
                ApiResult::setValue( $arr, 'baz', 74 );
+               ApiResult::setValue( $arr, null, "foo\x80bar" );
+               ApiResult::setValue( $arr, null, "a\xcc\x81" );
                $this->assertSame( array(
                        'foo' => "foo\xef\xbf\xbdbar",
                        'bar' => "\xc3\xa1",
                        'baz' => 74,
+                       0 => "foo\xef\xbf\xbdbar",
+                       1 => "\xc3\xa1",
                ), $arr );
        }
 
@@ -288,6 +333,16 @@ class ApiResultTest extends MediaWikiTestCase {
                                'Expected exception'
                        );
                }
+               try {
+                       $result->addValue( null, null, $fh );
+                       $this->fail( 'Expected exception not thrown' );
+               } catch ( InvalidArgumentException $ex ) {
+                       $this->assertSame(
+                               'Cannot add resource(stream) to ApiResult',
+                               $ex->getMessage(),
+                               'Expected exception'
+                       );
+               }
                try {
                        $obj->file = $fh;
                        $result->addValue( null, 'sub', $obj );
@@ -299,6 +354,17 @@ class ApiResultTest extends MediaWikiTestCase {
                                'Expected exception'
                        );
                }
+               try {
+                       $obj->file = $fh;
+                       $result->addValue( null, null, $obj );
+                       $this->fail( 'Expected exception not thrown' );
+               } catch ( InvalidArgumentException $ex ) {
+                       $this->assertSame(
+                               'Cannot add resource(stream) to ApiResult',
+                               $ex->getMessage(),
+                               'Expected exception'
+                       );
+               }
                fclose( $fh );
 
                try {
@@ -311,6 +377,16 @@ class ApiResultTest extends MediaWikiTestCase {
                                'Expected exception'
                        );
                }
+               try {
+                       $result->addValue( null, null, INF );
+                       $this->fail( 'Expected exception not thrown' );
+               } catch ( InvalidArgumentException $ex ) {
+                       $this->assertSame(
+                               'Cannot add non-finite floats to ApiResult',
+                               $ex->getMessage(),
+                               'Expected exception'
+                       );
+               }
                try {
                        $result->addValue( null, 'nan', NAN );
                        $this->fail( 'Expected exception not thrown' );
@@ -321,6 +397,16 @@ class ApiResultTest extends MediaWikiTestCase {
                                'Expected exception'
                        );
                }
+               try {
+                       $result->addValue( null, null, NAN );
+                       $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 );
@@ -374,10 +460,14 @@ class ApiResultTest extends MediaWikiTestCase {
                $result->addValue( null, 'foo', "foo\x80bar" );
                $result->addValue( null, 'bar', "a\xcc\x81" );
                $result->addValue( null, 'baz', 74 );
+               $result->addValue( null, null, "foo\x80bar" );
+               $result->addValue( null, null, "a\xcc\x81" );
                $this->assertSame( array(
                        'foo' => "foo\xef\xbf\xbdbar",
                        'bar' => "\xc3\xa1",
                        'baz' => 74,
+                       0 => "foo\xef\xbf\xbdbar",
+                       1 => "\xc3\xa1",
                        ApiResult::META_TYPE => 'assoc',
                ), $result->getResultData() );
        }