From 8c8c3ae405e14cbca25d5291f4022266d8c6cdc0 Mon Sep 17 00:00:00 2001 From: Jules Pietri Date: Tue, 8 Mar 2016 11:21:48 +0100 Subject: [PATCH] [Form] fix fatal error handling callable strings with PropertyAccessDecorator ref #17993. Handle edge cases where a callable string is passed to the `PropertyAccessDecorator` for choice list factories. Wrapping the callable string in a closure prevents a potential fatal error, since it may be called with the wrong arguments. --- .../Factory/PropertyAccessDecorator.php | 25 +++++ .../Factory/PropertyAccessDecoratorTest.php | 106 ++++++++++++++++++ 2 files changed, 131 insertions(+) diff --git a/src/Symfony/Component/Form/ChoiceList/Factory/PropertyAccessDecorator.php b/src/Symfony/Component/Form/ChoiceList/Factory/PropertyAccessDecorator.php index 1b68fd892428..3fbc62b84891 100644 --- a/src/Symfony/Component/Form/ChoiceList/Factory/PropertyAccessDecorator.php +++ b/src/Symfony/Component/Form/ChoiceList/Factory/PropertyAccessDecorator.php @@ -172,6 +172,11 @@ public function createView(ChoiceListInterface $list, $preferredChoices = null, if (is_string($label) && !is_callable($label)) { $label = new PropertyPath($label); + } elseif (is_string($label) && is_callable($label)) { + // Prevent a fatal error since a callable string may not handle the right arguments + $label = function ($choice) use ($label) { + return $label($choice); + }; } if ($label instanceof PropertyPath) { @@ -182,6 +187,11 @@ public function createView(ChoiceListInterface $list, $preferredChoices = null, if (is_string($preferredChoices) && !is_callable($preferredChoices)) { $preferredChoices = new PropertyPath($preferredChoices); + } elseif (is_string($preferredChoices) && is_callable($preferredChoices)) { + // Prevent a fatal error since a callable string may not handle the right arguments + $preferredChoices = function ($choice) use ($preferredChoices) { + return $preferredChoices($choice); + }; } if ($preferredChoices instanceof PropertyPath) { @@ -197,6 +207,11 @@ public function createView(ChoiceListInterface $list, $preferredChoices = null, if (is_string($index) && !is_callable($index)) { $index = new PropertyPath($index); + } elseif (is_string($index) && is_callable($index)) { + // Prevent a fatal error since a callable string may not handle the right arguments + $index = function ($choice) use ($index) { + return $index($choice); + }; } if ($index instanceof PropertyPath) { @@ -207,6 +222,11 @@ public function createView(ChoiceListInterface $list, $preferredChoices = null, if (is_string($groupBy) && !is_callable($groupBy)) { $groupBy = new PropertyPath($groupBy); + } elseif (is_string($groupBy) && is_callable($groupBy)) { + // Prevent a fatal error since a callable string may not handle the right arguments + $groupBy = function ($choice) use ($groupBy) { + return $groupBy($choice); + }; } if ($groupBy instanceof PropertyPath) { @@ -221,6 +241,11 @@ public function createView(ChoiceListInterface $list, $preferredChoices = null, if (is_string($attr) && !is_callable($attr)) { $attr = new PropertyPath($attr); + } elseif (is_string($attr) && is_callable($attr)) { + // Prevent a fatal error since a callable string may not handle the right arguments + $attr = function ($choice) use ($attr) { + return $attr($choice); + }; } if ($attr instanceof PropertyPath) { diff --git a/src/Symfony/Component/Form/Tests/ChoiceList/Factory/PropertyAccessDecoratorTest.php b/src/Symfony/Component/Form/Tests/ChoiceList/Factory/PropertyAccessDecoratorTest.php index 44490a686a83..eba5b4643981 100644 --- a/src/Symfony/Component/Form/Tests/ChoiceList/Factory/PropertyAccessDecoratorTest.php +++ b/src/Symfony/Component/Form/Tests/ChoiceList/Factory/PropertyAccessDecoratorTest.php @@ -11,6 +11,7 @@ namespace Symfony\Component\Form\Tests\ChoiceList\Factory; +use Symfony\Component\Form\ChoiceList\ArrayChoiceList; use Symfony\Component\Form\ChoiceList\Factory\PropertyAccessDecorator; use Symfony\Component\PropertyAccess\PropertyPath; @@ -192,6 +193,25 @@ public function testCreateViewAssumeNullIfPreferredChoicesPropertyPathUnreadable )); } + public function testCreateViewPreferredChoicesAsCallableString() + { + $list = new ArrayChoiceList(array( + array('end' => 'RESULT'), + )); + + $this->decoratedFactory->expects($this->once()) + ->method('createView') + ->with($list, $this->isInstanceOf('\Closure')) + ->will($this->returnCallback(function ($list, $preferred) { + return $preferred(array('end' => 'RESULT')); + })); + + $this->assertSame('RESULT', $this->factory->createView( + $list, + 'end' + )); + } + public function testCreateViewLabelsAsPropertyPath() { $list = $this->getMock('Symfony\Component\Form\ChoiceList\ChoiceListInterface'); @@ -228,6 +248,26 @@ public function testCreateViewLabelsAsPropertyPathInstance() )); } + public function testCreateViewLabelAsCallableString() + { + $list = new ArrayChoiceList(array( + array('end' => 'RESULT'), + )); + + $this->decoratedFactory->expects($this->once()) + ->method('createView') + ->with($list, null, $this->isInstanceOf('\Closure')) + ->will($this->returnCallback(function ($list, $preferred, $label) { + return $label(array('end' => 'RESULT')); + })); + + $this->assertSame('RESULT', $this->factory->createView( + $list, + null, // preferred choices + 'end' + )); + } + public function testCreateViewIndicesAsPropertyPath() { $list = $this->getMock('Symfony\Component\Form\ChoiceList\ChoiceListInterface'); @@ -266,6 +306,27 @@ public function testCreateViewIndicesAsPropertyPathInstance() )); } + public function testCreateViewIndicesAsCallableString() + { + $list = new ArrayChoiceList(array( + array('end' => 'RESULT'), + )); + + $this->decoratedFactory->expects($this->once()) + ->method('createView') + ->with($list, null, null, $this->isInstanceOf('\Closure')) + ->will($this->returnCallback(function ($list, $preferred, $label, $index) { + return $index(array('end' => 'RESULT')); + })); + + $this->assertSame('RESULT', $this->factory->createView( + $list, + null, // preferred choices + null, // label + 'end' + )); + } + public function testCreateViewGroupsAsPropertyPath() { $list = $this->getMock('Symfony\Component\Form\ChoiceList\ChoiceListInterface'); @@ -327,6 +388,28 @@ public function testCreateViewAssumeNullIfGroupsPropertyPathUnreadable() )); } + public function testCreateViewGroupsAsCallableString() + { + $list = new ArrayChoiceList(array( + array('end' => 'RESULT'), + )); + + $this->decoratedFactory->expects($this->once()) + ->method('createView') + ->with($list, null, null, null, $this->isInstanceOf('\Closure')) + ->will($this->returnCallback(function ($list, $preferred, $label, $index, $groupBy) { + return $groupBy(array('end' => 'RESULT')); + })); + + $this->assertSame('RESULT', $this->factory->createView( + $list, + null, // preferred choices + null, // label + null, // index + 'end' + )); + } + public function testCreateViewAttrAsPropertyPath() { $list = $this->getMock('Symfony\Component\Form\ChoiceList\ChoiceListInterface'); @@ -368,4 +451,27 @@ public function testCreateViewAttrAsPropertyPathInstance() new PropertyPath('property') )); } + + public function testCreateViewAttrAsCallableString() + { + $list = new ArrayChoiceList(array( + array('end' => 'RESULT'), + )); + + $this->decoratedFactory->expects($this->once()) + ->method('createView') + ->with($list, null, null, null, null, $this->isInstanceOf('\Closure')) + ->will($this->returnCallback(function ($list, $preferred, $label, $index, $groupBy, $attr) { + return $attr(array('end' => 'RESULT')); + })); + + $this->assertSame('RESULT', $this->factory->createView( + $list, + null, // preferred choices + null, // label + null, // index + null, // groups + 'end' + )); + } }