From 26d74ec6676dbb009d5edd5e959ecc43718f37e1 Mon Sep 17 00:00:00 2001 From: Eugene Fidelin Date: Sat, 22 Nov 2014 22:21:23 +0100 Subject: [PATCH 1/2] Fix that empty default value was converted to an empty array despite of strict mode If Request parameter is an array without default value, then an error should be thrown that "parameter value is not an array" in strict mode --- Request/ParamFetcher.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Request/ParamFetcher.php b/Request/ParamFetcher.php index 24eea53ca..de537364d 100644 --- a/Request/ParamFetcher.php +++ b/Request/ParamFetcher.php @@ -110,14 +110,14 @@ public function get($name, $strict = null) $default = $config->default; $paramType = $config instanceof QueryParam ? 'Query' : 'Request'; - if ($config->array) { - $default = (array) $default; - } - if (null === $strict) { $strict = $config->strict; } + if ($config->array && (null !== $default || !$strict)) { + $default = (array) $default; + } + if ($config instanceof RequestParam) { $param = $this->request->request->get($config->getKey(), $default); } elseif ($config instanceof QueryParam) { @@ -130,7 +130,7 @@ public function get($name, $strict = null) if (!is_array($param)) { if ($strict) { throw new BadRequestHttpException( - sprintf("% parameter value of '%s' is not an array", $paramType, $name) + sprintf("%s parameter value of '%s' is not an array", $paramType, $name) ); } From ec90c29a560a80f015b987878d3d7b54afe260e4 Mon Sep 17 00:00:00 2001 From: Eugene Fidelin Date: Tue, 2 Dec 2014 13:58:13 +0100 Subject: [PATCH 2/2] Add test that error is thrown when array parameter is missing in request --- Tests/Request/ParamFetcherTest.php | 64 ++++++++++++++++-------------- 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/Tests/Request/ParamFetcherTest.php b/Tests/Request/ParamFetcherTest.php index cad074a64..790b04f50 100644 --- a/Tests/Request/ParamFetcherTest.php +++ b/Tests/Request/ParamFetcherTest.php @@ -109,6 +109,10 @@ public function setup() $annotations['biz']->nullable = true; $annotations['biz']->description = 'A scalar param with an explicitly defined null default'; + $annotations['arr'] = new RequestParam(); + $annotations['arr']->name = 'arr'; + $annotations['arr']->array = true; + $this->paramReader ->expects($this->any()) ->method('read') @@ -174,23 +178,23 @@ public static function validatesConfiguredParamDataProvider() array( // check that non-strict missing params take default value 'foo', '1', - array('foo' => '1', 'bar' => '2', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null), + array('foo' => '1', 'bar' => '2', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()), array(), - array('bar' => '2', 'baz' => '4'), + array('bar' => '2', 'baz' => '4', 'arr' => array()), ), array( // pass Param in GET 'foo', '42', - array('foo' => '42', 'bar' => '2', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null), + array('foo' => '42', 'bar' => '2', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()), array('foo' => '42'), - array('bar' => '2', 'baz' => '4'), + array('bar' => '2', 'baz' => '4', 'arr' => array()), ), array( // check that invalid non-strict params take default value 'foo', '1', - array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null), + array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()), array('foo' => 'bar'), - array('bar' => '1', 'baz' => '4'), + array('bar' => '1', 'baz' => '4', 'arr' => array()), function (\PHPUnit_Framework_MockObject_MockObject $validator, \PHPUnit_Framework_TestCase $self) { $errors = new ConstraintViolationList(array( new ConstraintViolation("expected error", null, array(), null, null, null), @@ -210,31 +214,31 @@ function (\PHPUnit_Framework_MockObject_MockObject $validator, \PHPUnit_Framewor array( // invalid array 'buzz', array(1), - array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null), + array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()), array('buzz' => 'invaliddata'), - array('bar' => '1', 'baz' => '4'), + array('bar' => '1', 'baz' => '4', 'arr' => array()), ), array( // invalid array (multiple depth) 'buzz', array(1), - array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null), + array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()), array('buzz' => array(array(1))), - array('bar' => '1', 'baz' => '4'), + array('bar' => '1', 'baz' => '4', 'arr' => array()), ), array( // multiple array 'buzz', array(2, 3, 4), - array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array(), 'boozz' => null, 'biz' => null), + array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()), array('buzz' => array(2, 3, 4)), - array('bar' => '1', 'baz' => '4'), + array('bar' => '1', 'baz' => '4', 'arr' => array()), ), array( // multiple array with one invalid value 'buzz', array(2, 1, 4), - array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 1, 4), 'boo' => array(), 'boozz' => null, 'biz' => null), + array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 1, 4), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()), array('buzz' => array(2, 'invaliddata', 4)), - array('bar' => '1', 'baz' => '4'), + array('bar' => '1', 'baz' => '4', 'arr' => array()), function (\PHPUnit_Framework_MockObject_MockObject $validator, \PHPUnit_Framework_TestCase $self) { $errors = new ConstraintViolationList(array( new ConstraintViolation("expected error", null, array(), null, null, null), @@ -254,44 +258,44 @@ function (\PHPUnit_Framework_MockObject_MockObject $validator, \PHPUnit_Framewor array( // Array not provided in GET query 'boo', array(), - array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array(), 'boozz' => null, 'biz' => null), + array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()), array('buzz' => array(2, 3, 4)), - array('bar' => '1', 'baz' => '4'), + array('bar' => '1', 'baz' => '4', 'arr' => array()), ), array( // QueryParam provided in GET query but as a scalar 'boo', array(), - array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array(), 'boozz' => null, 'biz' => null), + array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()), array('buzz' => array(2, 3, 4), 'boo' => 'scalar'), - array('bar' => '1', 'baz' => '4'), + array('bar' => '1', 'baz' => '4', 'arr' => array()), ), array( // QueryParam provided in GET query with valid values 'boo', array('1', 'foo', 5), - array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => null, 'biz' => null), + array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => null, 'biz' => null, 'arr' => array()), array('buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5)), - array('bar' => '1', 'baz' => '4'), + array('bar' => '1', 'baz' => '4', 'arr' => array()), ), array( // QueryParam provided in GET query with valid values 'boozz', null, - array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => null, 'biz' => null), + array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => null, 'biz' => null, 'arr' => array()), array('buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5)), - array('bar' => '1', 'baz' => '4'), + array('bar' => '1', 'baz' => '4', 'arr' => array()), ), array( // QueryParam provided in GET query with valid values 'boozz', 5, - array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => 5, 'biz' => null), + array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => 5, 'biz' => null, 'arr' => array()), array('buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => 5), - array('bar' => '1', 'baz' => '4', 'boozz' => 5), + array('bar' => '1', 'baz' => '4', 'boozz' => 5, 'arr' => array()), ), ); } public function testValidatesAddParam() { - $queryFetcher = $this->getParamFetcher(array(), array('bar' => '2', 'baz' => '4','bub' => '10')); + $queryFetcher = $this->getParamFetcher(array(), array('bar' => '2', 'baz' => '4','bub' => '10', 'arr' => array())); $queryFetcher->setController($this->controller); $runtimeParam = new RequestParam(); @@ -301,7 +305,7 @@ public function testValidatesAddParam() $queryFetcher->addParam($runtimeParam); $this->assertEquals(10, $queryFetcher->get('bub')); - $this->assertEquals(array('foo' => '1', 'bar' => '2', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null,'bub' => 10), $queryFetcher->all()); + $this->assertEquals(array('foo' => '1', 'bar' => '2', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'bub' => 10, 'arr' => array()), $queryFetcher->all()); } public function testValidatesConfiguredParamStrictly() @@ -370,10 +374,10 @@ public function testExceptionOnValidatesFailure($query, $request, $param, \Closu public static function exceptionOnValidatesFailureDataProvider() { return array( - array( // test missing strict param - array(), - array(), - 'bar', + array( // test missing 'arr' request param of array type + array('boozz' => 'foo'), + array('bar' => 'foo', 'baz' => 'foo'), + 'arr', ), array( // test invalid strict param array(),