Bug: current_url() returning wrong value when different host

[darkylmnx]

PHP Version

8.0

CodeIgniter4 Version

4.1.9

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Windows

Which server did you use?

apache

Database

No response

What happened?

The function current_url() returns the $baseURL in app/Config/App.php instead of the real current url in the browser.
This leads to confusion, know the documentation says:

You can restrict groups of routes to function only in certain domain or sub-domains of your application by passing the “hostname” option along with the desired domain to allow it on as part of the options array:

https://codeigniter.com/user_guide/incoming/routing.html?highlight=router#limit-to-hostname

Which means an app could have multiple domain pointing at it, but current_url() would only return the one set in config.

Use case: CMS, multi-tenant app, multi-vendor app, etc.

Steps to Reproduce

Call current_url() when loading the app with a host like sub.example.com while in the config, you would set $baseURL to www.example.com for example.

Expected Output

current_url() is the name implies should retour the current url and not the current url with $baseURL as host.

Anything else?

No response

[kenjis]

All of the *_url() functions are intentionally based on the Config (i.e. baseURL, indexPage, forceSecureRequests) and not intended to match the browser URL.
#3004 (comment)

[kenjis]

At least current_url() assumes the$baseURL is correct.
It makes sense.

But the problem is there is no way to set the correct value to $baseURL
when you have more than one hostname.

[kenjis]

@darkylmnx
You need to set the correct $baseURL.
But be careful not to create security holes, please.
See

• https://expressionengine.com/blog/http-host-and-server-name-security-issues
• https://www.codeigniter.com/userguide3/installation/upgrade_303.html#step-2-make-sure-your-base-url-config-value-is-not-empty

You could set it in the constructor something like this:

<?php

namespace Config;

use CodeIgniter\Config\BaseConfig;

class App extends BaseConfig
{
    // ...

    public function __construct()
    {
        parent::__construct();

        $allowed_domains = ['domain1.tld', 'domain2.tld'];
        if (in_array($_SERVER['HTTP_HOST'], $allowed_domains, true)) {
             $this->baseURL = 'https://' . $_SERVER['HTTP_HOST'] . '/';
        }
    }

[darkylmnx]

It would make sens it CodeIgniter was designed to be mono-domained which it isn't anymore (because I guess on version 2 it was back then).

As said above, the current docs says you can constrain by hosts when setting up your routes, so it really confuses me that all *_url function act in the opposite way of what the router gives the ability to do.
https://codeigniter.com/user_guide/incoming/routing.html?highlight=router#limit-to-hostname

I think there should at least be an alternative helper to do that.
As for:

As you have noted, detection is pretty complicated and not guaranteed

It seems Laravel & Symfony handles it properly, is there something i'm missing here?

[kenjis]

Yes, it is very confusing.
At least I will fix the documentation about current_url(). See #5808

If there is something confusing, tell us. I will fix the docs.

Laravel & Symfony handles it properly

I don't know they really handle properly. Did you checked the implementation?
And how Symfony handles multiple domains?

[darkylmnx]

@kenjis thanks for the docs.
I did check because I use Laravel in another project. Basically it uses Symfony's Request class and narrows down to the methods getPathInfo & getHost around those 4 places:

• https://github.com/symfony/symfony/blob/5e4151fd88260adeac7de0425397b720233c31a2/src/Symfony/Component/HttpFoundation/Request.php#L1681

• https://github.com/symfony/symfony/blob/5e4151fd88260adeac7de0425397b720233c31a2/src/Symfony/Component/HttpFoundation/Request.php#L1114

• https://github.com/symfony/symfony/blob/5e4151fd88260adeac7de0425397b720233c31a2/src/Symfony/Component/HttpFoundation/Request.php#L854

• https://github.com/symfony/symfony/blob/5e4151fd88260adeac7de0425397b720233c31a2/src/Symfony/Component/HttpFoundation/Request.php#L1002

In my usage, I never had any problem with Laravel's url()->current() or url()->full() methods which are based on the above.

From what I read and understand in the Symfony code (as this is way above what I do in PHP), it seems to handle most cases, even edge cases like PHP loaded by CGI and thigns like that.

It's way better than what I did to handle this myself in CI 4 currently, since I'm building a multi-vendor app where each users can connect his/her custom domain, and therefore, creating an allow-list and remplacing $this->baseURL like you did in the example doesn't work.

I could indeed check the DB instead of doing an allow-list and if a match is found replace it, but in any case, I would have other types of issues in my specific case.

I actually had to create a method that does this:

public function getCurrentUrl() {
        $protocol = isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] === 'on' ? 'https' : 'http';
        $path = "://$_SERVER[HTTP_HOST]$_SERVER[REQUEST_URI]";

        return $protocol . $path;
    }

But I'd prefer the framework to have a built-in-more-secure-[insert anything]-way of doing this.
I could load Symfony's standalone Request class for that, but I would loose the whole point of using CI4, speed and simplicity.

[kenjis]

Thank you for your investigation.

It seems Symfony uses $trustedHostPatterns to check the correct hostname.
How do you set the values in Lavarel?

I could indeed check the DB instead of doing an allow-list and if a match is found replace it, but in any case, I would have other types of issues in my specific case.

What do you mean? If you don't check the hostname, attackers could easily attack your app by host header injection.

[darkylmnx]

It seems Symfony uses $trustedHostPatterns to check the correct hostname. How do you set the values in Lavarel?

What do you mean? If you don't check the hostname, attackers could easily attack your app by host header injection.

I never need to set it since I will then have a middleware or whatsoever that will check if the the host of the current url exists in my DB or some data source.

What I meant was, I would not load all my domains and then do a in_array that would be way too bloated in the futur.

That being said, the fundamental issue was to not have a valid current url I can do my check on.

[kenjis]

What I meant was, I would not load all my domains and then do a in_array that would be way too bloated in the futur.

Okay, the code using in_array() is just a sample.

[darkylmnx]

Okay, the code using in_array() is just a sample.

Yes, of course. I was just saying thing was to be able to retreive the current url safely.

[kenjis]

That being said, the fundamental issue was to not have a valid current url I can do my check on.

It seems CI4 does not support multiple domain environment completely,
or there is a design flaw.

$baseURL is a predefined static value, and CI4 relies on it.

We cannot set $baseURL with using DB data, because the Config\App is created
in very early stage of the framework bootstrap.

What I mean is this needs a big change, not just adding a new function.