yiisoft / router-fastroute

Yii Router FastRoute adapter
https://www.yiiframework.com/
BSD 3-Clause "New" or "Revised" License
39 stars 19 forks source link

host() in routes with schema and/or port does not work #151

Open Teran123 opened 3 months ago

Teran123 commented 3 months ago

https://github.com/yiisoft/router-fastroute/blob/09166194b86d011116763692d0c2c8319c863993/src/UrlMatcher.php#L109

What steps will reproduce the problem?

Specify host() with a schema and/or port in routes As example:

Group::create("/{_language}")
        ->routes(
            Route::get("/")
                ->action([SiteController::class, 'index'])
                ->name("site.index"),
        )
        ->host("http://localhost:8080") // doesn't work
or
        ->host("http://localhost") // doesn't work
or
        ->host("localhost:8080") // doesn't work
or
        ->host("localhost") // works fine, but only if the site is on port 80

What is the expected result?

When I go to the url http://localhost:8080 in my browser, I will see the page rendered by SiteController::index

What do you get instead?

404 error

Additional info

The problem spot is in the UrlMatcher class, when calling dispatch() on line 107: https://github.com/yiisoft/router-fastroute/blob/master/src/UrlMatcher.php#L107 Only host and path are specified in the $uri parameter. But schema and port are returned in getUri() separately from host.

So, for example, if this dispatch() call looked something like this, there wouldn't be a problem:

$uri = $request->getUri();
$scheme = $uri->getScheme();
$host = $uri->getHost();
$port = $uri->getPort() ? ":{$uri->getPort()}" : "";
$result = $this
    ->getDispatcher($dispatchData)
    ->dispatch($method, "{$scheme}://{$host}{$port}{$path}");
Q A
Version ^3.0
PHP version 8.3.9 / 8.2.20
Operating system Linux
samdark commented 3 months ago

Thanks for reporting. Do you want to work on pull request w/ tests and the fix?

Teran123 commented 3 months ago

Thanks for reporting. Do you want to work on pull request w/ tests and the fix?

I don't write tests, so I don't)

I also want to add that my example solution is just an example, and the problem here is probably getHost() which returns the host not in the same form as I specified it in the routes - your team knows better :)

Here's another example: If I want to use UrlGenerator to generate the absolute url of the route site.index from my example - I will be forced to re-specify the host in the form I have specified it in the routes. I.e. like this (if without schema):

$uri = $request->getUri();
$host = $uri->getHost();
$port = $uri->getPort() ? ":{$uri->getPort()}" : "";
$urlGenerator->generateAbsolute(name: "site.index", host: "{$host}{$port}");

Instead, I would like to be able to generate an absolute url as follows:

$urlGenerator->generateAbsolute("site.index")

Or at least something like this:

$urlGenerator->generateAbsolute(name: "site.index", host: $uri->getHost())

or

$urlGenerator->generateAbsolute(name: "site.index", host: $currentRoute->getHost())
ev-gor commented 1 day ago

I don't think it's a bug. According to RFC 3986, a host is a part of a URI located between a scheme and a port. Therefore, this host method expects only a host (localhost in this case). However, you provided not just a host, but also a scheme and a port. That's why there is no match. This behavior is expected, in my opinion. Perhaps it should be specifically emphasized in the documentation.