From 232466fc40e9bc67a142850588fe54836aa8d9ff Mon Sep 17 00:00:00 2001 From: Mohamed Said Date: Mon, 24 Oct 2016 16:36:48 +0300 Subject: [PATCH] Do not hydrate files in Validator (#16017) --- src/Illuminate/Validation/Validator.php | 80 ++--------- tests/Validation/ValidationValidatorTest.php | 138 ++++++------------- 2 files changed, 49 insertions(+), 169 deletions(-) diff --git a/src/Illuminate/Validation/Validator.php b/src/Illuminate/Validation/Validator.php index 9fd668e74873..111d8b0a2cb8 100755 --- a/src/Illuminate/Validation/Validator.php +++ b/src/Illuminate/Validation/Validator.php @@ -203,7 +203,7 @@ public function __construct(Translator $translator, array $data, array $rules, a $this->translator = $translator; $this->customMessages = $messages; $this->customAttributes = $customAttributes; - $this->data = $this->hydrateFiles($this->parseData($data)); + $this->data = $this->parseData($data); $this->setRules($rules); } @@ -233,38 +233,6 @@ public function parseData(array $data) return $newData; } - /** - * Hydrate the files array. - * - * @param array $data - * @param string $arrayKey - * @return array - */ - protected function hydrateFiles(array $data, $arrayKey = null) - { - if (is_null($arrayKey)) { - $this->files = []; - } - - foreach ($data as $key => $value) { - $newKey = $arrayKey ? "$arrayKey.$key" : $key; - - // If this value is an instance of the HttpFoundation File class we will - // remove it from the data array and add it to the files array, which - // we use to conveniently separate out these files from other data. - if ($value instanceof File) { - $this->files[$newKey] = $value; - - unset($data[$key]); - } elseif (is_array($value) && ! empty($value) && - empty($this->hydrateFiles($value, $newKey))) { - unset($data[$key]); - } - } - - return $data; - } - /** * Explode the rules into an array of rules. * @@ -341,9 +309,7 @@ public function sometimes($attribute, $rules, callable $callback) */ public function each($attribute, $rules) { - $data = array_merge( - Arr::dot($this->initializeAttributeOnData($attribute)), $this->files - ); + $data = Arr::dot($this->initializeAttributeOnData($attribute)); $pattern = str_replace('\*', '[^\.]+', preg_quote($attribute)); @@ -614,11 +580,7 @@ protected function attributesThatHaveMessages() */ protected function getValue($attribute) { - if (! is_null($value = Arr::get($this->data, $attribute))) { - return $value; - } elseif (! is_null($value = Arr::get($this->files, $attribute))) { - return $value; - } + return Arr::get($this->data, $attribute); } /** @@ -664,8 +626,7 @@ protected function passesOptionalCheck($attribute) { if ($this->hasRule($attribute, ['Sometimes'])) { return array_key_exists($attribute, Arr::dot($this->data)) - || in_array($attribute, array_keys($this->data)) - || array_key_exists($attribute, $this->files); + || in_array($attribute, array_keys($this->data)); } return true; @@ -837,7 +798,7 @@ protected function validateRequired($attribute, $value) */ protected function validatePresent($attribute, $value) { - return Arr::has(array_merge($this->data, $this->files), $attribute); + return Arr::has($this->data, $attribute); } /** @@ -849,7 +810,7 @@ protected function validatePresent($attribute, $value) */ protected function validateFilled($attribute, $value) { - if (Arr::has(array_merge($this->data, $this->files), $attribute)) { + if (Arr::has($this->data, $attribute)) { return $this->validateRequired($attribute, $value); } @@ -1025,7 +986,7 @@ protected function getPresentCount($attributes) $count = 0; foreach ($attributes as $key) { - if (Arr::get($this->data, $key) || Arr::get($this->files, $key)) { + if (Arr::get($this->data, $key)) { $count++; } } @@ -2187,7 +2148,7 @@ protected function getAttributeType($attribute) return 'numeric'; } elseif ($this->hasRule($attribute, ['Array'])) { return 'array'; - } elseif (array_key_exists($attribute, $this->files)) { + } elseif ($this->getValue($attribute) instanceof UploadedFile) { return 'file'; } @@ -2657,7 +2618,7 @@ protected function replaceAfter($message, $attribute, $rule, $parameters) */ public function attributes() { - return array_merge($this->data, $this->files); + return $this->data; } /** @@ -3093,29 +3054,6 @@ public function setValueNames(array $values) return $this; } - /** - * Get the files under validation. - * - * @return array - */ - public function getFiles() - { - return $this->files; - } - - /** - * Set the files under validation. - * - * @param array $files - * @return $this - */ - public function setFiles(array $files) - { - $this->files = $files; - - return $this; - } - /** * Get the Presence Verifier implementation. * diff --git a/tests/Validation/ValidationValidatorTest.php b/tests/Validation/ValidationValidatorTest.php index 0d64f2f500ee..86ca9dd0fda4 100755 --- a/tests/Validation/ValidationValidatorTest.php +++ b/tests/Validation/ValidationValidatorTest.php @@ -559,12 +559,6 @@ public function testValidateRequired() $file = new File(__FILE__, false); $v = new Validator($trans, ['name' => $file], ['name' => 'Required']); $this->assertTrue($v->passes()); - - $file = new File(__FILE__, false); - $foo = new File(__FILE__, false); - $v = new Validator($trans, ['name' => [$file, $foo]], ['name.0' => 'Required', 'name.1' => 'Required']); - $this->assertTrue($v->passes()); - $this->assertEmpty($v->getData()); } public function testValidateRequiredWith() @@ -807,16 +801,14 @@ public function testFailedFileUploads() $file = m::mock('Symfony\Component\HttpFoundation\File\UploadedFile'); $file->shouldReceive('isValid')->andReturn(false); $file->shouldNotReceive('getSize'); - $v = new Validator($trans, [], ['photo' => 'Max:10']); - $v->setFiles(['photo' => $file]); + $v = new Validator($trans, ['photo' => $file], ['photo' => 'Max:10']); $this->assertTrue($v->fails()); $this->assertEquals(['validation.uploaded'], $v->errors()->get('photo')); // Even "required" will not run if the file failed to upload. $file = m::mock('Symfony\Component\HttpFoundation\File\UploadedFile'); $file->shouldReceive('isValid')->once()->andReturn(false); - $v = new Validator($trans, [], ['photo' => 'required']); - $v->setFiles(['photo' => $file]); + $v = new Validator($trans, ['photo' => $file], ['photo' => 'required']); $this->assertTrue($v->fails()); $this->assertEquals(['validation.uploaded'], $v->errors()->get('photo')); @@ -824,16 +816,14 @@ public function testFailedFileUploads() // a file. Otherwise it should fail with the regular rule. $file = m::mock('Symfony\Component\HttpFoundation\File\UploadedFile'); $file->shouldReceive('isValid')->andReturn(false); - $v = new Validator($trans, [], ['photo' => 'string']); - $v->setFiles(['photo' => $file]); + $v = new Validator($trans, ['photo' => $file], ['photo' => 'string']); $this->assertTrue($v->fails()); $this->assertEquals(['validation.string'], $v->errors()->get('photo')); // Validation shouldn't continue if a file failed to upload. $file = m::mock('Symfony\Component\HttpFoundation\File\UploadedFile'); $file->shouldReceive('isValid')->once()->andReturn(false); - $v = new Validator($trans, [], ['photo' => 'file|mimes:pdf|min:10']); - $v->setFiles(['photo' => $file]); + $v = new Validator($trans, ['photo' => $file], ['photo' => 'file|mimes:pdf|min:10']); $this->assertTrue($v->fails()); $this->assertEquals(['validation.uploaded'], $v->errors()->get('photo')); } @@ -1149,14 +1139,12 @@ public function testValidateSize() $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\File')->setMethods(['getSize'])->setConstructorArgs([__FILE__, false])->getMock(); $file->expects($this->any())->method('getSize')->will($this->returnValue(3072)); - $v = new Validator($trans, [], ['photo' => 'Size:3']); - $v->setFiles(['photo' => $file]); + $v = new Validator($trans, ['photo' => $file], ['photo' => 'Size:3']); $this->assertTrue($v->passes()); $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\File')->setMethods(['getSize'])->setConstructorArgs([__FILE__, false])->getMock(); $file->expects($this->any())->method('getSize')->will($this->returnValue(4072)); - $v = new Validator($trans, [], ['photo' => 'Size:3']); - $v->setFiles(['photo' => $file]); + $v = new Validator($trans, ['photo' => $file], ['photo' => 'Size:3']); $this->assertFalse($v->passes()); } @@ -1189,14 +1177,12 @@ public function testValidateBetween() $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\File')->setMethods(['getSize'])->setConstructorArgs([__FILE__, false])->getMock(); $file->expects($this->any())->method('getSize')->will($this->returnValue(3072)); - $v = new Validator($trans, [], ['photo' => 'Between:1,5']); - $v->setFiles(['photo' => $file]); + $v = new Validator($trans, ['photo' => $file], ['photo' => 'Between:1,5']); $this->assertTrue($v->passes()); $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\File')->setMethods(['getSize'])->setConstructorArgs([__FILE__, false])->getMock(); $file->expects($this->any())->method('getSize')->will($this->returnValue(4072)); - $v = new Validator($trans, [], ['photo' => 'Between:1,2']); - $v->setFiles(['photo' => $file]); + $v = new Validator($trans, ['photo' => $file], ['photo' => 'Between:1,2']); $this->assertFalse($v->passes()); } @@ -1223,14 +1209,12 @@ public function testValidateMin() $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\File')->setMethods(['getSize'])->setConstructorArgs([__FILE__, false])->getMock(); $file->expects($this->any())->method('getSize')->will($this->returnValue(3072)); - $v = new Validator($trans, [], ['photo' => 'Min:2']); - $v->setFiles(['photo' => $file]); + $v = new Validator($trans, ['photo' => $file], ['photo' => 'Min:2']); $this->assertTrue($v->passes()); $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\File')->setMethods(['getSize'])->setConstructorArgs([__FILE__, false])->getMock(); $file->expects($this->any())->method('getSize')->will($this->returnValue(4072)); - $v = new Validator($trans, [], ['photo' => 'Min:10']); - $v->setFiles(['photo' => $file]); + $v = new Validator($trans, ['photo' => $file], ['photo' => 'Min:10']); $this->assertFalse($v->passes()); } @@ -1258,21 +1242,18 @@ public function testValidateMax() $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['isValid', 'getSize'])->setConstructorArgs([__FILE__, basename(__FILE__)])->getMock(); $file->expects($this->any())->method('isValid')->will($this->returnValue(true)); $file->expects($this->at(1))->method('getSize')->will($this->returnValue(3072)); - $v = new Validator($trans, [], ['photo' => 'Max:10']); - $v->setFiles(['photo' => $file]); + $v = new Validator($trans, ['photo' => $file], ['photo' => 'Max:10']); $this->assertTrue($v->passes()); $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['isValid', 'getSize'])->setConstructorArgs([__FILE__, basename(__FILE__)])->getMock(); $file->expects($this->at(0))->method('isValid')->will($this->returnValue(true)); $file->expects($this->at(1))->method('getSize')->will($this->returnValue(4072)); - $v = new Validator($trans, [], ['photo' => 'Max:2']); - $v->setFiles(['photo' => $file]); + $v = new Validator($trans, ['photo' => $file], ['photo' => 'Max:2']); $this->assertFalse($v->passes()); $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['isValid'])->setConstructorArgs([__FILE__, basename(__FILE__)])->getMock(); $file->expects($this->any())->method('isValid')->will($this->returnValue(false)); - $v = new Validator($trans, [], ['photo' => 'Max:10']); - $v->setFiles(['photo' => $file]); + $v = new Validator($trans, ['photo' => $file], ['photo' => 'Max:10']); $this->assertFalse($v->passes()); } @@ -1290,10 +1271,10 @@ public function testProperMessagesAreReturnedForSizes() $v->messages()->setFormat(':message'); $this->assertEquals('string', $v->messages()->first('name')); - $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\File')->setMethods(['getSize'])->setConstructorArgs([__FILE__, false])->getMock(); + $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['getSize', 'isValid'])->setConstructorArgs([__FILE__, false])->getMock(); $file->expects($this->any())->method('getSize')->will($this->returnValue(4072)); - $v = new Validator($trans, [], ['photo' => 'Max:3']); - $v->setFiles(['photo' => $file]); + $file->expects($this->any())->method('isValid')->will($this->returnValue(true)); + $v = new Validator($trans, ['photo' => $file], ['photo' => 'Max:3']); $this->assertFalse($v->passes()); $v->messages()->setFormat(':message'); $this->assertEquals('file', $v->messages()->first('photo')); @@ -1833,34 +1814,32 @@ public function testValidateImage() $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension'])->setConstructorArgs($uploadedFile)->getMock(); $file->expects($this->any())->method('guessExtension')->will($this->returnValue('php')); - $v = new Validator($trans, [], ['x' => 'Image']); - $v->setFiles(['x' => $file]); + $v = new Validator($trans, ['x' => $file], ['x' => 'Image']); $this->assertFalse($v->passes()); - $v = new Validator($trans, [], ['x' => 'Image']); $file2 = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension'])->setConstructorArgs($uploadedFile)->getMock(); $file2->expects($this->any())->method('guessExtension')->will($this->returnValue('jpeg')); - $v->setFiles(['x' => $file2]); + $v = new Validator($trans, ['x' => $file2], ['x' => 'Image']); $this->assertTrue($v->passes()); $file3 = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension'])->setConstructorArgs($uploadedFile)->getMock(); $file3->expects($this->any())->method('guessExtension')->will($this->returnValue('gif')); - $v->setFiles(['x' => $file3]); + $v = new Validator($trans, ['x' => $file3], ['x' => 'Image']); $this->assertTrue($v->passes()); $file4 = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension'])->setConstructorArgs($uploadedFile)->getMock(); $file4->expects($this->any())->method('guessExtension')->will($this->returnValue('bmp')); - $v->setFiles(['x' => $file4]); + $v = new Validator($trans, ['x' => $file4], ['x' => 'Image']); $this->assertTrue($v->passes()); $file5 = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension'])->setConstructorArgs($uploadedFile)->getMock(); $file5->expects($this->any())->method('guessExtension')->will($this->returnValue('png')); - $v->setFiles(['x' => $file5]); + $v = new Validator($trans, ['x' => $file5], ['x' => 'Image']); $this->assertTrue($v->passes()); $file6 = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension'])->setConstructorArgs($uploadedFile)->getMock(); $file6->expects($this->any())->method('guessExtension')->will($this->returnValue('svg')); - $v->setFiles(['x' => $file6]); + $v = new Validator($trans, ['x' => $file6], ['x' => 'Image']); $this->assertTrue($v->passes()); } @@ -1873,60 +1852,46 @@ public function testValidateImageDimensions() $v = new Validator($trans, ['x' => 'file'], ['x' => 'dimensions']); $this->assertTrue($v->fails()); - $v = new Validator($trans, [], ['x' => 'dimensions:min_width=1']); - $v->setFiles(['x' => $uploadedFile]); + $v = new Validator($trans, ['x' => $uploadedFile], ['x' => 'dimensions:min_width=1']); $this->assertTrue($v->passes()); - $v = new Validator($trans, [], ['x' => 'dimensions:min_width=5']); - $v->setFiles(['x' => $uploadedFile]); + $v = new Validator($trans, ['x' => $uploadedFile], ['x' => 'dimensions:min_width=5']); $this->assertTrue($v->fails()); - $v = new Validator($trans, [], ['x' => 'dimensions:max_width=10']); - $v->setFiles(['x' => $uploadedFile]); + $v = new Validator($trans, ['x' => $uploadedFile], ['x' => 'dimensions:max_width=10']); $this->assertTrue($v->passes()); - $v = new Validator($trans, [], ['x' => 'dimensions:max_width=1']); - $v->setFiles(['x' => $uploadedFile]); + $v = new Validator($trans, ['x' => $uploadedFile], ['x' => 'dimensions:max_width=1']); $this->assertTrue($v->fails()); - $v = new Validator($trans, [], ['x' => 'dimensions:min_height=1']); - $v->setFiles(['x' => $uploadedFile]); + $v = new Validator($trans, ['x' => $uploadedFile], ['x' => 'dimensions:min_height=1']); $this->assertTrue($v->passes()); - $v = new Validator($trans, [], ['x' => 'dimensions:min_height=5']); - $v->setFiles(['x' => $uploadedFile]); + $v = new Validator($trans, ['x' => $uploadedFile], ['x' => 'dimensions:min_height=5']); $this->assertTrue($v->fails()); - $v = new Validator($trans, [], ['x' => 'dimensions:max_height=10']); - $v->setFiles(['x' => $uploadedFile]); + $v = new Validator($trans, ['x' => $uploadedFile], ['x' => 'dimensions:max_height=10']); $this->assertTrue($v->passes()); - $v = new Validator($trans, [], ['x' => 'dimensions:max_height=1']); - $v->setFiles(['x' => $uploadedFile]); + $v = new Validator($trans, ['x' => $uploadedFile], ['x' => 'dimensions:max_height=1']); $this->assertTrue($v->fails()); - $v = new Validator($trans, [], ['x' => 'dimensions:width=3']); - $v->setFiles(['x' => $uploadedFile]); + $v = new Validator($trans, ['x' => $uploadedFile], ['x' => 'dimensions:width=3']); $this->assertTrue($v->passes()); - $v = new Validator($trans, [], ['x' => 'dimensions:height=2']); - $v->setFiles(['x' => $uploadedFile]); + $v = new Validator($trans, ['x' => $uploadedFile], ['x' => 'dimensions:height=2']); $this->assertTrue($v->passes()); - $v = new Validator($trans, [], ['x' => 'dimensions:min_height=2,ratio=3/2']); - $v->setFiles(['x' => $uploadedFile]); + $v = new Validator($trans, ['x' => $uploadedFile], ['x' => 'dimensions:min_height=2,ratio=3/2']); $this->assertTrue($v->passes()); - $v = new Validator($trans, [], ['x' => 'dimensions:ratio=1.5']); - $v->setFiles(['x' => $uploadedFile]); + $v = new Validator($trans, ['x' => $uploadedFile], ['x' => 'dimensions:ratio=1.5']); $this->assertTrue($v->passes()); - $v = new Validator($trans, [], ['x' => 'dimensions:ratio=1/1']); - $v->setFiles(['x' => $uploadedFile]); + $v = new Validator($trans, ['x' => $uploadedFile], ['x' => 'dimensions:ratio=1/1']); $this->assertTrue($v->fails()); - $v = new Validator($trans, [], ['x' => 'dimensions:ratio=1']); - $v->setFiles(['x' => $uploadedFile]); + $v = new Validator($trans, ['x' => $uploadedFile], ['x' => 'dimensions:ratio=1']); $this->assertTrue($v->fails()); } @@ -1940,8 +1905,7 @@ public function testValidateMimetypes() $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension'])->setConstructorArgs($uploadedFile)->getMock(); $file->expects($this->any())->method('guessExtension')->will($this->returnValue('php')); - $v = new Validator($trans, [], ['x' => 'mimetypes:text/x-php']); - $v->setFiles(['x' => $file]); + $v = new Validator($trans, ['x' => $file], ['x' => 'mimetypes:text/x-php']); $this->assertTrue($v->passes()); } @@ -1952,15 +1916,13 @@ public function testValidateMime() $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension'])->setConstructorArgs($uploadedFile)->getMock(); $file->expects($this->any())->method('guessExtension')->will($this->returnValue('php')); - $v = new Validator($trans, [], ['x' => 'mimes:php']); - $v->setFiles(['x' => $file]); + $v = new Validator($trans, ['x' => $file], ['x' => 'mimes:php']); $this->assertTrue($v->passes()); $file2 = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'isValid'])->setConstructorArgs($uploadedFile)->getMock(); $file2->expects($this->any())->method('guessExtension')->will($this->returnValue('php')); $file2->expects($this->any())->method('isValid')->will($this->returnValue(false)); - $v = new Validator($trans, [], ['x' => 'mimes:php']); - $v->setFiles(['x' => $file2]); + $v = new Validator($trans, ['x' => $file2], ['x' => 'mimes:php']); $this->assertFalse($v->passes()); } @@ -1975,8 +1937,7 @@ public function testValidateFile() $v = new Validator($trans, ['x' => '1'], ['x' => 'file']); $this->assertTrue($v->fails()); - $v = new Validator($trans, [], ['x' => 'file']); - $v->setFiles(['x' => $file]); + $v = new Validator($trans, ['x' => $file], ['x' => 'file']); $this->assertTrue($v->passes()); } @@ -3221,25 +3182,6 @@ public function testValidMethod() ]); } - public function testFilesHydration() - { - $trans = $this->getIlluminateArrayTranslator(); - $file = new File(__FILE__, false); - $v = new Validator($trans, ['file' => $file, 'text' => 'text'], ['text' => 'Required']); - $this->assertEquals(['file' => $file], $v->getFiles()); - $this->assertEquals(['text' => 'text'], $v->getData()); - } - - public function testArrayOfFilesHydration() - { - $trans = $this->getIlluminateArrayTranslator(); - $file = new File(__FILE__, false); - $file2 = new File(__FILE__, false); - $v = new Validator($trans, ['file' => [$file, $file2], 'text' => 'text'], ['text' => 'Required']); - $this->assertEquals(['file.0' => $file, 'file.1' => $file2], $v->getFiles()); - $this->assertEquals(['text' => 'text'], $v->getData()); - } - public function testMultipleFileUploads() { $trans = $this->getIlluminateArrayTranslator();