Bug: routing option ['subdomain' => '*'] doesn't match www

[webalchemist]

PHP Version

8.2

CodeIgniter4 Version

4.4.3

CodeIgniter4 Installation Method

Composer (as dependency to an existing project)

Which operating systems have you tested for this bug?

Windows

Which server did you use?

apache

Database

MariaDB 10

What happened?

I have a site with a wildcard subdomain, where I use for the subdomains for localization. I was having difficulties getting the subdomains to work as they don't match the base_url. I saw a post suggesting using an option in the routing to allow any domain to match ['subdomain => '*'].

If you try to set the option ['subdomain => '*'] on a route, it works for all subdomains except www. There it generates an error saying it could not find a route for the path.

I looked into the code and found the following at line 1649 in RouteCollection.php

 if ($host[0] === 'www') {
     unset($host[0]);
 }

This explicitly sets a special case for www and removes it as the subdomain and I can't see why!
If I comment these lines out it works as I would expect.

I assume that this code was added for some use-case where there is an extra www in front of the 'proper' subdomain.
However, since www may be the actual subdomain, this is a bug. ['subdomain => '*'] should match ANY subdomain, not any subdomain except www.

Steps to Reproduce

Setup a route as follows:

$routes->get ('/', 'PublicPages::index', ['subdomain' => '*'] );

try accessing the page as
https://sub.mydomain.com/ - loads the correct page

try accessing the page as
https://www.mydomain.com/ - 404 Can't find a route for 'get: /'.

Expected Output

Normal page loading, not a 404 Can't find a route for 'get: /'.

Anything else?

No response

[webalchemist]

Looking at the code this would also fail for ['subdomain' => 'www'] since the 'www' is removed from the host array before any comparison with the subdomain option value.

[kenjis]

Thank you for reporting.

The current subdomain determination is quite lax.
Also, I'm not sure what a subdomain means there in the first place.

Important
The system is not perfect and should be tested for your specific domain before being used in production. Most domains should work fine but some edge case ones, especially with a period in the domain itself (not used to separate suffixes or www) can potentially lead to false positives.
https://codeigniter4.github.io/CodeIgniter4/incoming/routing.html#limit-to-subdomains

I assume that this code was added for some use-case where there is an extra www in front of the 'proper' subdomain.

The code was there from the beginning:
13259443c8d#diff-7fc8cb2cff68c2c83b13a33474230889ec109fffda6fc0a732910a76c5ad8ec7R835

I'm not sure, but something like this?
www.example.com: the main server, not subdomain
foo.example.com: subdomain

I believe this hostname/subdomain features (and the routing data structure) should be rewritten.
There is also issue #7214.

If you can try, please send a PR.
https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md

[kenjis]

As a workaround, I recommend you extend RouteCollection.
https://codeigniter4.github.io/CodeIgniter4/extending/core_classes.html

[webalchemist]

It doesn't appear to be that simple.

1, Although the example you linked (very conveniently) shows how to replace the RouteCollection class. The docs are out of date and show the wrong parameter list for the constructor.

2, The functions I needed to change are declared private (rather than protected) so I can't simply extend RouteCollection and replace one function. If there's no particular reason to set something as private they should be protected, especially when you're clearly expecting people to extend the base classes. This should be fixed.

3, I tried to make a new RouteCollection by implementing the RouteCollectionInterface as in the docs. However, when I tried this I got type mismatch errors. This is because at various points variables/parameters have been coded with type 'RouteCollection' rather than 'RouteCollectionInterface'.
e.g., CodeIgniter\Routes\DefinedRouteCollector, CodeIgniter\Shield\Auth
It rather defeats the point of setting-up the interfaces if they're not used polymorphically. This should be fixed.

I guess the easiest thing to do will be to extend RouteCollection but by replacing a larger chunk of it until I get back to a public or protected function that I can override.

[kenjis]

If you have a reason why it should be protected, please send a PR.
We accept it if it is reasonable.

Sorry, some interfaces are broken. They don't have some methods that are used in the framework.
But changing interfaces is a breaking change. So we can't do easily. See #4356, #6814

[webalchemist]

With regard to setting functions to protected. I just believe that in classes you expect to be extended the default for non-public functions should be protected not private. It just gives more flexibility in extending the classes.
In the current situation I've had to copy 5 functions across from the original version of RouteCollection in order to override a protected function rather than just the 1 private function I needed to change. This exposes me to more work merging changes during future updates. Also, changing functions from private to protected is a non-breaking change. I won't raise a PR as its more of a comment on general coding policy rather than anything specific to this one class.

I have a working solution to my original problem, so I'll close this issue here.

[kenjis]

I don't want people to extend core class personally.
I think we should use interfaces, but some of them are broken...

We use private as the default visibility. Because the private method in core classes was a part of a public method originally, and we do not intend to change the API of the class.
If there is a clear reason to make it protected, we consider it.
Frankly, CI4's APIs are not mature or stable, we need to change them for fixing bugs, but if it is protected, we cannot change it easily because it will be a breaking change.