Skip to content

Commit 6714a42

Browse files
lsmith77lsmith77
committed
Merge pull request #910 from eugef/patch-1
Fixed validation for Request parameter that is an array without default value
2 parents 221ceea + ec90c29 commit 6714a42

File tree

2 files changed

+39
-35
lines changed

2 files changed

+39
-35
lines changed

‎Request/ParamFetcher.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,14 +110,14 @@ public function get($name, $strict = null)
110110
$default = $config->default;
111111
$paramType = $config instanceof QueryParam ? 'Query' : 'Request';
112112

113-
if ($config->array) {
114-
$default = (array) $default;
115-
}
116-
117113
if (null === $strict) {
118114
$strict = $config->strict;
119115
}
120116

117+
if ($config->array && (null !== $default || !$strict)) {
118+
$default = (array) $default;
119+
}
120+
121121
if ($config instanceof RequestParam) {
122122
$param = $this->request->request->get($config->getKey(), $default);
123123
} elseif ($config instanceof QueryParam) {
@@ -130,7 +130,7 @@ public function get($name, $strict = null)
130130
if (!is_array($param)) {
131131
if ($strict) {
132132
throw new BadRequestHttpException(
133-
sprintf("% parameter value of '%s' is not an array", $paramType, $name)
133+
sprintf("%s parameter value of '%s' is not an array", $paramType, $name)
134134
);
135135
}
136136

‎Tests/Request/ParamFetcherTest.php‎

Lines changed: 34 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,10 @@ public function setup()
109109
$annotations['biz']->nullable = true;
110110
$annotations['biz']->description = 'A scalar param with an explicitly defined null default';
111111

112+
$annotations['arr'] = new RequestParam();
113+
$annotations['arr']->name = 'arr';
114+
$annotations['arr']->array = true;
115+
112116
$this->paramReader
113117
->expects($this->any())
114118
->method('read')
@@ -174,23 +178,23 @@ public static function validatesConfiguredParamDataProvider()
174178
array( // check that non-strict missing params take default value
175179
'foo',
176180
'1',
177-
array('foo' => '1', 'bar' => '2', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null),
181+
array('foo' => '1', 'bar' => '2', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()),
178182
array(),
179-
array('bar' => '2', 'baz' => '4'),
183+
array('bar' => '2', 'baz' => '4', 'arr' => array()),
180184
),
181185
array( // pass Param in GET
182186
'foo',
183187
'42',
184-
array('foo' => '42', 'bar' => '2', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null),
188+
array('foo' => '42', 'bar' => '2', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()),
185189
array('foo' => '42'),
186-
array('bar' => '2', 'baz' => '4'),
190+
array('bar' => '2', 'baz' => '4', 'arr' => array()),
187191
),
188192
array( // check that invalid non-strict params take default value
189193
'foo',
190194
'1',
191-
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null),
195+
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()),
192196
array('foo' => 'bar'),
193-
array('bar' => '1', 'baz' => '4'),
197+
array('bar' => '1', 'baz' => '4', 'arr' => array()),
194198
function (\PHPUnit_Framework_MockObject_MockObject $validator, \PHPUnit_Framework_TestCase $self) {
195199
$errors = new ConstraintViolationList(array(
196200
new ConstraintViolation("expected error", null, array(), null, null, null),
@@ -210,31 +214,31 @@ function (\PHPUnit_Framework_MockObject_MockObject $validator, \PHPUnit_Framewor
210214
array( // invalid array
211215
'buzz',
212216
array(1),
213-
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null),
217+
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()),
214218
array('buzz' => 'invaliddata'),
215-
array('bar' => '1', 'baz' => '4'),
219+
array('bar' => '1', 'baz' => '4', 'arr' => array()),
216220
),
217221
array( // invalid array (multiple depth)
218222
'buzz',
219223
array(1),
220-
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null),
224+
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()),
221225
array('buzz' => array(array(1))),
222-
array('bar' => '1', 'baz' => '4'),
226+
array('bar' => '1', 'baz' => '4', 'arr' => array()),
223227
),
224228

225229
array( // multiple array
226230
'buzz',
227231
array(2, 3, 4),
228-
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array(), 'boozz' => null, 'biz' => null),
232+
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()),
229233
array('buzz' => array(2, 3, 4)),
230-
array('bar' => '1', 'baz' => '4'),
234+
array('bar' => '1', 'baz' => '4', 'arr' => array()),
231235
),
232236
array( // multiple array with one invalid value
233237
'buzz',
234238
array(2, 1, 4),
235-
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 1, 4), 'boo' => array(), 'boozz' => null, 'biz' => null),
239+
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 1, 4), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()),
236240
array('buzz' => array(2, 'invaliddata', 4)),
237-
array('bar' => '1', 'baz' => '4'),
241+
array('bar' => '1', 'baz' => '4', 'arr' => array()),
238242
function (\PHPUnit_Framework_MockObject_MockObject $validator, \PHPUnit_Framework_TestCase $self) {
239243
$errors = new ConstraintViolationList(array(
240244
new ConstraintViolation("expected error", null, array(), null, null, null),
@@ -254,44 +258,44 @@ function (\PHPUnit_Framework_MockObject_MockObject $validator, \PHPUnit_Framewor
254258
array( // Array not provided in GET query
255259
'boo',
256260
array(),
257-
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array(), 'boozz' => null, 'biz' => null),
261+
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()),
258262
array('buzz' => array(2, 3, 4)),
259-
array('bar' => '1', 'baz' => '4'),
263+
array('bar' => '1', 'baz' => '4', 'arr' => array()),
260264
),
261265
array( // QueryParam provided in GET query but as a scalar
262266
'boo',
263267
array(),
264-
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array(), 'boozz' => null, 'biz' => null),
268+
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array(), 'boozz' => null, 'biz' => null, 'arr' => array()),
265269
array('buzz' => array(2, 3, 4), 'boo' => 'scalar'),
266-
array('bar' => '1', 'baz' => '4'),
270+
array('bar' => '1', 'baz' => '4', 'arr' => array()),
267271
),
268272
array( // QueryParam provided in GET query with valid values
269273
'boo',
270274
array('1', 'foo', 5),
271-
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => null, 'biz' => null),
275+
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => null, 'biz' => null, 'arr' => array()),
272276
array('buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5)),
273-
array('bar' => '1', 'baz' => '4'),
277+
array('bar' => '1', 'baz' => '4', 'arr' => array()),
274278
),
275279
array( // QueryParam provided in GET query with valid values
276280
'boozz',
277281
null,
278-
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => null, 'biz' => null),
282+
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => null, 'biz' => null, 'arr' => array()),
279283
array('buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5)),
280-
array('bar' => '1', 'baz' => '4'),
284+
array('bar' => '1', 'baz' => '4', 'arr' => array()),
281285
),
282286
array( // QueryParam provided in GET query with valid values
283287
'boozz',
284288
5,
285-
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => 5, 'biz' => null),
289+
array('foo' => '1', 'bar' => '1', 'baz' => '4', 'buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => 5, 'biz' => null, 'arr' => array()),
286290
array('buzz' => array(2, 3, 4), 'boo' => array('1', 'foo', 5), 'boozz' => 5),
287-
array('bar' => '1', 'baz' => '4', 'boozz' => 5),
291+
array('bar' => '1', 'baz' => '4', 'boozz' => 5, 'arr' => array()),
288292
),
289293
);
290294
}
291295

292296
public function testValidatesAddParam()
293297
{
294-
$queryFetcher = $this->getParamFetcher(array(), array('bar' => '2', 'baz' => '4','bub' => '10'));
298+
$queryFetcher = $this->getParamFetcher(array(), array('bar' => '2', 'baz' => '4','bub' => '10', 'arr' => array()));
295299
$queryFetcher->setController($this->controller);
296300

297301
$runtimeParam = new RequestParam();
@@ -301,7 +305,7 @@ public function testValidatesAddParam()
301305
$queryFetcher->addParam($runtimeParam);
302306

303307
$this->assertEquals(10, $queryFetcher->get('bub'));
304-
$this->assertEquals(array('foo' => '1', 'bar' => '2', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null,'bub' => 10), $queryFetcher->all());
308+
$this->assertEquals(array('foo' => '1', 'bar' => '2', 'baz' => '4', 'buzz' => array(1), 'boo' => array(), 'boozz' => null, 'biz' => null, 'bub' => 10, 'arr' => array()), $queryFetcher->all());
305309
}
306310

307311
public function testValidatesConfiguredParamStrictly()
@@ -370,10 +374,10 @@ public function testExceptionOnValidatesFailure($query, $request, $param, \Closu
370374
public static function exceptionOnValidatesFailureDataProvider()
371375
{
372376
return array(
373-
array( // test missing strict param
374-
array(),
375-
array(),
376-
'bar',
377+
array( // test missing 'arr' request param of array type
378+
array('boozz' => 'foo'),
379+
array('bar' => 'foo', 'baz' => 'foo'),
380+
'arr',
377381
),
378382
array( // test invalid strict param
379383
array(),

0 commit comments

Comments
 (0)