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

[Teran123]

router-fastroute/src/UrlMatcher.php

Line 109 in 0916619

->getHost() . $path);

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]

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

[Teran123]

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]

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.

[vjik]

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.