Skip to content

[12.x] Fix URL generation with optional parameters (regression in #54811) #55213

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

Merged
merged 3 commits into from
Mar 30, 2025
Merged

[12.x] Fix URL generation with optional parameters (regression in #54811) #55213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
928d350
Fix URL generation with optional parameters (regression in #54811)
stancl Mar 30, 2025
dd33ae1
Apply StyleCI diff
stancl Mar 30, 2025
1a070e0
formatting
taylorotwell Mar 30, 2025
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
4 changes: 2 additions & 2 deletions src/Illuminate/Routing/Route.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1321,9 +1321,9 @@ public function toSymfonyRoute()
/**
* Get the optional parameter names for the route.
*
* @return array
* @return array<string, null>
*/
protected function getOptionalParameterNames()
public function getOptionalParameterNames()
{
preg_match_all('/\{(\w+?)\?\}/', $this->uri(), $matches);

Expand Down
73 changes: 57 additions & 16 deletions src/Illuminate/Routing/RouteUrlGenerator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,13 +183,12 @@ protected function formatParameters(Route $route, $parameters)
{
$parameters = Arr::wrap($parameters);

$passedParameterCount = count($parameters);

$namedParameters = [];
$namedQueryParameters = [];
$routeParametersWithoutDefaultsOrNamedParameters = [];
$requiredRouteParametersWithoutDefaultsOrNamedParameters = [];

$routeParameters = $route->parameterNames();
$optionalParameters = $route->getOptionalParameterNames();

foreach ($routeParameters as $name) {
if (isset($parameters[$name])) {
Expand All @@ -198,9 +197,9 @@ protected function formatParameters(Route $route, $parameters)
unset($parameters[$name]);

continue;
} elseif (! isset($this->defaultParameters[$name])) {
// No named parameter or default value, try to match to positional parameter below...
array_push($routeParametersWithoutDefaultsOrNamedParameters, $name);
} elseif (! isset($this->defaultParameters[$name]) && ! isset($optionalParameters[$name])) {
// No named parameter or default value for a required parameter, try to match to positional parameter below...
array_push($requiredRouteParametersWithoutDefaultsOrNamedParameters, $name);
}

$namedParameters[$name] = '';
Expand All @@ -216,8 +215,8 @@ protected function formatParameters(Route $route, $parameters)
}

// Match positional parameters to the route parameters that didn't have a value in order...
if (count($parameters) == count($routeParametersWithoutDefaultsOrNamedParameters)) {
foreach (array_reverse($routeParametersWithoutDefaultsOrNamedParameters) as $name) {
if (count($parameters) == count($requiredRouteParametersWithoutDefaultsOrNamedParameters)) {
foreach (array_reverse($requiredRouteParametersWithoutDefaultsOrNamedParameters) as $name) {
if (count($parameters) === 0) {
break;
}
Expand All @@ -226,19 +225,61 @@ protected function formatParameters(Route $route, $parameters)
}
}

// If there are extra parameters, just fill left to right... if not, fill right to left and try to use defaults...
$extraParameters = $passedParameterCount > count($routeParameters);
$offset = 0;
$emptyParameters = array_filter($namedParameters, static fn ($val) => $val === '');

foreach ($extraParameters ? $namedParameters : array_reverse($namedParameters) as $key => $value) {
$bindingField = $route->bindingFieldFor($key);
if (count($requiredRouteParametersWithoutDefaultsOrNamedParameters) !== 0 &&
count($parameters) !== count($emptyParameters)) {
// Find the index of the first required parameter...
$offset = array_search($requiredRouteParametersWithoutDefaultsOrNamedParameters[0], array_keys($namedParameters));

$defaultParameterKey = $bindingField ? "$key:$bindingField" : $key;
// If more empty parameters remain, adjust the offset...
$remaining = count($emptyParameters) - $offset - count($parameters);

if ($remaining < 0) {
// Effectively subtract the remaining count since it's negative...
$offset += $remaining;
}

if ($value !== '') {
// Correct offset if it goes below zero...
if ($offset < 0) {
$offset = 0;
}
} elseif (count($requiredRouteParametersWithoutDefaultsOrNamedParameters) === 0 && count($parameters) !== 0) {
// Handle the case where all passed parameters are for parameters that have default values...
$remainingCount = count($parameters);

// Loop over empty parameters backwards and stop when we run out of passed parameters...
for ($i = count($namedParameters) - 1; $i >= 0; $i--) {
if ($namedParameters[array_keys($namedParameters)[$i]] === '') {
$offset = $i;
$remainingCount--;

if ($remainingCount === 0) {
// If there are no more passed parameters, we stop here...
break;
}
}
}
}

// Starting from the offset, match any passed parameters from left to right...
for ($i = $offset; $i < count($namedParameters); $i++) {
$key = array_keys($namedParameters)[$i];

if ($namedParameters[$key] !== '') {
continue;
} elseif (! empty($parameters)) {
$namedParameters[$key] = $extraParameters ? array_shift($parameters) : array_pop($parameters);
} elseif (isset($this->defaultParameters[$defaultParameterKey])) {
$namedParameters[$key] = array_shift($parameters);
}
}

// Fill leftmost parameters with defaults if the loop above was offset...
foreach ($namedParameters as $key => $value) {
$bindingField = $route->bindingFieldFor($key);
$defaultParameterKey = $bindingField ? "$key:$bindingField" : $key;

if ($value === '' && isset($this->defaultParameters[$defaultParameterKey])) {
$namedParameters[$key] = $this->defaultParameters[$defaultParameterKey];
}
}
Expand Down
161 changes: 161 additions & 0 deletions tests/Routing/RoutingUrlGeneratorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1821,6 +1821,167 @@ public function testDefaultsCanBeCombinedWithExtraQueryParameters()
$url->route('tenantPostUser', ['concretePost', 'extra' => 'query']),
);
}

public function testUrlGenerationWithOptionalParameters(): void
{
$url = new UrlGenerator(
$routes = new RouteCollection,
Request::create('https://www.foo.com/')
);

$url->defaults([
'tenant' => 'defaultTenant',
'user' => 'defaultUser',
]);

/**
* Route with one required parameter and one optional parameter.
*/
$route = new Route(['GET'], 'postOptionalMethod/{post}/{method?}', ['as' => 'postOptionalMethod', fn () => '']);
$routes->add($route);

$this->assertSame(
'https://www.foo.com/postOptionalMethod/1',
$url->route('postOptionalMethod', 1),
);

$this->assertSame(
'https://www.foo.com/postOptionalMethod/1/2',
$url->route('postOptionalMethod', [1, 2]),
);

/**
* Route with two optional parameters.
*/
$route = new Route(['GET'], 'optionalPostOptionalMethod/{post}/{method?}', ['as' => 'optionalPostOptionalMethod', fn () => '']);
$routes->add($route);

$this->assertSame(
'https://www.foo.com/optionalPostOptionalMethod/1',
$url->route('optionalPostOptionalMethod', 1),
);

$this->assertSame(
'https://www.foo.com/optionalPostOptionalMethod/1/2',
$url->route('optionalPostOptionalMethod', [1, 2]),
);

/**
* Route with one default parameter, one required parameter, and one optional parameter.
*/
$route = new Route(['GET'], 'tenantPostOptionalMethod/{tenant}/{post}/{method?}', ['as' => 'tenantPostOptionalMethod', fn () => '']);
$routes->add($route);

// Passing one parameter
$this->assertSame(
'https://www.foo.com/tenantPostOptionalMethod/defaultTenant/concretePost',
$url->route('tenantPostOptionalMethod', ['concretePost']),
);

// Passing two parameters: optional parameter is prioritized over parameter with a default value
$this->assertSame(
'https://www.foo.com/tenantPostOptionalMethod/defaultTenant/concretePost/concreteMethod',
$url->route('tenantPostOptionalMethod', ['concretePost', 'concreteMethod']),
);

// Passing all three parameters
$this->assertSame(
'https://www.foo.com/tenantPostOptionalMethod/concreteTenant/concretePost/concreteMethod',
$url->route('tenantPostOptionalMethod', ['concreteTenant', 'concretePost', 'concreteMethod']),
);

/**
* Route with two default parameters, one required parameter, and one optional parameter.
*/
$route = new Route(['GET'], 'tenantUserPostOptionalMethod/{tenant}/{user}/{post}/{method?}', ['as' => 'tenantUserPostOptionalMethod', fn () => '']);
$routes->add($route);

// Passing one parameter
$this->assertSame(
'https://www.foo.com/tenantUserPostOptionalMethod/defaultTenant/defaultUser/concretePost',
$url->route('tenantUserPostOptionalMethod', ['concretePost']),
);

// Passing two parameters: optional parameter is prioritized over parameters with default values
$this->assertSame(
'https://www.foo.com/tenantUserPostOptionalMethod/defaultTenant/defaultUser/concretePost/concreteMethod',
$url->route('tenantUserPostOptionalMethod', ['concretePost', 'concreteMethod']),
);

// Passing three parameters: only the leftmost parameter with a default value uses its default value
$this->assertSame(
'https://www.foo.com/tenantUserPostOptionalMethod/defaultTenant/concreteUser/concretePost/concreteMethod',
$url->route('tenantUserPostOptionalMethod', ['concreteUser', 'concretePost', 'concreteMethod']),
);

// Same as the assertion above, but using some named parameters
$this->assertSame(
'https://www.foo.com/tenantUserPostOptionalMethod/defaultTenant/concreteUser/concretePost/concreteMethod',
$url->route('tenantUserPostOptionalMethod', ['user' => 'concreteUser', 'concretePost', 'concreteMethod']),
);

// Also using a named parameter, but this time for the post parameter
$this->assertSame(
'https://www.foo.com/tenantUserPostOptionalMethod/defaultTenant/concreteUser/concretePost/concreteMethod',
$url->route('tenantUserPostOptionalMethod', ['concreteUser', 'post' => 'concretePost', 'concreteMethod']),
);

// Also using a named parameter, but this time for the optional method parameter
$this->assertSame(
'https://www.foo.com/tenantUserPostOptionalMethod/defaultTenant/concreteUser/concretePost/concreteMethod',
$url->route('tenantUserPostOptionalMethod', ['concreteUser', 'concretePost', 'method' => 'concreteMethod']),
);

// Passing all four parameters
$this->assertSame(
'https://www.foo.com/tenantUserPostOptionalMethod/concreteTenant/concreteUser/concretePost/concreteMethod',
$url->route('tenantUserPostOptionalMethod', ['concreteTenant', 'concreteUser', 'concretePost', 'concreteMethod']),
);

/**
* Route with a default parameter, a required parameter, another default parameter, and finally an optional parameter.
*
* This tests interleaved default parameters when combined with optional parameters.
*/
$route = new Route(['GET'], 'tenantPostUserOptionalMethod/{tenant}/{post}/{user}/{method?}', ['as' => 'tenantPostUserOptionalMethod', fn () => '']);
$routes->add($route);

// Passing one parameter
$this->assertSame(
'https://www.foo.com/tenantPostUserOptionalMethod/defaultTenant/concretePost/defaultUser',
$url->route('tenantPostUserOptionalMethod', ['concretePost']),
);

// Passing two parameters: optional parameter is prioritized over parameters with default values
$this->assertSame(
'https://www.foo.com/tenantPostUserOptionalMethod/defaultTenant/concretePost/defaultUser/concreteMethod',
$url->route('tenantPostUserOptionalMethod', ['concretePost', 'concreteMethod']),
);

// Same as the assertion above, but using some named parameters
$this->assertSame(
'https://www.foo.com/tenantPostUserOptionalMethod/defaultTenant/concretePost/defaultUser/concreteMethod',
$url->route('tenantPostUserOptionalMethod', ['post' => 'concretePost', 'concreteMethod']),
);

// Also using a named parameter, but this time for the optional parameter
$this->assertSame(
'https://www.foo.com/tenantPostUserOptionalMethod/defaultTenant/concretePost/defaultUser/concreteMethod',
$url->route('tenantPostUserOptionalMethod', ['concretePost', 'method' => 'concreteMethod']),
);

// Passing three parameters: only the leftmost parameter with a default value uses its default value
$this->assertSame(
'https://www.foo.com/tenantPostUserOptionalMethod/defaultTenant/concretePost/concreteUser/concreteMethod',
$url->route('tenantPostUserOptionalMethod', ['concretePost', 'concreteUser', 'concreteMethod']),
);

// Passing all four parameters
$this->assertSame(
'https://www.foo.com/tenantPostUserOptionalMethod/concreteTenant/concretePost/concreteUser/concreteMethod',
$url->route('tenantPostUserOptionalMethod', ['concreteTenant', 'concretePost', 'concreteUser', 'concreteMethod']),
);
}
}

class RoutableInterfaceStub implements UrlRoutable
Expand Down
Loading