URL::defaults() are ignored in UrlGenerator::toRoute()

[stancl]

Laravel Version

12

PHP Version

any

Database Driver & Version

irrelevant

Description

Given:

• A route like Route::get('/{foo:slug}/example/{bar}', ...)->name('example');
• URL::defaults(['foo' => 'some_value']);

route('example', $bar) will produce a "Missing required parameter" exception.

On the other hand, route('example', ['bar' => $bar]) will work fine.

The issue comes from UrlGenerator::toRoute():

public function toRoute($route, $parameters, $absolute)
{
    // $parameters = [$bar]
    $parameters = Collection::wrap($parameters)->map(function ($value, $key) use ($route) {
        // $value = $bar, $key = 0
        //     true since it's a model       && true because field '0' (foo:slug) has a binding field (slug)
        return $value instanceof UrlRoutable && $route->bindingFieldFor($key)
                ? $value->{$route->bindingFieldFor($key)}
                : $value;
    })->all();

    // $parameters = [null because we've incorrectly done $bar->slug]

    array_walk_recursive($parameters, function (&$item) {
        if ($item instanceof BackedEnum) {
            $item = $item->value;
        }
    });

    return $this->routeUrl()->to(
        $route, $this->formatParameters($parameters), $absolute
    );
}

The default parameter only gets used in $this->routeUrl() (RouteUrlGenerator), but by that point the UrlGenerator has broken the provided parameters, passing a [null] array.

I think the solution would be something along the lines of: Actually probably a bit more complex than this because defaults can work for latter parameters too, not just the earliest ones. I'll try to send a PR with some reasonable implementation.

• If $key is numeric, aka we've passed route('example', $bar) not route('example', ['bar' => $bar])
• And array_key_exists($route->parameterNames()[$key], $this->getDefaultParameters())
• Skip the [$key] parameter and move on to the next one
• (Repeat as many times as needed — in my example there's one parameter with a default value but there can be several)

So in this case $bar should be matched against the second parameter, not the first one.

Steps To Reproduce

See above

[github-actions]

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

[stancl]

I'm going over some hypothetical scenarios to see what'd be the ideal implementation here and this makes the most sense to me:

• If count($route->parameterNames()) == count($parameters), use positional logic like the current implementation
• If named parameters are passed, obviously use the names. Possible edge case with mixed arrays, not sure how those are handled now*

In the following examples * denotes a default value set. I use binding fields for all params since it simplifies the examples:

• {tenant:slug*}/{user:slug} basic scenario
  • route(x, $param1, $param2) provides all params so it's just route(x, $tenant, $user)
  • route(x, $param) should be interpreted as route(x, $user) since the other field can be filled using defaults
• {tenant:slug*}/{team:slug*}/{user:slug}
  • route(x, $param1, $param2, $param3) provides all params, so it's just `route(x, $tenant, $team, $user)
  • route(x, $param) should be interpreted as route(x, $user) since all other fields can be filled using defaults
  • route(x, $param1, $param2) should be interpreted as route(x, $team, $user). Meaning we apply defaults to as many leftmost parameters as needed to suffice with the passed parameter values
• {post:slug}/comments/{user:slug*} (realistically URL::defaults() is only used for "prefix params" but this is supported too)
  • route(x, $param1, $param2) specifies both so it's just route(x, $post, $user)
  • route(x, $param) should be interpreted as route(x, $post)since{user}` has a default value
• {team:slug*}/{post:slug}/{user:slug*}
  • route(x, $param1, $param2, $param3) all 3 specified
  • route(x, $param) = route(x, $post) since others have defaults
  • route(x, $param1, $param2) should probably be route(x, $post, $user) following the idea that given multiple defaults we prefer to apply them to the earlier params and use these provided values for latter params

Which would look something like:

• $route->getBindingFields() returns ['param' => 'column', ...] (keys are just $route->parameterNames() *that have binding fields set)
• The array keys presumably follow $route->parameterNames() order, but to make sure we use the right order we can iterate over $route->parameterNames() and index into the binding fields
• Keep removing leftmost binding fields that have a default value set ($this->getDefaultParameters()) until we're left with count($parameters)
• Run the existing logic but using $route->bindingFieldFor($parameterName) instead of a numeric key argument

* Assuming mixed arrays are supported, we should probably treat named params similarly to defaults in how we remove them from the list used for transforming positional parameters. However unlike defaults, named params also get transformed and included in the final array passed to the RouteUrlGenerator.

There may be an elegant way to implement this that I'm not seeing, but I think unfortunately the fix will make the method a lot uglier.

[stancl]

While trying to reproduce this in laravel/framework tests, I ran into some seemingly different issues, likely related to the objects used in those tests, will need to investigate further. However that made me try to reproduce this in just a fresh Laravel app and while testing that I thought that even the behavior I'm trying to fix here wouldn't actually be correct.

If we define URL::defaults(['team' => $team->id]) and have a route with {team:slug}, it shouldn't be trying to put the ID there, which it currently is.

I think URL::defaults() should support binding field syntax too:

• URL::defaults(['team' => $team->getRouteKey()])
• URL::defaults(['team:slug' => $team->slug])

That would make defaults actually correctly usable with binding fields. @crynobone can I get some feedback on this?

The current behavior is broken in several ways, I'm trying to fix one of those broken things, but taking a step back it wouldn't make sense anyway so I think first I'd need to know if you'd like the URL::defaults(['team:slug' => $team->slug]) addition. Then I could implement it along with the handling of "partial parameters" as described in the previous comment, if you agree with that description of how the URL generator should behave.

[stancl]

Another case:

Route::get('/test/{foo}/{user}', fn () => 'foo')->name('abc');
URL::defaults(['foo' => 'bar']);

$user = User::first();

// http://foo.com/test/bar/bar?1
dd(route('abc', ['bar', $user]));

// http://foo.com/test/bar/1
dd(route('abc', ['foo' => 'bar', 'user' => $user]));

Here URL::defaults() have a higher precedence than the actual passed arguments. Passing the parameters using actual names fixes this, but I think that shouldn't be necessary.

If the URL::defaults() line is commented out, the first dd() shows the correct route — we've passed parameters positionally and they got used as passed. Defaults shouldn't override this, the current behavior is inconsistent.

I think the handling of defaults should be rewritten overall a bit. Luckily it's not too much logic so I think this should be feasible without any real breaking changes.