merged branch bschussek/property-access-public-check (PR #7711)
This PR was merged into the master branch.
Discussion
----------
[PropertyAccess] Changed PropertyAccessor to continue searching when a non-public method/property are found
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | yes
| BC breaks? | yes
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | not necessary
Commits
-------
2a666cb
[PropertyAccess] Changed PropertyAccessor to continue searching when a non-public method/property are found
This commit is contained in:
commit
d450477ef3
|
@ -35,3 +35,55 @@ UPGRADE FROM 2.2 to 2.3
|
||||||
"validation_groups" => false
|
"validation_groups" => false
|
||||||
"validation_groups" => array()
|
"validation_groups" => array()
|
||||||
```
|
```
|
||||||
|
|
||||||
|
### PropertyAccess
|
||||||
|
|
||||||
|
* PropertyAccessor was changed to continue its search for a property or method
|
||||||
|
even if a non-public match was found. This means that the property "author"
|
||||||
|
in the following class will now correctly be found:
|
||||||
|
|
||||||
|
```
|
||||||
|
class Article
|
||||||
|
{
|
||||||
|
public $author;
|
||||||
|
|
||||||
|
private function getAuthor()
|
||||||
|
{
|
||||||
|
// ...
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Although this is uncommon, similar cases exist in practice.
|
||||||
|
|
||||||
|
Instead of the PropertyAccessDeniedException that was thrown here, the more
|
||||||
|
generic NoSuchPropertyException is thrown now if no public property nor
|
||||||
|
method are found by the PropertyAccessor. PropertyAccessDeniedException was
|
||||||
|
removed completely.
|
||||||
|
|
||||||
|
Before:
|
||||||
|
|
||||||
|
```
|
||||||
|
use Symfony\Component\PropertyAccess\Exception\PropertyAccessDeniedException;
|
||||||
|
use Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException;
|
||||||
|
|
||||||
|
try {
|
||||||
|
$value = $accessor->getValue($article, 'author');
|
||||||
|
} catch (PropertyAccessDeniedException $e) {
|
||||||
|
// Method/property was found but not public
|
||||||
|
} catch (NoSuchPropertyException $e) {
|
||||||
|
// Method/property was not found
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
After:
|
||||||
|
|
||||||
|
```
|
||||||
|
use Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException;
|
||||||
|
|
||||||
|
try {
|
||||||
|
$value = $accessor->getValue($article, 'author');
|
||||||
|
} catch (NoSuchPropertyException $e) {
|
||||||
|
// Method/property was not found or not public
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
|
@ -0,0 +1,10 @@
|
||||||
|
CHANGELOG
|
||||||
|
=========
|
||||||
|
|
||||||
|
2.3.0
|
||||||
|
------
|
||||||
|
|
||||||
|
* [BC BREAK] changed PropertyAccessor to continue its search for a property or
|
||||||
|
method even if a non-public match was found. Before, a PropertyAccessDeniedException
|
||||||
|
was thrown in this case. Class PropertyAccessDeniedException was removed
|
||||||
|
now.
|
|
@ -1,21 +0,0 @@
|
||||||
<?php
|
|
||||||
|
|
||||||
/*
|
|
||||||
* This file is part of the Symfony package.
|
|
||||||
*
|
|
||||||
* (c) Fabien Potencier <fabien@symfony.com>
|
|
||||||
*
|
|
||||||
* For the full copyright and license information, please view the LICENSE
|
|
||||||
* file that was distributed with this source code.
|
|
||||||
*/
|
|
||||||
|
|
||||||
namespace Symfony\Component\PropertyAccess\Exception;
|
|
||||||
|
|
||||||
/**
|
|
||||||
* Thrown when a property cannot be accessed because it is not public.
|
|
||||||
*
|
|
||||||
* @author Bernhard Schussek <bschussek@gmail.com>
|
|
||||||
*/
|
|
||||||
class PropertyAccessDeniedException extends RuntimeException
|
|
||||||
{
|
|
||||||
}
|
|
|
@ -12,7 +12,6 @@
|
||||||
namespace Symfony\Component\PropertyAccess;
|
namespace Symfony\Component\PropertyAccess;
|
||||||
|
|
||||||
use Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException;
|
use Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException;
|
||||||
use Symfony\Component\PropertyAccess\Exception\PropertyAccessDeniedException;
|
|
||||||
use Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException;
|
use Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -180,9 +179,8 @@ class PropertyAccessor implements PropertyAccessorInterface
|
||||||
*
|
*
|
||||||
* @return mixed The value of the read property
|
* @return mixed The value of the read property
|
||||||
*
|
*
|
||||||
* @throws NoSuchPropertyException If the property does not exist
|
* @throws NoSuchPropertyException If the property does not exist or is not
|
||||||
* @throws PropertyAccessDeniedException If the property cannot be accessed due to
|
* public.
|
||||||
* access restrictions (private or protected)
|
|
||||||
*/
|
*/
|
||||||
private function &readProperty(&$object, $property)
|
private function &readProperty(&$object, $property)
|
||||||
{
|
{
|
||||||
|
@ -202,41 +200,38 @@ class PropertyAccessor implements PropertyAccessorInterface
|
||||||
$getter = 'get'.$camelProp;
|
$getter = 'get'.$camelProp;
|
||||||
$isser = 'is'.$camelProp;
|
$isser = 'is'.$camelProp;
|
||||||
$hasser = 'has'.$camelProp;
|
$hasser = 'has'.$camelProp;
|
||||||
|
$classHasProperty = $reflClass->hasProperty($property);
|
||||||
|
|
||||||
if ($reflClass->hasMethod($getter)) {
|
if ($reflClass->hasMethod($getter) && $reflClass->getMethod($getter)->isPublic()) {
|
||||||
if (!$reflClass->getMethod($getter)->isPublic()) {
|
|
||||||
throw new PropertyAccessDeniedException(sprintf('Method "%s()" is not public in class "%s"', $getter, $reflClass->name));
|
|
||||||
}
|
|
||||||
|
|
||||||
$result[self::VALUE] = $object->$getter();
|
$result[self::VALUE] = $object->$getter();
|
||||||
} elseif ($reflClass->hasMethod($isser)) {
|
} elseif ($reflClass->hasMethod($isser) && $reflClass->getMethod($isser)->isPublic()) {
|
||||||
if (!$reflClass->getMethod($isser)->isPublic()) {
|
|
||||||
throw new PropertyAccessDeniedException(sprintf('Method "%s()" is not public in class "%s"', $isser, $reflClass->name));
|
|
||||||
}
|
|
||||||
|
|
||||||
$result[self::VALUE] = $object->$isser();
|
$result[self::VALUE] = $object->$isser();
|
||||||
} elseif ($reflClass->hasMethod($hasser)) {
|
} elseif ($reflClass->hasMethod($hasser) && $reflClass->getMethod($hasser)->isPublic()) {
|
||||||
if (!$reflClass->getMethod($hasser)->isPublic()) {
|
|
||||||
throw new PropertyAccessDeniedException(sprintf('Method "%s()" is not public in class "%s"', $hasser, $reflClass->name));
|
|
||||||
}
|
|
||||||
|
|
||||||
$result[self::VALUE] = $object->$hasser();
|
$result[self::VALUE] = $object->$hasser();
|
||||||
} elseif ($reflClass->hasMethod('__get')) {
|
} elseif ($reflClass->hasMethod('__get') && $reflClass->getMethod('__get')->isPublic()) {
|
||||||
// needed to support magic method __get
|
|
||||||
$result[self::VALUE] = $object->$property;
|
$result[self::VALUE] = $object->$property;
|
||||||
} elseif ($reflClass->hasProperty($property)) {
|
} elseif ($classHasProperty && $reflClass->getProperty($property)->isPublic()) {
|
||||||
if (!$reflClass->getProperty($property)->isPublic()) {
|
|
||||||
throw new PropertyAccessDeniedException(sprintf('Property "%s" is not public in class "%s". Maybe you should create the method "%s()" or "%s()" or "%s()"?', $property, $reflClass->name, $getter, $isser, $hasser));
|
|
||||||
}
|
|
||||||
|
|
||||||
$result[self::VALUE] =& $object->$property;
|
$result[self::VALUE] =& $object->$property;
|
||||||
$result[self::IS_REF] = true;
|
$result[self::IS_REF] = true;
|
||||||
} elseif (property_exists($object, $property)) {
|
} elseif (!$classHasProperty && property_exists($object, $property)) {
|
||||||
// needed to support \stdClass instances
|
// Needed to support \stdClass instances. We need to explicitly
|
||||||
|
// exclude $classHasProperty, otherwise if in the previous clause
|
||||||
|
// a *protected* property was found on the class, property_exists()
|
||||||
|
// returns true, consequently the following line will result in a
|
||||||
|
// fatal error.
|
||||||
$result[self::VALUE] =& $object->$property;
|
$result[self::VALUE] =& $object->$property;
|
||||||
$result[self::IS_REF] = true;
|
$result[self::IS_REF] = true;
|
||||||
} else {
|
} else {
|
||||||
throw new NoSuchPropertyException(sprintf('Neither property "%s" nor method "%s()" nor method "%s()" exists in class "%s"', $property, $getter, $isser, $reflClass->name));
|
throw new NoSuchPropertyException(sprintf(
|
||||||
|
'Neither the property "%s" nor one of the methods "%s()", '.
|
||||||
|
'"%s()", "%s()" or "__get()" exist and have public access in '.
|
||||||
|
'class "%s".',
|
||||||
|
$property,
|
||||||
|
$getter,
|
||||||
|
$isser,
|
||||||
|
$hasser,
|
||||||
|
$reflClass->name
|
||||||
|
));
|
||||||
}
|
}
|
||||||
|
|
||||||
// Objects are always passed around by reference
|
// Objects are always passed around by reference
|
||||||
|
@ -268,18 +263,17 @@ class PropertyAccessor implements PropertyAccessorInterface
|
||||||
/**
|
/**
|
||||||
* Sets the value of the property at the given index in the path
|
* Sets the value of the property at the given index in the path
|
||||||
*
|
*
|
||||||
* @param object|array $object The object or array to write to
|
* @param object|array $object The object or array to write to
|
||||||
* @param string $property The property to write
|
* @param string $property The property to write
|
||||||
* @param string|null $singular The singular form of the property name or null
|
* @param string|null $singular The singular form of the property name or null
|
||||||
* @param mixed $value The value to write
|
* @param mixed $value The value to write
|
||||||
*
|
*
|
||||||
* @throws NoSuchPropertyException If the property does not exist
|
* @throws NoSuchPropertyException If the property does not exist or is not
|
||||||
* @throws PropertyAccessDeniedException If the property cannot be accessed due to
|
* public.
|
||||||
* access restrictions (private or protected)
|
|
||||||
*/
|
*/
|
||||||
private function writeProperty(&$object, $property, $singular, $value)
|
private function writeProperty(&$object, $property, $singular, $value)
|
||||||
{
|
{
|
||||||
$adderRemoverError = null;
|
$guessedAdders = '';
|
||||||
|
|
||||||
if (!is_object($object)) {
|
if (!is_object($object)) {
|
||||||
throw new NoSuchPropertyException(sprintf('Cannot write property "%s" to an array. Maybe you should write the property path as "[%s]" instead?', $property, $property));
|
throw new NoSuchPropertyException(sprintf('Cannot write property "%s" to an array. Maybe you should write the property path as "[%s]" instead?', $property, $property));
|
||||||
|
@ -330,38 +324,39 @@ class PropertyAccessor implements PropertyAccessorInterface
|
||||||
|
|
||||||
return;
|
return;
|
||||||
} else {
|
} else {
|
||||||
$adderRemoverError = ', nor could adders and removers be found based on the ';
|
// It is sufficient to include only the adders in the error
|
||||||
if (null === $singular) {
|
// message. If the user implements the adder but not the remover,
|
||||||
// $adderRemoverError .= 'guessed singulars: '.implode(', ', $singulars).' (provide a singular by suffixing the property path with "|{singular}" to override the guesser)';
|
// an exception will be thrown in findAdderAndRemover() that
|
||||||
$adderRemoverError .= 'guessed singulars: '.implode(', ', $singulars);
|
// the remover has to be implemented as well.
|
||||||
} else {
|
$guessedAdders = '"add'.implode('()", "add', $singulars).'()", ';
|
||||||
$adderRemoverError .= 'passed singular: '.$singular;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
$setter = 'set'.$this->camelize($property);
|
$setter = 'set'.$this->camelize($property);
|
||||||
|
$classHasProperty = $reflClass->hasProperty($property);
|
||||||
|
|
||||||
if ($reflClass->hasMethod($setter)) {
|
if ($reflClass->hasMethod($setter) && $reflClass->getMethod($setter)->isPublic()) {
|
||||||
if (!$reflClass->getMethod($setter)->isPublic()) {
|
|
||||||
throw new PropertyAccessDeniedException(sprintf('Method "%s()" is not public in class "%s"', $setter, $reflClass->name));
|
|
||||||
}
|
|
||||||
|
|
||||||
$object->$setter($value);
|
$object->$setter($value);
|
||||||
} elseif ($reflClass->hasMethod('__set')) {
|
} elseif ($reflClass->hasMethod('__set') && $reflClass->getMethod('__set')->isPublic()) {
|
||||||
// needed to support magic method __set
|
|
||||||
$object->$property = $value;
|
$object->$property = $value;
|
||||||
} elseif ($reflClass->hasProperty($property)) {
|
} elseif ($classHasProperty && $reflClass->getProperty($property)->isPublic()) {
|
||||||
if (!$reflClass->getProperty($property)->isPublic()) {
|
|
||||||
throw new PropertyAccessDeniedException(sprintf('Property "%s" is not public in class "%s"%s. Maybe you should create the method "%s()"?', $property, $reflClass->name, $adderRemoverError, $setter));
|
|
||||||
}
|
|
||||||
|
|
||||||
$object->$property = $value;
|
$object->$property = $value;
|
||||||
} elseif (property_exists($object, $property)) {
|
} elseif (!$classHasProperty && property_exists($object, $property)) {
|
||||||
// needed to support \stdClass instances
|
// Needed to support \stdClass instances. We need to explicitly
|
||||||
|
// exclude $classHasProperty, otherwise if in the previous clause
|
||||||
|
// a *protected* property was found on the class, property_exists()
|
||||||
|
// returns true, consequently the following line will result in a
|
||||||
|
// fatal error.
|
||||||
$object->$property = $value;
|
$object->$property = $value;
|
||||||
} else {
|
} else {
|
||||||
throw new NoSuchPropertyException(sprintf('Neither element "%s" nor method "%s()" exists in class "%s"%s', $property, $setter, $reflClass->name, $adderRemoverError));
|
throw new NoSuchPropertyException(sprintf(
|
||||||
|
'Neither the property "%s" nor one of the methods %s"%s()" or '.
|
||||||
|
'"__set()" exist and have public access in class "%s".',
|
||||||
|
$property,
|
||||||
|
$guessedAdders,
|
||||||
|
$setter,
|
||||||
|
$reflClass->name
|
||||||
|
));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -402,7 +397,7 @@ class PropertyAccessor implements PropertyAccessorInterface
|
||||||
|
|
||||||
if ($addMethodFound xor $removeMethodFound) {
|
if ($addMethodFound xor $removeMethodFound) {
|
||||||
throw new NoSuchPropertyException(sprintf(
|
throw new NoSuchPropertyException(sprintf(
|
||||||
'Found the public method "%s", but did not find a public "%s" on class %s',
|
'Found the public method "%s()", but did not find a public "%s()" on class %s',
|
||||||
$addMethodFound ? $addMethod : $removeMethod,
|
$addMethodFound ? $addMethod : $removeMethod,
|
||||||
$addMethodFound ? $removeMethod : $addMethod,
|
$addMethodFound ? $removeMethod : $addMethod,
|
||||||
$reflClass->name
|
$reflClass->name
|
||||||
|
|
|
@ -43,11 +43,9 @@ interface PropertyAccessorInterface
|
||||||
* @param string|PropertyPathInterface $propertyPath The property path to modify
|
* @param string|PropertyPathInterface $propertyPath The property path to modify
|
||||||
* @param mixed $value The value to set at the end of the property path
|
* @param mixed $value The value to set at the end of the property path
|
||||||
*
|
*
|
||||||
* @throws Exception\NoSuchPropertyException If a property does not exist
|
* @throws Exception\NoSuchPropertyException If a property does not exist or is not public.
|
||||||
* @throws Exception\PropertyAccessDeniedException If a property cannot be accessed due to
|
* @throws Exception\UnexpectedTypeException If a value within the path is neither object
|
||||||
* access restrictions (private or protected)
|
* nor array
|
||||||
* @throws Exception\UnexpectedTypeException If a value within the path is neither object
|
|
||||||
* nor array
|
|
||||||
*/
|
*/
|
||||||
public function setValue(&$objectOrArray, $propertyPath, $value);
|
public function setValue(&$objectOrArray, $propertyPath, $value);
|
||||||
|
|
||||||
|
@ -77,8 +75,7 @@ interface PropertyAccessorInterface
|
||||||
*
|
*
|
||||||
* @return mixed The value at the end of the property path
|
* @return mixed The value at the end of the property path
|
||||||
*
|
*
|
||||||
* @throws Exception\NoSuchPropertyException If the property/getter does not exist
|
* @throws Exception\NoSuchPropertyException If a property does not exist or is not public.
|
||||||
* @throws Exception\PropertyAccessDeniedException If the property/getter exists but is not public
|
|
||||||
*/
|
*/
|
||||||
public function getValue($objectOrArray, $propertyPath);
|
public function getValue($objectOrArray, $propertyPath);
|
||||||
}
|
}
|
||||||
|
|
|
@ -288,14 +288,10 @@ abstract class PropertyAccessorCollectionTest extends \PHPUnit_Framework_TestCas
|
||||||
$car = $this->getMock(__CLASS__.'_CarNoAdderAndRemover');
|
$car = $this->getMock(__CLASS__.'_CarNoAdderAndRemover');
|
||||||
$propertyPath = 'axes';
|
$propertyPath = 'axes';
|
||||||
$expectedMessage = sprintf(
|
$expectedMessage = sprintf(
|
||||||
'Neither element "axes" nor method "setAxes()" exists in class '
|
'Neither the property "axes" nor one of the methods "addAx()", '.
|
||||||
.'"%s", nor could adders and removers be found based on the '
|
'"addAxe()", "addAxis()", "setAxes()" or "__set()" exist and have '.
|
||||||
.'guessed singulars: %s'
|
'public access in class "%s".',
|
||||||
// .'(provide a singular by suffixing the '
|
get_class($car)
|
||||||
// .'property path with "|{singular}" to override the guesser)'
|
|
||||||
,
|
|
||||||
get_class($car),
|
|
||||||
implode(', ', (array) $singulars = StringUtil::singularify('Axes'))
|
|
||||||
);
|
);
|
||||||
$data[] = array($car, $propertyPath, $expectedMessage);
|
$data[] = array($car, $propertyPath, $expectedMessage);
|
||||||
|
|
||||||
|
@ -316,14 +312,10 @@ abstract class PropertyAccessorCollectionTest extends \PHPUnit_Framework_TestCas
|
||||||
$car = $this->getMock(__CLASS__.'_CarNoAdderAndRemoverWithProperty');
|
$car = $this->getMock(__CLASS__.'_CarNoAdderAndRemoverWithProperty');
|
||||||
$propertyPath = 'axes';
|
$propertyPath = 'axes';
|
||||||
$expectedMessage = sprintf(
|
$expectedMessage = sprintf(
|
||||||
'Property "axes" is not public in class "%s", nor could adders and '
|
'Neither the property "axes" nor one of the methods "addAx()", '.
|
||||||
.'removers be found based on the guessed singulars: %s'
|
'"addAxe()", "addAxis()", "setAxes()" or "__set()" exist and have '.
|
||||||
// .' (provide a singular by suffixing the property path with '
|
'public access in class "%s".',
|
||||||
// .'"|{singular}" to override the guesser)'
|
get_class($car)
|
||||||
.'. Maybe you should '
|
|
||||||
.'create the method "setAxes()"?',
|
|
||||||
get_class($car),
|
|
||||||
implode(', ', (array) $singulars = StringUtil::singularify('Axes'))
|
|
||||||
);
|
);
|
||||||
$data[] = array($car, $propertyPath, $expectedMessage);
|
$data[] = array($car, $propertyPath, $expectedMessage);
|
||||||
|
|
||||||
|
|
|
@ -114,7 +114,7 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @expectedException \Symfony\Component\PropertyAccess\Exception\PropertyAccessDeniedException
|
* @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException
|
||||||
*/
|
*/
|
||||||
public function testGetValueThrowsExceptionIfPropertyIsNotPublic()
|
public function testGetValueThrowsExceptionIfPropertyIsNotPublic()
|
||||||
{
|
{
|
||||||
|
@ -138,7 +138,7 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @expectedException \Symfony\Component\PropertyAccess\Exception\PropertyAccessDeniedException
|
* @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException
|
||||||
*/
|
*/
|
||||||
public function testGetValueThrowsExceptionIfGetterIsNotPublic()
|
public function testGetValueThrowsExceptionIfGetterIsNotPublic()
|
||||||
{
|
{
|
||||||
|
@ -180,7 +180,7 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @expectedException \Symfony\Component\PropertyAccess\Exception\PropertyAccessDeniedException
|
* @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException
|
||||||
*/
|
*/
|
||||||
public function testGetValueThrowsExceptionIfIsserIsNotPublic()
|
public function testGetValueThrowsExceptionIfIsserIsNotPublic()
|
||||||
{
|
{
|
||||||
|
@ -295,7 +295,7 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @expectedException \Symfony\Component\PropertyAccess\Exception\PropertyAccessDeniedException
|
* @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException
|
||||||
*/
|
*/
|
||||||
public function testSetValueThrowsExceptionIfGetterIsNotPublic()
|
public function testSetValueThrowsExceptionIfGetterIsNotPublic()
|
||||||
{
|
{
|
||||||
|
|
Reference in New Issue