bug #15878 [OptionsResolver] Fix catched exception along the dependency tree mistakenly detects cyclic dependencies (lemoinem)
This PR was submitted for the 2.8 branch but it was merged into the 2.7 branch instead (closes #15878).
Discussion
----------
[OptionsResolver] Fix catched exception along the dependency tree mistakenly detects cyclic dependencies
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | N/A
| License | MIT
| Doc PR | N/A
If an Option's normalizer (or lazy default) catches an exception thrown from one of its dependencies, the OptionResolver was left in an inconsistent state which would trigger a false positive cyclic dependency if the throwing option or any of its parent dependencies between it and the catching one are ever called again.
Commits
-------
9a188c5
[OptionsResolver] Fix catched exception along the dependency tree mistakenly detects cyclic dependencies
This commit is contained in:
commit
8eda312fc4
@ -854,9 +854,14 @@ class OptionsResolver implements Options, OptionsResolverInterface
|
||||
// dependency
|
||||
// BEGIN
|
||||
$this->calling[$option] = true;
|
||||
try {
|
||||
foreach ($this->lazy[$option] as $closure) {
|
||||
$value = $closure($this, $value);
|
||||
}
|
||||
} catch (\Exception $e) {
|
||||
unset($this->calling[$option]);
|
||||
throw $e;
|
||||
}
|
||||
unset($this->calling[$option]);
|
||||
// END
|
||||
}
|
||||
@ -953,7 +958,12 @@ class OptionsResolver implements Options, OptionsResolverInterface
|
||||
// dependency
|
||||
// BEGIN
|
||||
$this->calling[$option] = true;
|
||||
try {
|
||||
$value = $normalizer($this, $value);
|
||||
} catch (\Exception $e) {
|
||||
unset($this->calling[$option]);
|
||||
throw $e;
|
||||
}
|
||||
unset($this->calling[$option]);
|
||||
// END
|
||||
}
|
||||
|
@ -1103,6 +1103,56 @@ class OptionsResolver2Dot6Test extends \PHPUnit_Framework_TestCase
|
||||
$this->resolver->resolve();
|
||||
}
|
||||
|
||||
public function testCatchedExceptionFromNormalizerDoesNotCrashOptionResolver()
|
||||
{
|
||||
$throw = true;
|
||||
|
||||
$this->resolver->setDefaults(array('catcher' => null, 'thrower' => null));
|
||||
|
||||
$this->resolver->setNormalizer('catcher', function (Options $options) {
|
||||
try {
|
||||
return $options['thrower'];
|
||||
} catch(\Exception $e) {
|
||||
return false;
|
||||
}
|
||||
});
|
||||
|
||||
$this->resolver->setNormalizer('thrower', function (Options $options) use (&$throw) {
|
||||
if ($throw) {
|
||||
$throw = false;
|
||||
throw new \UnexpectedValueException('throwing');
|
||||
}
|
||||
|
||||
return true;
|
||||
});
|
||||
|
||||
$this->resolver->resolve();
|
||||
}
|
||||
|
||||
public function testCatchedExceptionFromLazyDoesNotCrashOptionResolver()
|
||||
{
|
||||
$throw = true;
|
||||
|
||||
$this->resolver->setDefault('catcher', function (Options $options) {
|
||||
try {
|
||||
return $options['thrower'];
|
||||
} catch(\Exception $e) {
|
||||
return false;
|
||||
}
|
||||
});
|
||||
|
||||
$this->resolver->setDefault('thrower', function (Options $options) use (&$throw) {
|
||||
if ($throw) {
|
||||
$throw = false;
|
||||
throw new \UnexpectedValueException('throwing');
|
||||
}
|
||||
|
||||
return true;
|
||||
});
|
||||
|
||||
$this->resolver->resolve();
|
||||
}
|
||||
|
||||
public function testInvokeEachNormalizerOnlyOnce()
|
||||
{
|
||||
$calls = 0;
|
||||
|
Reference in New Issue
Block a user