10000 [Routing] force stringable object to string in route parameter by garak · Pull Request #40342 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Routing] force stringable object to string in route parameter #40342

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 9 commits into from
Closed

[Routing] force stringable object to string in route parameter #40342

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6200b47
force stringable object to string in route parameter
garak Mar 2, 2021
cacdfab
fix cs
garak Mar 2, 2021
cf1ebac
map $extra instead of $parameters
garak Mar 2, 2021
d7ee79b
catch non-stringable objects
garak Mar 2, 2021
06c8319
fix unwanted cs changes
garak Mar 2, 2021
0db1cad
improve error message
garak Mar 2, 2021
e9e5ac6
expect expectation when is actually expected
garak Mar 2, 2021
4ff52e2
Let'use sprintf instead of strtr
garak Mar 2, 2021
3b83ec5
fix cs
garak Mar 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 25 additions & 13 deletions src/Symfony/Component/Routing/Generator/UrlGenerator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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('/\(\?(?:=|<=|!|<!)((?:[^()\\\\]+|\\\\.|\((?1)\))*)\)/', '', $token[2]).'$#i'.(empty($token[4]) ? '' : 'u'), $mergedParams[$token[3]])) {
if (null !== $this->strictRequirements && !preg_match('#^' . preg_replace('/\(\?(?:=|<=|!|<!)((?:[^()\\\\]+|\\\\.|\((?1)\))*)\)/', '', $token[2]) . '$#i' . (empty($token[4]) ? '' : 'u'), $mergedParams[$token[3]])) {
if ($this->strictRequirements) {
throw new InvalidParameterException(strtr($message, ['{parameter}' => $varName, '{route}' => $name, '{expected}' => $token[2], '{given}' => $mergedParams[$varName]]));
}
Expand All @@ -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;
}
}
Expand All @@ -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 = '';
Expand All @@ -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('/\(\?(?:=|<=|!|<!)((?:[^()\\\\]+|\\\\.|\((?1)\))*)\)/', '', $token[2]).'$#i'.(empty($token[4]) ? '' : 'u'), $mergedParams[$token[3]])) {
if (null !== $this->strictRequirements && !preg_match('#^' . preg_replace('/\(\?(?:=|<=|!|<!)((?:[^()\\\\]+|\\\\.|\((?1)\))*)\)/', '', $token[2]) . '$#i' . (empty($token[4]) ? '' : 'u'), $mergedParams[$token[3]])) {
if ($this->strictRequirements) {
throw new InvalidParameterException(strtr($message, ['{parameter}' => $token[3], '{route}' => $name, '{expected}' => $token[2], '{given}' => $mergedParams[$token[3]]]));
}
Expand All @@ -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;
}
}

Expand All @@ -273,27 +273,39 @@ 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
$extra = array_udiff_assoc(array_diff_key($parameters, $variables), $defaults, function ($a, $b) {
return $a == $b ? 0 : 1;
});

// force string for an object. See https://bugs.php.net/bug.php?id=66966
foreach ($extra as $paramName => $param) {
if (!\is_object($param)) {
continue;
}
if (!\is_callable([$param, '__toString'])) {
Comment on lines +299 to +302
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a future PR can deal with if you pass an array or callable.
Or we may think that is not needed.

$message = 'Cannot convert parameter "{parameter}" to string.';
throw new InvalidParameterException(strtr($message, '{parameter}', $paramName));
}
$extra[$paramName] = (string) $param;
}

// extract fragment
$fragment = $defaults['_fragment'] ?? '';

Expand Down
11 changes: 11 additions & 0 deletions src/Symfony/Component/Routing/Tests/Generator/StringableStub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

namespace Symfony\Component\Routing\Tests\Generator;

class StringableStub
{
public function __toString()
{
return 'dummy';
}
}
17 changes: 17 additions & 0 deletions src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,23 @@ 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();
$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
*/
Expand Down
0