router-fastroute icon indicating copy to clipboard operation
router-fastroute copied to clipboard

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

Open Teran123 opened this issue 1 year ago • 4 comments

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

Teran123 avatar Jul 17 '24 23:07 Teran123

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

samdark avatar Jul 18 '24 12:07 samdark

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())

Teran123 avatar Jul 18 '24 20:07 Teran123

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.

ev-gor avatar Nov 02 '24 13:11 ev-gor

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.

No. Url generation works correct:

Route::get('/home/index')->name('index')->host('http://test.com:8080');

// Result: http://test.com:8080/home/index
$generator->generateAbsolute('index');

Problem with URL matching only.


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.

Good point. We should to decide whether to allow Route to have host, port and scheme in "host" property or host only.

vjik avatar Nov 07 '24 18:11 vjik