Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 11bb865

Browse filesBrowse files
committed
bug #18210 [PropertyAccess] Throw an UnexpectedTypeException when the type do not match (dunglas, nicolas-grekas)
This PR was merged into the 2.3 branch. Discussion ---------- [PropertyAccess] Throw an UnexpectedTypeException when the type do not match | Q | A | ------------- | --- | Branch? | 2.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #17738, #18032 | License | MIT | Doc PR | - Made in coordination with @dunglas, Diff best viewed [ignoring whitspaces](https://github.com/symfony/symfony/pull/18210/files?w=1). Quoting #18032: > it appears that the current implementation is error prone because it throws a `\TypeError` that is an `Error` in PHP 7 but a regular `Exception` in PHP 5 because it uses the Symfony polyfill. Programs wrote in PHP 5 and catching all exceptions will catch this "custom" `\TypeError ` but not those wrote in PHP 7. It can be very hard to debug. > In this alternative implementation: > * catching type mismatch is considered a bug fix and applied to Symfony 2.3 > * for every PHP versions, a domain exception is thrown Commits ------- 5fe2b06 [PropertyAccess] Reduce overhead of UnexpectedTypeException tracking 10c8d5e [PropertyAccess] Throw an UnexpectedTypeException when the type do not match
2 parents a5d9414 + 5fe2b06 commit 11bb865
Copy full SHA for 11bb865

File tree

4 files changed

+107
-16
lines changed
Filter options

4 files changed

+107
-16
lines changed

‎src/Symfony/Component/PropertyAccess/PropertyAccessor.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/PropertyAccess/PropertyAccessor.php
+57-14Lines changed: 57 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ class PropertyAccessor implements PropertyAccessorInterface
8989
private $magicCall;
9090
private $readPropertyCache = array();
9191
private $writePropertyCache = array();
92+
private static $previousErrorHandler;
9293

9394
/**
9495
* Should not be used by application code. Use
@@ -131,23 +132,66 @@ public function setValue(&$objectOrArray, $propertyPath, $value)
131132
self::IS_REF => true,
132133
));
133134

134-
for ($i = count($propertyValues) - 1; $i >= 0; --$i) {
135-
$objectOrArray = &$propertyValues[$i][self::VALUE];
135+
try {
136+
if (PHP_VERSION_ID < 70000) {
137+
self::$previousErrorHandler = set_error_handler(array(__CLASS__, 'handleError'));
138+
}
136139

137-
if ($overwrite) {
138-
$property = $propertyPath->getElement($i);
139-
//$singular = $propertyPath->singulars[$i];
140-
$singular = null;
140+
for ($i = count($propertyValues) - 1; $i >= 0; --$i) {
141+
$objectOrArray = &$propertyValues[$i][self::VALUE];
141142

142-
if ($propertyPath->isIndex($i)) {
143-
$this->writeIndex($objectOrArray, $property, $value);
144-
} else {
145-
$this->writeProperty($objectOrArray, $property, $singular, $value);
143+
if ($overwrite) {
144+
$property = $propertyPath->getElement($i);
145+
//$singular = $propertyPath->singulars[$i];
146+
$singular = null;
147+
148+
if ($propertyPath->isIndex($i)) {
149+
$this->writeIndex($objectOrArray, $property, $value);
150+
} else {
151+
$this->writeProperty($objectOrArray, $property, $singular, $value);
152+
}
146153
}
154+
155+
$value = &$objectOrArray;
156+
$overwrite = !$propertyValues[$i][self::IS_REF];
157+
}
158+
} catch (\TypeError $e) {
159+
try {
160+
self::throwUnexpectedTypeException($e->getMessage(), $e->getTrace(), 0);
161+
} catch (UnexpectedTypeException $e) {
147162
}
163+
} catch (\Exception $e) {
164+
} catch (\Throwable $e) {
165+
}
148166

149-
$value = &$objectOrArray;
150-
$overwrite = !$propertyValues[$i][self::IS_REF];
167+
if (PHP_VERSION_ID < 70000) {
168+
restore_error_handler();
169+
self::$previousErrorHandler = null;
170+
}
171+
if (isset($e)) {
172+
throw $e;
173+
}
174+
}
175+
176+
/**
177+
* @internal
178+
*/
179+
public static function handleError($type, $message, $file, $line, $context)
180+
{
181+
if (E_RECOVERABLE_ERROR === $type) {
182+
self::throwUnexpectedTypeException($message, debug_backtrace(false), 1);
183+
}
184+
185+
return null !== self::$previousErrorHandler && false !== call_user_func(self::$previousErrorHandler, $type, $message, $file, $line, $context);
186+
}
187+
188+
private static function throwUnexpectedTypeException($message, $trace, $i)
189+
{
190+
if (isset($trace[$i]['file']) && __FILE__ === $trace[$i]['file']) {
191+
$pos = strpos($message, $delim = 'must be of the type ') ?: strpos($message, $delim = 'must be an instance of ');
192+
$pos += strlen($delim);
193+
194+
throw new UnexpectedTypeException($trace[$i]['args'][0], substr($message, $pos, strpos($message, ',', $pos) - $pos));
151195
}
152196
}
153197

@@ -398,8 +442,7 @@ private function writeIndex(&$array, $index, $value)
398442
* @param string|null $singular The singular form of the property name or null
399443
* @param mixed $value The value to write
400444
*
401-
* @throws NoSuchPropertyException If the property does not exist or is not
402-
* public.
445+
* @throws NoSuchPropertyException If the property does not exist or is not public.
403446
*/
404447
private function writeProperty(&$object, $property, $singular, $value)
405448
{

‎src/Symfony/Component/PropertyAccess/PropertyAccessorInterface.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/PropertyAccess/PropertyAccessorInterface.php
+1-2Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,7 @@ interface PropertyAccessorInterface
4444
* @param mixed $value The value to set at the end of the property path
4545
*
4646
* @throws Exception\NoSuchPropertyException If a property does not exist or is not public.
47-
* @throws Exception\UnexpectedTypeException If a value within the path is neither object
48-
* nor array
47+
* @throws Exception\UnexpectedTypeException If a value within the path is neither object nor array.
4948
*/
5049
public function setValue(&$objectOrArray, $propertyPath, $value);
5150

+30Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\PropertyAccess\Tests\Fixtures;
13+
14+
/**
15+
* @author Kévin Dunglas <dunglas@gmail.com>
16+
*/
17+
class TypeHinted
18+
{
19+
private $date;
20+
21+
public function setDate(\DateTime $date)
22+
{
23+
$this->date = $date;
24+
}
25+
26+
public function getDate()
27+
{
28+
return $this->date;
29+
}
30+
}

‎src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php
+19Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Component\PropertyAccess\Tests\Fixtures\Magician;
1717
use Symfony\Component\PropertyAccess\Tests\Fixtures\MagicianCall;
1818
use Symfony\Component\PropertyAccess\Tests\Fixtures\Ticket5775Object;
19+
use Symfony\Component\PropertyAccess\Tests\Fixtures\TypeHinted;
1920

2021
class PropertyAccessorTest extends \PHPUnit_Framework_TestCase
2122
{
@@ -403,4 +404,22 @@ public function getValidPropertyPaths()
403404
array(array('root' => array('index' => array())), '[root][index][firstName]', null),
404405
);
405406
}
407+
408+
/**
409+
* @expectedException \Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException
410+
* @expectedExceptionMessage Expected argument of type "DateTime", "string" given
411+
*/
412+
public function testThrowTypeError()
413+
{
414+
$this->propertyAccessor->setValue(new TypeHinted(), 'date', 'This is a string, \DateTime excepted.');
415+
}
416+
417+
public function testSetTypeHint()
418+
{
419+
$date = new \DateTime();
420+
$object = new TypeHinted();
421+
422+
$this->propertyAccessor->setValue($object, 'date', $date);
423+
$this->assertSame($date, $object->getDate());
424+
}
406425
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.