Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed validation for Request parameter that is an array without default value #910

Merged
merged 2 commits into from
Dec 2, 2014
Merged

Fixed validation for Request parameter that is an array without default value #910

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
26d74ec
Fix that empty default value was converted to an empty array despite …
eugef Nov 22, 2014
ec90c29
Add test that error is thrown when array parameter is missing in request
eugef Dec 2, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions Request/ParamFetcher.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
);
}

Expand Down
64 changes: 34 additions & 30 deletions Tests/Request/ParamFetcherTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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),
Expand All @@ -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),
Expand All @@ -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();
Expand All @@ -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()
Expand Down Expand Up @@ -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(),
Expand Down