From 71e5c19f289dd9417cf711e1c523680dafe223a0 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Mon, 4 Dec 2017 14:24:21 -0600 Subject: [PATCH 1/4] Ensure filter and validator chains receive default plugin managers When using the factory, the plugin managers used with the default filter and validator chains should be used with all newly created inputs, as these will have any custom services registered (vs. those that are in the initially created inputs, which are unconfigured). This means that: - For cases where the instance was pulled from the `InputFilterManager`, we need to inject the filter and validator chains with the plugin managers pulled from the default instances; if the chain does not exist, we clone the default instance. - For other instances, we need to simply clone the default filter and validator chains (which are empty, but have the configured filter and validator plugin managers). --- src/Factory.php | 52 ++++++++++++-- test/FactoryTest.php | 20 ++++++ .../InputFilterAbstractServiceFactoryTest.php | 72 +++++++++++++++++++ 3 files changed, 138 insertions(+), 6 deletions(-) diff --git a/src/Factory.php b/src/Factory.php index 0a0d38ac..01f299e4 100644 --- a/src/Factory.php +++ b/src/Factory.php @@ -189,12 +189,9 @@ public function createInput($inputSpecification) )); } - if (! $managerInstance && $this->defaultFilterChain) { - $input->setFilterChain(clone $this->defaultFilterChain); - } - if (! $managerInstance && $this->defaultValidatorChain) { - $input->setValidatorChain(clone $this->defaultValidatorChain); - } + $managerInstance + ? $this->injectFilterAndValidatorChainsWithPluginManagers($input) + : $this->injectDefaultFilterAndValidatorChains($input); foreach ($inputSpecification as $key => $value) { switch ($key) { @@ -434,4 +431,47 @@ protected function populateValidators(ValidatorChain $chain, $validators) ); } } + + /** + * Inject the default filter and validator chains into the input, if present. + * + * This ensures custom plugins are made available to the input instance. + * + * @param InputInterface $input + * @return void + */ + protected function injectDefaultFilterAndValidatorChains(InputInterface $input) + { + if ($this->defaultFilterChain) { + $input->setFilterChain(clone $this->defaultFilterChain); + } + + if ($this->defaultValidatorChain) { + $input->setValidatorChain(clone $this->defaultValidatorChain); + } + } + + /** + * Inject filter and validator chains with the plugin managers from + * the default chains, if present. + * + * This ensures custom plugins are made available to the input instance. + * + * @param InputInterface $input + * @return void + */ + protected function injectFilterAndValidatorChainsWithPluginManagers(InputInterface $input) + { + if ($this->defaultFilterChain) { + $input->getFilterChain() + ? $input->getFilterChain()->setPluginManager($this->defaultFilterChain->getPluginManager()) + : $input->setFilterChain(clone $this->defaultFilterChain); + } + + if ($this->defaultValidatorChain) { + $input->getValidatorChain() + ? $input->getValidatorChain()->setPluginManager($this->defaultValidatorChain->getPluginManager()) + : $input->setValidatorChain(clone $this->defaultValidatorChain); + } + } } diff --git a/test/FactoryTest.php b/test/FactoryTest.php index 17de82ae..228cd0f8 100644 --- a/test/FactoryTest.php +++ b/test/FactoryTest.php @@ -13,6 +13,7 @@ use PHPUnit_Framework_MockObject_MockObject as MockObject; use PHPUnit\Framework\TestCase; use Prophecy\Argument; +use ReflectionProperty; use Zend\Filter; use Zend\InputFilter\CollectionInputFilter; use Zend\InputFilter\Exception\InvalidArgumentException; @@ -999,9 +1000,11 @@ public function testClearDefaultValidatorChain() /** * @see https://github.com/zendframework/zend-inputfilter/issues/8 + * @see https://github.com/zendframework/zend-inputfilter/issues/155 */ public function testWhenCreateInputPullsInputFromThePluginManagerItMustNotOverwriteFilterAndValidatorChains() { + $input = $this->prophesize(InputInterface::class); $input->setFilterChain(Argument::any())->shouldNotBeCalled(); $input->setValidatorChain(Argument::any())->shouldNotBeCalled(); @@ -1015,6 +1018,23 @@ public function testWhenCreateInputPullsInputFromThePluginManagerItMustNotOverwr $factory = new Factory($pluginManager->reveal()); + $r = new ReflectionProperty($factory, 'defaultFilterChain'); + $r->setAccessible(true); + $defaultFilterChain = $r->getValue($factory); + + $filterChain = $this->prophesize(Filter\FilterChain::class); + $filterChain->setPluginManager($defaultFilterChain->getPluginManager())->shouldBeCalled(); + + $r = new ReflectionProperty($factory, 'defaultValidatorChain'); + $r->setAccessible(true); + $defaultValidatorChain = $r->getValue($factory); + + $validatorChain = $this->prophesize(Validator\ValidatorChain::class); + $validatorChain->setPluginManager($defaultValidatorChain->getPluginManager())->shouldBeCalled(); + + $input->getFilterChain()->will([$filterChain, 'reveal'])->shouldBeCalled(); + $input->getValidatorChain()->will([$validatorChain, 'reveal'])->shouldBeCalled(); + $this->assertSame($input->reveal(), $factory->createInput($spec)); } diff --git a/test/InputFilterAbstractServiceFactoryTest.php b/test/InputFilterAbstractServiceFactoryTest.php index 8cdaa0de..1fe3a3a7 100644 --- a/test/InputFilterAbstractServiceFactoryTest.php +++ b/test/InputFilterAbstractServiceFactoryTest.php @@ -12,11 +12,14 @@ use PHPUnit_Framework_MockObject_MockObject as MockObject; use PHPUnit\Framework\TestCase; +use Zend\Filter; use Zend\Filter\FilterPluginManager; +use Zend\InputFilter\FileInput; use Zend\InputFilter\InputFilterAbstractServiceFactory; use Zend\InputFilter\InputFilterInterface; use Zend\InputFilter\InputFilterPluginManager; use Zend\ServiceManager\ServiceManager; +use Zend\Validator; use Zend\Validator\ValidatorInterface; use Zend\Validator\ValidatorPluginManager; @@ -301,4 +304,73 @@ public function testAllowsPassingNonPluginManagerContainerToFactoryWithServiceMa $inputFilter = call_user_func_array([$this->factory, $create], $args); $this->assertInstanceOf(InputFilterInterface::class, $inputFilter); } + + /** + * @see https://github.com/zendframework/zend-inputfilter/issues/155 + */ + public function testWillUseCustomFiltersWhenProvided() + { + $filter = $this->prophesize(Filter\FilterInterface::class)->reveal(); + + $filters = new FilterPluginManager($this->services); + $filters->setService('CustomFilter', $filter); + + $validators = new ValidatorPluginManager($this->services); + + $this->services->setService('FilterManager', $filters); + $this->services->setService('ValidatorManager', $validators); + + $this->services->setService('config', [ + 'input_filter_specs' => [ + 'test' => [ + [ + 'name' => 'a-file-element', + 'type' => FileInput::class, + 'required' => true, + 'validators' => [ + [ + 'name' => Validator\File\UploadFile::class, + 'options' => [ + 'breakchainonfailure' => true, + ], + ], + [ + 'name' => Validator\File\Size::class, + 'options' => [ + 'breakchainonfailure' => true, + 'max' => '6GB', + ], + ], + [ + 'name' => Validator\File\Extension::class, + 'options' => [ + 'breakchainonfailure' => true, + 'extension' => 'csv,zip', + ], + ], + ], + 'filters' => [ + ['name' => 'CustomFilter'], + ], + ], + ], + ], + ]); + + $this->services->get('InputFilterManager') + ->addAbstractFactory(InputFilterAbstractServiceFactory::class); + + $inputFilter = $this->services->get('InputFilterManager')->get('test'); + $this->assertInstanceOf(InputFilterInterface::class, $inputFilter); + + $input = $inputFilter->get('a-file-element'); + $this->assertInstanceOf(FileInput::class, $input); + + $filters = $input->getFilterChain(); + $this->assertCount(1, $filters); + + $callback = $filters->getFilters()->top(); + $this->assertInternalType('array', $callback); + $this->assertSame($filter, $callback[0]); + } } From fa9770826411bbcc5dc348b678a69855821c03bd Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Mon, 4 Dec 2017 14:30:37 -0600 Subject: [PATCH 2/4] Fix signature of CollectionInputFilter::isValid Must conform to parent signature, even if `$context` optional argument is ignored. --- src/CollectionInputFilter.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/CollectionInputFilter.php b/src/CollectionInputFilter.php index 38d425cf..33b28c84 100644 --- a/src/CollectionInputFilter.php +++ b/src/CollectionInputFilter.php @@ -166,8 +166,9 @@ public function setData($data) /** * {@inheritdoc} + * @param mixed $context Ignored, but present to retain signature compatibility. */ - public function isValid() + public function isValid($context = null) { $this->collectionMessages = []; $inputFilter = $this->getInputFilter(); From 816b0fdda952c133b1d01e01f5f57091b6c58ce7 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Mon, 4 Dec 2017 14:38:02 -0600 Subject: [PATCH 3/4] Update tests to use PHP 7.2-safe validators The `EmailAddress` validator is using a deprecated constant, which causes errors in this test suite. As such, updated several tests to use a different validator to ensure we can test on 7.2. --- test/CollectionInputFilterTest.php | 44 ++++++++++++++---------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/test/CollectionInputFilterTest.php b/test/CollectionInputFilterTest.php index 95188d7d..72eace01 100644 --- a/test/CollectionInputFilterTest.php +++ b/test/CollectionInputFilterTest.php @@ -19,7 +19,7 @@ use Zend\InputFilter\Exception\RuntimeException; use Zend\InputFilter\Input; use Zend\InputFilter\InputFilter; -use Zend\Validator\EmailAddress; +use Zend\Validator\Digits; use Zend\Validator\NotEmpty; /** @@ -512,10 +512,10 @@ public function testCollectionValidationDoesNotReuseMessagesBetweenInputs() { $inputFilter = new InputFilter(); $inputFilter->add([ - 'name' => 'email', + 'name' => 'phone', 'required' => true, 'validators' => [ - ['name' => EmailAddress::class], + ['name' => Digits::class], ['name' => NotEmpty::class], ], ]); @@ -535,7 +535,7 @@ public function testCollectionValidationDoesNotReuseMessagesBetweenInputs() 'name' => 'Tom', ], [ - 'email' => 'tom@tom', + 'phone' => 'tom@tom', 'name' => 'Tom', ], ]); @@ -547,16 +547,14 @@ public function testCollectionValidationDoesNotReuseMessagesBetweenInputs() $this->assertFalse($isValid); $this->assertCount(2, $messages); - $this->assertArrayHasKey('email', $messages[0]); - $this->assertCount(1, $messages[0]['email']); - $this->assertContains('Value is required and can\'t be empty', $messages[0]['email']); + $this->assertArrayHasKey('phone', $messages[0]); + $this->assertCount(1, $messages[0]['phone']); + $this->assertContains('Value is required and can\'t be empty', $messages[0]['phone']); - $this->assertArrayHasKey('email', $messages[1]); - $this->assertCount(3, $messages[1]['email']); - $this->assertNotContains('Value is required and can\'t be empty', $messages[1]['email']); - $this->assertContains('\'tom\' is not a valid hostname for the email address', $messages[1]['email']); - $this->assertContains('The input does not match the expected structure for a DNS hostname', $messages[1]['email']); - $this->assertContains('The input appears to be a local network name but local network names are not allowed', $messages[1]['email']); + $this->assertArrayHasKey('phone', $messages[1]); + $this->assertCount(1, $messages[1]['phone']); + $this->assertNotContains('Value is required and can\'t be empty', $messages[1]['phone']); + $this->assertContains('The input must contain only digits', $messages[1]['phone']); // @codingStandardsIgnoreEnd } @@ -564,10 +562,10 @@ public function testCollectionValidationUsesCustomInputErrorMessages() { $inputFilter = new InputFilter(); $inputFilter->add([ - 'name' => 'email', + 'name' => 'phone', 'required' => true, 'validators' => [ - ['name' => EmailAddress::class], + ['name' => Digits::class], ['name' => NotEmpty::class], ], 'error_message' => 'CUSTOM ERROR MESSAGE', @@ -588,7 +586,7 @@ public function testCollectionValidationUsesCustomInputErrorMessages() 'name' => 'Tom', ], [ - 'email' => 'tom@tom', + 'phone' => 'tom@tom', 'name' => 'Tom', ], ]); @@ -600,14 +598,14 @@ public function testCollectionValidationUsesCustomInputErrorMessages() $this->assertFalse($isValid); $this->assertCount(2, $messages); - $this->assertArrayHasKey('email', $messages[0]); - $this->assertCount(1, $messages[0]['email']); - $this->assertContains('CUSTOM ERROR MESSAGE', $messages[0]['email']); - $this->assertNotContains('Value is required and can\'t be empty', $messages[0]['email']); + $this->assertArrayHasKey('phone', $messages[0]); + $this->assertCount(1, $messages[0]['phone']); + $this->assertContains('CUSTOM ERROR MESSAGE', $messages[0]['phone']); + $this->assertNotContains('Value is required and can\'t be empty', $messages[0]['phone']); - $this->assertArrayHasKey('email', $messages[1]); - $this->assertCount(1, $messages[1]['email']); - $this->assertContains('CUSTOM ERROR MESSAGE', $messages[1]['email']); + $this->assertArrayHasKey('phone', $messages[1]); + $this->assertCount(1, $messages[1]['phone']); + $this->assertContains('CUSTOM ERROR MESSAGE', $messages[1]['phone']); } public function testDuplicatedErrorMessages() From a6ba46a2e8e1669c3918ebdd7120360084336831 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Mon, 4 Dec 2017 14:47:21 -0600 Subject: [PATCH 4/4] Adds CHANGELOG entry for #156 --- CHANGELOG.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 96dc2cde..f8f89f30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,33 @@ All notable changes to this project will be documented in this file, in reverse chronological order by release. +## 2.7.6 - 2017-12-04 + +### Added + +- Nothing. + +### Changed + +- Nothing. + +### Deprecated + +- Nothing. + +### Removed + +- Nothing. + +### Fixed + +- [#156](https://github.com/zendframework/zend-inputfilter/pull/156) fixes an + issue introduced in 2.7.5 whereby the filter and validator chains composed in + inputs pulled from the `InputFilterPluginManager` were not receiving the + default filter and validator plugin manager instances. A solution was created + that preserves the original behavior as well as the bugfix that created the + regression. + ## 2.7.5 - 2017-11-07 ### Added