From 7521af7ea0d0cd77604c7c11bfd58714cec776e3 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 3 Dec 2018 14:19:27 +0100 Subject: [PATCH 1/4] [Routing] ignore trailing slash for non-GET requests --- .../Component/Routing/Matcher/UrlMatcher.php | 13 ++++++------- .../Routing/Tests/Matcher/UrlMatcherTest.php | 11 +++++++++++ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/Symfony/Component/Routing/Matcher/UrlMatcher.php b/src/Symfony/Component/Routing/Matcher/UrlMatcher.php index fc4554fff5..4b6494288e 100644 --- a/src/Symfony/Component/Routing/Matcher/UrlMatcher.php +++ b/src/Symfony/Component/Routing/Matcher/UrlMatcher.php @@ -118,6 +118,10 @@ class UrlMatcher implements UrlMatcherInterface, RequestMatcherInterface */ protected function matchCollection($pathinfo, RouteCollection $routes) { + // HEAD and GET are equivalent as per RFC + if ('HEAD' === $method = $this->context->getMethod()) { + $method = 'GET'; + } $supportsTrailingSlash = '/' !== $pathinfo && '' !== $pathinfo && $this instanceof RedirectableUrlMatcherInterface; foreach ($routes as $name => $route) { @@ -128,7 +132,7 @@ class UrlMatcher implements UrlMatcherInterface, RequestMatcherInterface // check the static prefix of the URL first. Only use the more expensive preg_match when it matches if ('' === $staticPrefix || 0 === strpos($pathinfo, $staticPrefix)) { // no-op - } elseif (!$supportsTrailingSlash || ($requiredMethods && !\in_array('GET', $requiredMethods))) { + } elseif (!$supportsTrailingSlash || ($requiredMethods && !\in_array('GET', $requiredMethods)) || 'GET' !== $method) { continue; } elseif ('/' === substr($staticPrefix, -1) && substr($staticPrefix, 0, -1) === $pathinfo) { return $this->allow = array(); @@ -149,7 +153,7 @@ class UrlMatcher implements UrlMatcherInterface, RequestMatcherInterface } if ($hasTrailingSlash && '/' !== substr($pathinfo, -1)) { - if (!$requiredMethods || \in_array('GET', $requiredMethods)) { + if ((!$requiredMethods || \in_array('GET', $requiredMethods)) && 'GET' === $method) { return $this->allow = array(); } continue; @@ -168,11 +172,6 @@ class UrlMatcher implements UrlMatcherInterface, RequestMatcherInterface // check HTTP method requirement if ($requiredMethods) { - // HEAD and GET are equivalent as per RFC - if ('HEAD' === $method = $this->context->getMethod()) { - $method = 'GET'; - } - if (!\in_array($method, $requiredMethods)) { if (self::REQUIREMENT_MATCH === $status[0]) { $this->allow = array_merge($this->allow, $requiredMethods); diff --git a/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php b/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php index ab682732ca..85647f0bea 100644 --- a/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php +++ b/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php @@ -514,6 +514,17 @@ class UrlMatcherTest extends TestCase 'customerId' => '123', ); $this->assertEquals($expected, $matcher->match('/api/customers/123/contactpersons')); + + $coll = new RouteCollection(); + $coll->add('a', new Route('/api/customers/{customerId}/contactpersons/', array(), array(), array(), '', array(), array('get'))); + $coll->add('b', new Route('/api/customers/{customerId}/contactpersons', array(), array(), array(), '', array(), array('post'))); + + $matcher = $this->getUrlMatcher($coll, new RequestContext('', 'POST')); + $expected = array( + '_route' => 'b', + 'customerId' => '123', + ); + $this->assertEquals($expected, $matcher->match('/api/customers/123/contactpersons')); } protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null) From 5691818397659ccf623a92d7b9b3ea03d2a7d7b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Mon, 3 Dec 2018 19:01:20 +0100 Subject: [PATCH 2/4] [Workflow] Fixed BC break for Workflow metadata --- .../DependencyInjection/FrameworkExtension.php | 4 ++-- .../Tests/DependencyInjection/FrameworkExtensionTest.php | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 132d77f060..c42f91c408 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -525,7 +525,7 @@ class FrameworkExtension extends Extension } if ($transition['metadata']) { $transitionsMetadataDefinition->addMethodCall('attach', array( - $transitionDefinition, + new Reference($transitionId), $transition['metadata'], )); } @@ -547,7 +547,7 @@ class FrameworkExtension extends Extension } if ($transition['metadata']) { $transitionsMetadataDefinition->addMethodCall('attach', array( - $transitionDefinition, + new Reference($transitionId), $transition['metadata'], )); } diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index 2fbf496223..35da98ef7b 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -256,10 +256,8 @@ abstract class FrameworkExtensionTest extends TestCase $this->assertSame('attach', $transitionsMetadataCall[0]); $params = $transitionsMetadataCall[1]; $this->assertCount(2, $params); - $this->assertInstanceOf(Definition::class, $params[0]); - $this->assertSame(Workflow\Transition::class, $params[0]->getClass()); - $this->assertSame(array('submit', 'start', 'travis'), $params[0]->getArguments()); - $this->assertSame(array('title' => 'transition submit title'), $params[1]); + $this->assertInstanceOf(Reference::class, $params[0]); + $this->assertSame('state_machine.pull_request.transition.0', (string) $params[0]); $serviceMarkingStoreWorkflowDefinition = $container->getDefinition('workflow.service_marking_store_workflow'); /** @var Reference $markingStoreRef */ From bd0dbe415fb8ee1baa5742346aba396e741ee927 Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Mon, 3 Dec 2018 19:20:34 +0100 Subject: [PATCH 3/4] Fix empty output for debug:autowiring when reflection-docblock is not available --- .../Bundle/FrameworkBundle/Console/Descriptor/Descriptor.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/Descriptor.php b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/Descriptor.php index 3cfca24983..22f2d41d68 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/Descriptor.php +++ b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/Descriptor.php @@ -292,7 +292,7 @@ abstract class Descriptor implements DescriptorInterface */ public static function getClassDescription(string $class, string &$resolvedClass = null): string { - $resolvedClass = null; + $resolvedClass = $class; if (!interface_exists(DocBlockFactoryInterface::class)) { return ''; From b10e2638a4dbf58b1548dcef7ae3152123020a7b Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 3 Dec 2018 22:34:05 +0000 Subject: [PATCH 4/4] [VarExporter] fix dumping private properties from abstract classes --- src/Symfony/Component/VarExporter/Internal/Exporter.php | 3 ++- src/Symfony/Component/VarExporter/Internal/Hydrator.php | 7 ++++++- .../VarExporter/Tests/Fixtures/abstract-parent.php | 5 ++++- .../Component/VarExporter/Tests/Fixtures/final-error.php | 2 +- .../Component/VarExporter/Tests/Fixtures/private.php | 6 +----- .../Component/VarExporter/Tests/VarExporterTest.php | 7 +++++++ 6 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/Symfony/Component/VarExporter/Internal/Exporter.php b/src/Symfony/Component/VarExporter/Internal/Exporter.php index 563eadc8c4..bbb409e194 100644 --- a/src/Symfony/Component/VarExporter/Internal/Exporter.php +++ b/src/Symfony/Component/VarExporter/Internal/Exporter.php @@ -125,7 +125,8 @@ class Exporter $c = 'stdClass'; } elseif ('*' === $n[1]) { $n = substr($n, 3); - if ('Error' === $c = $class) { + $c = $reflector->getProperty($n)->class; + if ('Error' === $c) { $c = 'TypeError'; } elseif ('Exception' === $c) { $c = 'ErrorException'; diff --git a/src/Symfony/Component/VarExporter/Internal/Hydrator.php b/src/Symfony/Component/VarExporter/Internal/Hydrator.php index 0de27743d9..a1a60112d9 100644 --- a/src/Symfony/Component/VarExporter/Internal/Hydrator.php +++ b/src/Symfony/Component/VarExporter/Internal/Hydrator.php @@ -11,6 +11,8 @@ namespace Symfony\Component\VarExporter\Internal; +use Symfony\Component\VarExporter\Exception\ClassNotFoundException; + /** * @author Nicolas Grekas * @@ -59,7 +61,10 @@ class Hydrator }; } - $classReflector = Registry::$reflectors[$class] ?? Registry::getClassReflector($class); + if (!\class_exists($class) && !\interface_exists($class, false) && !\trait_exists($class, false)) { + throw new ClassNotFoundException($class); + } + $classReflector = new \ReflectionClass($class); if (!$classReflector->isInternal()) { return self::$hydrators[$class] = (self::$hydrators['stdClass'] ?? self::getHydrator('stdClass'))->bindTo(null, $class); diff --git a/src/Symfony/Component/VarExporter/Tests/Fixtures/abstract-parent.php b/src/Symfony/Component/VarExporter/Tests/Fixtures/abstract-parent.php index 5548b9f8c8..df6a067087 100644 --- a/src/Symfony/Component/VarExporter/Tests/Fixtures/abstract-parent.php +++ b/src/Symfony/Component/VarExporter/Tests/Fixtures/abstract-parent.php @@ -6,10 +6,13 @@ return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate( ], null, [ - 'Symfony\\Component\\VarExporter\\Tests\\ConcreteClass' => [ + 'Symfony\\Component\\VarExporter\\Tests\\AbstractClass' => [ 'foo' => [ 123, ], + 'bar' => [ + 234, + ], ], ], $o[0], diff --git a/src/Symfony/Component/VarExporter/Tests/Fixtures/final-error.php b/src/Symfony/Component/VarExporter/Tests/Fixtures/final-error.php index 78da251141..dc260dc024 100644 --- a/src/Symfony/Component/VarExporter/Tests/Fixtures/final-error.php +++ b/src/Symfony/Component/VarExporter/Tests/Fixtures/final-error.php @@ -6,7 +6,7 @@ return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate( ]), null, [ - 'Symfony\\Component\\VarExporter\\Tests\\FinalError' => [ + 'TypeError' => [ 'file' => [ \dirname(__DIR__).\DIRECTORY_SEPARATOR.'VarExporterTest.php', ], diff --git a/src/Symfony/Component/VarExporter/Tests/Fixtures/private.php b/src/Symfony/Component/VarExporter/Tests/Fixtures/private.php index a901afe6ea..a9e28416e5 100644 --- a/src/Symfony/Component/VarExporter/Tests/Fixtures/private.php +++ b/src/Symfony/Component/VarExporter/Tests/Fixtures/private.php @@ -10,17 +10,13 @@ return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate( 'Symfony\\Component\\VarExporter\\Tests\\MyPrivateValue' => [ 'prot' => [ 123, + 123, ], 'priv' => [ 234, 234, ], ], - 'Symfony\\Component\\VarExporter\\Tests\\MyPrivateChildValue' => [ - 'prot' => [ - 1 => 123, - ], - ], ], [ $o[0], diff --git a/src/Symfony/Component/VarExporter/Tests/VarExporterTest.php b/src/Symfony/Component/VarExporter/Tests/VarExporterTest.php index c88f445b76..ea0b6a4585 100644 --- a/src/Symfony/Component/VarExporter/Tests/VarExporterTest.php +++ b/src/Symfony/Component/VarExporter/Tests/VarExporterTest.php @@ -326,6 +326,12 @@ final class FinalStdClass extends \stdClass abstract class AbstractClass { protected $foo; + private $bar; + + protected function setBar($bar) + { + $this->bar = $bar; + } } class ConcreteClass extends AbstractClass @@ -333,5 +339,6 @@ class ConcreteClass extends AbstractClass public function __construct() { $this->foo = 123; + $this->setBar(234); } }