From 6200b47f1a6d6b9dd2952903a5d844a85d23903e Mon Sep 17 00:00:00 2001 From: Massimiliano Arione Date: Tue, 2 Mar 2021 11:56:05 +0100 Subject: [PATCH 1/9] force stringable object to string in route parameter --- .../Component/Routing/Generator/UrlGenerator.php | 3 +++ .../Routing/Tests/Generator/StringableStub.php | 11 +++++++++++ .../Routing/Tests/Generator/UrlGeneratorTest.php | 9 +++++++++ 3 files changed, 23 insertions(+) create mode 100644 src/Symfony/Component/Routing/Tests/Generator/StringableStub.php diff --git a/src/Symfony/Component/Routing/Generator/UrlGenerator.php b/src/Symfony/Component/Routing/Generator/UrlGenerator.php index 7adf2ed27e77c..168723a62cf02 100644 --- a/src/Symfony/Component/Routing/Generator/UrlGenerator.php +++ b/src/Symfony/Component/Routing/Generator/UrlGenerator.php @@ -172,6 +172,9 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa $variables = array_flip($variables); $mergedParams = array_replace($defaults, $this->context->getParameters(), $parameters); + // force string for an object. See https://bugs.php.net/bug.php?id=66966 + $parameters = array_map(static function ($param) { return is_object($param) ? (string) $param : $param; }, $parameters); + // all params must be given if ($diff = array_diff_key($variables, $mergedParams)) { throw new MissingMandatoryParametersException(sprintf('Some mandatory parameters are missing ("%s") to generate a URL for route "%s".', implode('", "', array_keys($diff)), $name)); diff --git a/src/Symfony/Component/Routing/Tests/Generator/StringableStub.php b/src/Symfony/Component/Routing/Tests/Generator/StringableStub.php new file mode 100644 index 0000000000000..47ea8583b3c08 --- /dev/null +++ b/src/Symfony/Component/Routing/Tests/Generator/StringableStub.php @@ -0,0 +1,11 @@ +assertEquals('/app.php/testing#fragment', $url); } + public function testStringableObjectParameterIsConvertedToString() + { + $object = new StringableStub(); + $routes = $this->getRoutes('test', new Route('/testing')); + $url = $this->getGenerator($routes)->generate('test', ['param' => $object]); + + $this->assertEquals('/app.php/testing?param=dummy', $url); + } + /** * @dataProvider provideLookAroundRequirementsInPath */ From cacdfab451b218174513625b2e6921d4cb50dc73 Mon Sep 17 00:00:00 2001 From: Massimiliano Arione Date: Tue, 2 Mar 2021 12:15:12 +0100 Subject: [PATCH 2/9] fix cs --- src/Symfony/Component/Routing/Generator/UrlGenerator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Routing/Generator/UrlGenerator.php b/src/Symfony/Component/Routing/Generator/UrlGenerator.php index 168723a62cf02..59dcba91a268f 100644 --- a/src/Symfony/Component/Routing/Generator/UrlGenerator.php +++ b/src/Symfony/Component/Routing/Generator/UrlGenerator.php @@ -173,7 +173,7 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa $mergedParams = array_replace($defaults, $this->context->getParameters(), $parameters); // force string for an object. See https://bugs.php.net/bug.php?id=66966 - $parameters = array_map(static function ($param) { return is_object($param) ? (string) $param : $param; }, $parameters); + $parameters = array_map(static function ($param) { return \is_object($param) ? (string) $param : $param; }, $parameters); // all params must be given if ($diff = array_diff_key($variables, $mergedParams)) { From cf1ebac252ccc0555875d8bad05d95b7e3e5f7d6 Mon Sep 17 00:00:00 2001 From: Massimiliano Arione Date: Tue, 2 Mar 2021 12:30:53 +0100 Subject: [PATCH 3/9] map $extra instead of $parameters --- src/Symfony/Component/Routing/Generator/UrlGenerator.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/Routing/Generator/UrlGenerator.php b/src/Symfony/Component/Routing/Generator/UrlGenerator.php index 59dcba91a268f..a99975190cc35 100644 --- a/src/Symfony/Component/Routing/Generator/UrlGenerator.php +++ b/src/Symfony/Component/Routing/Generator/UrlGenerator.php @@ -172,9 +172,6 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa $variables = array_flip($variables); $mergedParams = array_replace($defaults, $this->context->getParameters(), $parameters); - // force string for an object. See https://bugs.php.net/bug.php?id=66966 - $parameters = array_map(static function ($param) { return \is_object($param) ? (string) $param : $param; }, $parameters); - // all params must be given if ($diff = array_diff_key($variables, $mergedParams)) { throw new MissingMandatoryParametersException(sprintf('Some mandatory parameters are missing ("%s") to generate a URL for route "%s".', implode('", "', array_keys($diff)), $name)); @@ -297,6 +294,9 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa return $a == $b ? 0 : 1; }); + // force string for an object. See https://bugs.php.net/bug.php?id=66966 + $extra = array_map(static function ($param) { return \is_object($param) ? (string) $param : $param; }, $extra); + // extract fragment $fragment = $defaults['_fragment'] ?? ''; From d7ee79b2d25e158725d547678323597c822472dc Mon Sep 17 00:00:00 2001 From: Massimiliano Arione Date: Tue, 2 Mar 2021 13:43:59 +0100 Subject: [PATCH 4/9] catch non-stringable objects --- .../Routing/Generator/UrlGenerator.php | 37 ++++++++++++------- .../Tests/Generator/UrlGeneratorTest.php | 8 ++++ 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/Symfony/Component/Routing/Generator/UrlGenerator.php b/src/Symfony/Component/Routing/Generator/UrlGenerator.php index a99975190cc35..efb9fd9f1166e 100644 --- a/src/Symfony/Component/Routing/Generator/UrlGenerator.php +++ b/src/Symfony/Component/Routing/Generator/UrlGenerator.php @@ -186,9 +186,9 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa // variable is not important by default $important = $token[5] ?? false; - if (!$optional || $important || !\array_key_exists($varName, $defaults) || (null !== $mergedParams[$varName] && (string) $mergedParams[$varName] !== (string) $defaults[$varName])) { + if (!$optional || $important || !\array_key_exists($varName, $defaults) || (null !== $mergedParams[$varName] && (string)$mergedParams[$varName] !== (string)$defaults[$varName])) { // check requirement (while ignoring look-around patterns) - if (null !== $this->strictRequirements && !preg_match('#^'.preg_replace('/\(\?(?:=|<=|!|strictRequirements && !preg_match('#^' . preg_replace('/\(\?(?:=|<=|!|strictRequirements) { throw new InvalidParameterException(strtr($message, ['{parameter}' => $varName, '{route}' => $name, '{expected}' => $token[2], '{given}' => $mergedParams[$varName]])); } @@ -200,12 +200,12 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa return ''; } - $url = $token[1].$mergedParams[$varName].$url; + $url = $token[1] . $mergedParams[$varName] . $url; $optional = false; } } else { // static text - $url = $token[1].$url; + $url = $token[1] . $url; $optional = false; } } @@ -222,9 +222,9 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa // otherwise we would generate a URI that, when followed by a user agent (e.g. browser), does not match this route $url = strtr($url, ['/../' => '/%2E%2E/', '/./' => '/%2E/']); if ('/..' === substr($url, -3)) { - $url = substr($url, 0, -2).'%2E%2E'; + $url = substr($url, 0, -2) . '%2E%2E'; } elseif ('/.' === substr($url, -2)) { - $url = substr($url, 0, -1).'%2E'; + $url = substr($url, 0, -1) . '%2E'; } $schemeAuthority = ''; @@ -243,7 +243,7 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa foreach ($hostTokens as $token) { if ('variable' === $token[0]) { // check requirement (while ignoring look-around patterns) - if (null !== $this->strictRequirements && !preg_match('#^'.preg_replace('/\(\?(?:=|<=|!|strictRequirements && !preg_match('#^' . preg_replace('/\(\?(?:=|<=|!|strictRequirements) { throw new InvalidParameterException(strtr($message, ['{parameter}' => $token[3], '{route}' => $name, '{expected}' => $token[2], '{given}' => $mergedParams[$token[3]]])); } @@ -255,9 +255,9 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa return ''; } - $routeHost = $token[1].$mergedParams[$token[3]].$routeHost; + $routeHost = $token[1] . $mergedParams[$token[3]] . $routeHost; } else { - $routeHost = $token[1].$routeHost; + $routeHost = $token[1] . $routeHost; } } @@ -273,20 +273,20 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa if ('' !== $host || ('' !== $scheme && 'http' !== $scheme && 'https' !== $scheme)) { $port = ''; if ('http' === $scheme && 80 !== $this->context->getHttpPort()) { - $port = ':'.$this->context->getHttpPort(); + $port = ':' . $this->context->getHttpPort(); } elseif ('https' === $scheme && 443 !== $this->context->getHttpsPort()) { - $port = ':'.$this->context->getHttpsPort(); + $port = ':' . $this->context->getHttpsPort(); } $schemeAuthority = self::NETWORK_PATH === $referenceType || '' === $scheme ? '//' : "$scheme://"; - $schemeAuthority .= $host.$port; + $schemeAuthority .= $host . $port; } } if (self::RELATIVE_PATH === $referenceType) { $url = self::getRelativePath($this->context->getPathInfo(), $url); } else { - $url = $schemeAuthority.$this->context->getBaseUrl().$url; + $url = $schemeAuthority . $this->context->getBaseUrl() . $url; } // add a query string if needed @@ -295,7 +295,16 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa }); // force string for an object. See https://bugs.php.net/bug.php?id=66966 - $extra = array_map(static function ($param) { return \is_object($param) ? (string) $param : $param; }, $extra); + foreach ($extra as $paramName => $param) { + if (!\is_object($param)) { + continue; + } + if (!\is_callable([$param, '__toString'])) { + $message = 'Cannot convert parameter "{parameter}" to string.'; + throw new InvalidParameterException(strtr($message, '{parameter}', $paramName)); + } + $extra[$paramName] = (string) $param; + } // extract fragment $fragment = $defaults['_fragment'] ?? ''; diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php index 9c87e10da1082..4ace87634d96c 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php @@ -856,6 +856,14 @@ public function testFragmentsCanBeDefinedAsDefaults() $this->assertEquals('/app.php/testing#fragment', $url); } + public function testNonStringableObjectParameterCannotBeConvertedToString() + { + $this->expectException(InvalidParameterException::class); + $object = new \stdClass(); + $routes = $this->getRoutes('test', new Route('/testing')); + $this->getGenerator($routes)->generate('test', ['param' => $object]); + } + public function testStringableObjectParameterIsConvertedToString() { $object = new StringableStub(); From 06c8319ac4eae380a7ac604864f070740ad09030 Mon Sep 17 00:00:00 2001 From: Massimiliano Arione Date: Tue, 2 Mar 2021 14:13:20 +0100 Subject: [PATCH 5/9] fix unwanted cs changes --- .../Routing/Generator/UrlGenerator.php | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/Symfony/Component/Routing/Generator/UrlGenerator.php b/src/Symfony/Component/Routing/Generator/UrlGenerator.php index efb9fd9f1166e..eeb9050fb4730 100644 --- a/src/Symfony/Component/Routing/Generator/UrlGenerator.php +++ b/src/Symfony/Component/Routing/Generator/UrlGenerator.php @@ -186,9 +186,9 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa // variable is not important by default $important = $token[5] ?? false; - if (!$optional || $important || !\array_key_exists($varName, $defaults) || (null !== $mergedParams[$varName] && (string)$mergedParams[$varName] !== (string)$defaults[$varName])) { + if (!$optional || $important || !\array_key_exists($varName, $defaults) || (null !== $mergedParams[$varName] && (string) $mergedParams[$varName] !== (string) $defaults[$varName])) { // check requirement (while ignoring look-around patterns) - if (null !== $this->strictRequirements && !preg_match('#^' . preg_replace('/\(\?(?:=|<=|!|strictRequirements && !preg_match('#^'.preg_replace('/\(\?(?:=|<=|!|strictRequirements) { throw new InvalidParameterException(strtr($message, ['{parameter}' => $varName, '{route}' => $name, '{expected}' => $token[2], '{given}' => $mergedParams[$varName]])); } @@ -200,12 +200,12 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa return ''; } - $url = $token[1] . $mergedParams[$varName] . $url; + $url = $token[1].$mergedParams[$varName].$url; $optional = false; } } else { // static text - $url = $token[1] . $url; + $url = $token[1].$url; $optional = false; } } @@ -222,9 +222,9 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa // otherwise we would generate a URI that, when followed by a user agent (e.g. browser), does not match this route $url = strtr($url, ['/../' => '/%2E%2E/', '/./' => '/%2E/']); if ('/..' === substr($url, -3)) { - $url = substr($url, 0, -2) . '%2E%2E'; + $url = substr($url, 0, -2).'%2E%2E'; } elseif ('/.' === substr($url, -2)) { - $url = substr($url, 0, -1) . '%2E'; + $url = substr($url, 0, -1).'%2E'; } $schemeAuthority = ''; @@ -243,7 +243,7 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa foreach ($hostTokens as $token) { if ('variable' === $token[0]) { // check requirement (while ignoring look-around patterns) - if (null !== $this->strictRequirements && !preg_match('#^' . preg_replace('/\(\?(?:=|<=|!|strictRequirements && !preg_match('#^'.preg_replace('/\(\?(?:=|<=|!|strictRequirements) { throw new InvalidParameterException(strtr($message, ['{parameter}' => $token[3], '{route}' => $name, '{expected}' => $token[2], '{given}' => $mergedParams[$token[3]]])); } @@ -255,9 +255,9 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa return ''; } - $routeHost = $token[1] . $mergedParams[$token[3]] . $routeHost; + $routeHost = $token[1].$mergedParams[$token[3]].$routeHost; } else { - $routeHost = $token[1] . $routeHost; + $routeHost = $token[1].$routeHost; } } @@ -273,20 +273,20 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa if ('' !== $host || ('' !== $scheme && 'http' !== $scheme && 'https' !== $scheme)) { $port = ''; if ('http' === $scheme && 80 !== $this->context->getHttpPort()) { - $port = ':' . $this->context->getHttpPort(); + $port = ':'.$this->context->getHttpPort(); } elseif ('https' === $scheme && 443 !== $this->context->getHttpsPort()) { - $port = ':' . $this->context->getHttpsPort(); + $port = ':'.$this->context->getHttpsPort(); } $schemeAuthority = self::NETWORK_PATH === $referenceType || '' === $scheme ? '//' : "$scheme://"; - $schemeAuthority .= $host . $port; + $schemeAuthority .= $host.$port; } } if (self::RELATIVE_PATH === $referenceType) { $url = self::getRelativePath($this->context->getPathInfo(), $url); } else { - $url = $schemeAuthority . $this->context->getBaseUrl() . $url; + $url = $schemeAuthority.$this->context->getBaseUrl().$url; } // add a query string if needed From 0db1cad29da45b86510c8196954779526b970232 Mon Sep 17 00:00:00 2001 From: Massimiliano Arione Date: Tue, 2 Mar 2021 14:20:23 +0100 Subject: [PATCH 6/9] improve error message --- src/Symfony/Component/Routing/Generator/UrlGenerator.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Routing/Generator/UrlGenerator.php b/src/Symfony/Component/Routing/Generator/UrlGenerator.php index eeb9050fb4730..b07d5022d4b77 100644 --- a/src/Symfony/Component/Routing/Generator/UrlGenerator.php +++ b/src/Symfony/Component/Routing/Generator/UrlGenerator.php @@ -300,8 +300,8 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa continue; } if (!\is_callable([$param, '__toString'])) { - $message = 'Cannot convert parameter "{parameter}" to string.'; - throw new InvalidParameterException(strtr($message, '{parameter}', $paramName)); + $message = 'Error when generating an URL for "{route}". Cannot convert object of type {type} to string for parameter "{parameter}".'; + throw new InvalidParameterException(strtr($message, ['{route}' => $name, '{type}' => get_debug_type($param), '{parameter}' => $paramName])); } $extra[$paramName] = (string) $param; } From e9e5ac694f78e200e8f8c1b0885b7b4c322b66b9 Mon Sep 17 00:00:00 2001 From: Massimiliano Arione Date: Tue, 2 Mar 2021 14:49:15 +0100 Subject: [PATCH 7/9] expect expectation when is actually expected --- .../Component/Routing/Tests/Generator/UrlGeneratorTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php index 4ace87634d96c..3acf908b5ee7c 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php @@ -858,9 +858,9 @@ public function testFragmentsCanBeDefinedAsDefaults() public function testNonStringableObjectParameterCannotBeConvertedToString() { - $this->expectException(InvalidParameterException::class); $object = new \stdClass(); $routes = $this->getRoutes('test', new Route('/testing')); + $this->expectException(InvalidParameterException::class); $this->getGenerator($routes)->generate('test', ['param' => $object]); } From 4ff52e25596073a8c90f03b62ac289b9d597fd67 Mon Sep 17 00:00:00 2001 From: Massimiliano Arione Date: Tue, 2 Mar 2021 14:52:23 +0100 Subject: [PATCH 8/9] Let'use sprintf instead of strtr Co-authored-by: Tobias Nyholm --- src/Symfony/Component/Routing/Generator/UrlGenerator.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Symfony/Component/Routing/Generator/UrlGenerator.php b/src/Symfony/Component/Routing/Generator/UrlGenerator.php index b07d5022d4b77..1266db81490b9 100644 --- a/src/Symfony/Component/Routing/Generator/UrlGenerator.php +++ b/src/Symfony/Component/Routing/Generator/UrlGenerator.php @@ -300,8 +300,7 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa continue; } if (!\is_callable([$param, '__toString'])) { - $message = 'Error when generating an URL for "{route}". Cannot convert object of type {type} to string for parameter "{parameter}".'; - throw new InvalidParameterException(strtr($message, ['{route}' => $name, '{type}' => get_debug_type($param), '{parameter}' => $paramName])); + throw new InvalidParameterException(\sprintf('Error when generating an URL for "%s". Cannot convert object of type "%s" to string for parameter "%s".', $name, get_debug_type($param), $paramName)); } $extra[$paramName] = (string) $param; } From 3b83ec505fbfe80b37765918a453c15ffa44fa29 Mon Sep 17 00:00:00 2001 From: Massimiliano Arione Date: Tue, 2 Mar 2021 15:05:46 +0100 Subject: [PATCH 9/9] fix cs --- src/Symfony/Component/Routing/Generator/UrlGenerator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Routing/Generator/UrlGenerator.php b/src/Symfony/Component/Routing/Generator/UrlGenerator.php index 1266db81490b9..dfecac9ee6862 100644 --- a/src/Symfony/Component/Routing/Generator/UrlGenerator.php +++ b/src/Symfony/Component/Routing/Generator/UrlGenerator.php @@ -300,7 +300,7 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa continue; } if (!\is_callable([$param, '__toString'])) { - throw new InvalidParameterException(\sprintf('Error when generating an URL for "%s". Cannot convert object of type "%s" to string for parameter "%s".', $name, get_debug_type($param), $paramName)); + throw new InvalidParameterException(sprintf('Error when generating an URL for "%s". Cannot convert object of type "%s" to string for parameter "%s".', $name, get_debug_type($param), $paramName)); } $extra[$paramName] = (string) $param; }