From a7a5c414bb1191b5eea0931c8f7718a0568cc621 Mon Sep 17 00:00:00 2001 From: Ori Livneh Date: Thu, 2 Jan 2014 21:57:23 -0800 Subject: [PATCH] MWMessagePack: improvements to test suite, exception handling, array detection * Throw InvalidArgumentException * Use data provider in unit tests * Detect associative arrays without copying Per Tyler's post-merge review of Id2833c5a9. Change-Id: Iec6b135238ca5da3002944066843102f0ae8d23d --- includes/libs/MWMessagePack.php | 27 +++-- .../includes/libs/MWMessagePackTest.php | 104 +++++++++--------- 2 files changed, 69 insertions(+), 62 deletions(-) diff --git a/includes/libs/MWMessagePack.php b/includes/libs/MWMessagePack.php index b44635df47..c61e8f80cf 100644 --- a/includes/libs/MWMessagePack.php +++ b/includes/libs/MWMessagePack.php @@ -35,7 +35,7 @@ class MWMessagePack { /** @var boolean|null Whether current system is bigendian. **/ - public static $bigendian; + public static $bigendian = null; /** * Encode a value using MessagePack @@ -46,6 +46,7 @@ class MWMessagePack { * * @param mixed $value * @return string + * @throws InvalidArgumentException if $value is an unsupported type or too long a string */ public static function pack( $value ) { if ( self::$bigendian === null ) { @@ -74,7 +75,7 @@ class MWMessagePack { } elseif ( $length <= 0xFFFFFFFF ) { return pack( 'CNa*', 0xDB, $length, $value ); } - throw new LengthException( "String too long: $length (max: 4294967295)." ); + throw new InvalidArgumentException( __METHOD__ . ": string too long (length: $length; max: 4294967295)" ); case 'integer': if ( $value >= 0 ) { @@ -113,7 +114,7 @@ class MWMessagePack { } if ( $value >= -0x8000 ) { // int16 - $p = pack('s',$value); + $p = pack( 's', $value ); return self::$bigendian ? pack( 'Ca2', 0xD1, $p ) : pack( 'Ca2', 0xD1, strrev( $p ) ); @@ -135,16 +136,24 @@ class MWMessagePack { : pack( 'Ca4a4', 0xD3, strrev( $p2 ), strrev( $p1 ) ); } } - throw new LengthException( 'Invalid integer: ' . $value ); + throw new InvalidArgumentException( __METHOD__ . ": invalid integer '$value'" ); case 'array': - $associative = array_values( $value ) !== $value; - $length = count( $value ); $buffer = ''; - + $length = count( $value ); if ( $length > 0xFFFFFFFF ) { - throw new LengthException( "Array too long: $length (max: 4294967295)." ); + throw new InvalidArgumentException( __METHOD__ . ": array too long (length: $length, max: 4294967295)" ); + } + + $index = 0; + foreach ( $value as $k => $v ) { + if ( $index !== $k || $index === $length ) { + break; + } else { + $index++; + } } + $associative = $index !== $length; if ( $associative ) { if ( $length < 16 ) { @@ -173,7 +182,7 @@ class MWMessagePack { return $buffer; default: - throw new LengthException( 'Unsupported type: ' . gettype( $value ) ); + throw new InvalidArgumentException( __METHOD__ . ': unsupported type ' . gettype( $value ) ); } } } diff --git a/tests/phpunit/includes/libs/MWMessagePackTest.php b/tests/phpunit/includes/libs/MWMessagePackTest.php index de5848d8a9..b99ef86549 100644 --- a/tests/phpunit/includes/libs/MWMessagePackTest.php +++ b/tests/phpunit/includes/libs/MWMessagePackTest.php @@ -5,68 +5,66 @@ */ class MWMessagePackTest extends MediaWikiTestCase { - /* @var array Array of test cases, keyed by type. Each type is an array of - * (value, expected encoding as hex string). The expected values were - * generated using , which - * includes a serialization function. + /** + * Provides test cases for MWMessagePackTest::testMessagePack + * + * Returns an array of test cases. Each case is an array of (type, value, + * expected encoding as hex string). The expected values were generated + * using , which includes a + * serialization function. */ - public $data = array( - 'integer' => array( - array( 0, '00' ), - array( 1, '01' ), - array( 5, '05' ), - array( -1, 'ff' ), - array( -2, 'fe' ), - array( 35, '23' ), - array( -35, 'd0dd' ), - array( 128, 'cc80' ), - array( -128, 'd080' ), - array( 1000, 'cd03e8' ), - array( -1000, 'd1fc18' ), - array( 100000, 'ce000186a0' ), - array( -100000, 'd2fffe7960' ), - array( 10000000000, 'cf00000002540be400' ), - array( -10000000000, 'd3fffffffdabf41c00' ), - array( -223372036854775807, 'd3fce66c50e2840001' ), - array( -9223372036854775807, 'd38000000000000001' ), - ), - 'NULL' => array( - array( null, 'c0' ), - ), - 'boolean' => array( - array( true, 'c3' ), - array( false, 'c2' ), - ), - 'double' => array( - array( 0.1, 'cb3fb999999999999a' ), - array( 1.1, 'cb3ff199999999999a' ), - array( 123.456, 'cb405edd2f1a9fbe77' ), - ), - 'string' => array( - array( '', 'a0' ), - array( 'foobar', 'a6666f6f626172' ), + public function provider() { + return array( + array( 'nil', null, 'c0' ), + array( 'bool', true, 'c3' ), + array( 'bool', false, 'c2' ), + array( 'positive fixnum', 0, '00' ), + array( 'positive fixnum', 1, '01' ), + array( 'positive fixnum', 5, '05' ), + array( 'positive fixnum', 35, '23' ), + array( 'uint 8', 128, 'cc80' ), + array( 'uint 16', 1000, 'cd03e8' ), + array( 'uint 32', 100000, 'ce000186a0' ), + array( 'uint 64', 10000000000, 'cf00000002540be400' ), + array( 'negative fixnum', -1, 'ff' ), + array( 'negative fixnum', -2, 'fe' ), + array( 'int 8', -128, 'd080' ), + array( 'int 8', -35, 'd0dd' ), + array( 'int 16', -1000, 'd1fc18' ), + array( 'int 32', -100000, 'd2fffe7960' ), + array( 'int 64', -10000000000, 'd3fffffffdabf41c00' ), + array( 'int 64', -223372036854775807, 'd3fce66c50e2840001' ), + array( 'int 64', -9223372036854775807, 'd38000000000000001' ), + array( 'double', 0.1, 'cb3fb999999999999a' ), + array( 'double', 1.1, 'cb3ff199999999999a' ), + array( 'double', 123.456, 'cb405edd2f1a9fbe77' ), + array( 'fix raw', '', 'a0' ), + array( 'fix raw', 'foobar', 'a6666f6f626172' ), array( + 'raw 16', 'Lorem ipsum dolor sit amet amet.', 'da00204c6f72656d20697073756d20646f6c6f722073697420616d657420616d65742e' ), - ), - 'array' => array( - array( array( 'abc', 'def', 'ghi' ), '93a3616263a3646566a3676869' ), - array( array( 'one' => 1, 'two' => 2 ), '82a36f6e6501a374776f02' ), - ), - ); + array( + 'fix array', + array( 'abc', 'def', 'ghi' ), + '93a3616263a3646566a3676869' + ), + array( + 'fix map', + array( 'one' => 1, 'two' => 2 ), + '82a36f6e6501a374776f02' + ), + ); + } /** * Verify that values are serialized correctly. * @covers MWMessagePack::pack + * @dataProvider provider */ - public function testMessagePack() { - foreach( $this->data as $type => $cases ) { - foreach( $cases as $case ) { - list( $value, $expected ) = $case; - $actual = bin2hex( MWMessagePack::pack( $value ) ); - $this->assertEquals( $actual, $expected, $type ); - } - } + public function testPack( $type, $value, $expected ) { + $actual = bin2hex( MWMessagePack::pack( $value ) ); + $this->assertEquals( $actual, $expected, $type ); } } -- 2.20.1