codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.35k stars 1.9k forks source link

Bug: env var not read from system anymore since version update v.4.1.2 #4768

Closed sfox-developer closed 3 years ago

sfox-developer commented 3 years ago

Describe the bug After updating to version 4.1.2 the deployment failed due to missing baseURL var.

Type:        InvalidArgumentException
Message:     _get_uri() requires a valid baseURL.
Filename:    */vendor/codeigniter4/framework/system/Helpers/url_helper.php
Line Number: 43

CodeIgniter 4 version 4.1.2

Affected module(s) This is not yet clear.

Expected behavior, and steps to reproduce if appropriate We are using heroku for deploying a ci4 project. As far as i know Heroku does not write a .env file into the root folder, but writes the vars into the systems env vars. So here then defining baseURL worked until the latest update. With the latest update it does not read the baseURL var from the system anymore causing the issue mentioned on top.

Context

MGatner commented 3 years ago

My only guess is maybe it was this? https://github.com/codeigniter4/CodeIgniter4/pull/4561/files

Are you using app.baseURL for the key? if not, try that. Otherwise, maybe share the whole trace stack? I'm curious if somehow it is trying to load App before server variables have been propagated.

sfox-developer commented 3 years ago

I usually only have to set baseURL. But i tried to set app.baseURL and baseURL into the heroku settings without any luck.

Issue comes within the "post-install-cmd". defined in my composer.json like this

"scripts": {
    "post-update-cmd": [
         "@composer dump-autoload"
    ],
    "post-install-cmd": [
            "php spark assets:publish",
        "php spark migrate"
        ],
    "test": "phpunit"
},

The package used is "tatter/assets": "^2.2", here for the spark assets:publish cmd.

Terminal

...
Published FontAwesome.json
Published Select2.json
Published Swiper.json
Published jQuery.json
6 manifests published.
An uncaught Exception was encountered

Type:        InvalidArgumentException
Message:     _get_uri() requires a valid baseURL.
Filename:    /Users/***/Sites/***/***/vendor/codeigniter4/framework/system/Helpers/url_helper.php
Line Number: 43
Script php spark assets:publish handling the post-install-cmd event returned with error code 9

Log

CRITICAL - 2021-06-02 13:49:19 --> _get_uri() requires a valid baseURL.
#0 /Users/***/Sites/***/***/vendor/codeigniter4/framework/system/Helpers/url_helper.php(163): _get_uri('')
#1 /Users/***/Sites/***/***/vendor/codeigniter4/framework/system/CodeIgniter.php(477): current_url(true)
#2 /Users/***/Sites/***/***/vendor/codeigniter4/framework/system/CodeIgniter.php(339): CodeIgniter\CodeIgniter->handleRequest(NULL, Object(Config\Cache), false)
#3 /Users/***/Sites/***/***/vendor/codeigniter4/framework/system/CLI/Console.php(61): CodeIgniter\CodeIgniter->run()
#4 /Users/***/Sites/***/***/spark(57): CodeIgniter\CLI\Console->run()
#5 {main}
MGatner commented 3 years ago

6 manifests published.

That's the end of the Assets publish script, so I don't believe it is related.

It's very odd that your first spark command succeeds but not the second one. I cannot dig on this any more right now but maybe try setting $baseURL in app/Config/App.php to something benign (like http://localhost) and see if that fixes the CLI yet still uses your env setting for HTTP?

Also N.B. you can remove this, Composer handles it automatically now:

"post-update-cmd": [
         "@composer dump-autoload"
    ],
sfox-developer commented 3 years ago

It's very odd that your first spark command succeeds but not the second one. php spark migrate does not run before, php spark assets:publish is first.

Defining anything inside app/Config/App.php (http://localhost) worked fine here. Deployment runs fine now and page still looks good and does take the correct var.

Thanks for the tip!

But no clue what happens here.

That's the end of the Assets publish script, so I don't believe it is related.

Thought because of the line Script php spark assets:publish handling the post-install-cmd event returned with error code 9

MGatner commented 3 years ago

Yay! Glad it worked. Please leave this issue open though, since that was definitely a bug that needs addressing.

Thought because of the line

You're totally right, I missed that. It runs the entire command and then proceeds to handleRequest() and crashes. Definitely something fishy there.

sfox-developer commented 3 years ago

Alright, let me know if you need anything to reproduce the issue.

Thanks for your help.

paulbalandan commented 3 years ago

@MGatner do you think this is due to the (string) cast on the URI object returned by current_url(true)? This is the diff of the change between v4.1.2...HEAD in system/CodeIgniter.php:

-$this->storePreviousURL((string) current_url(true));
+$this->storePreviousURL(current_url(true));

If I recall, string casting is problematic for URI?

MGatner commented 3 years ago

String casts for URI are only a problem when it might be not be a project URI. In this case we're guaranteed it is this should be safe.

paulbalandan commented 3 years ago

My best guess is due in part of #4561 disallowing non-namespaced calls to env vars and Heroku's policy of enforcing alphanum and underscore for its env vars' keys. Since @sfox-developer has said he's setting only baseURL as config var, app.baseURL won't be populated.

In this case, I think we can use our own env function to retrieve directly the env var. @sfox-developer can you try change app/Config/App.php and change $baseURL to env('baseURL', http://localhost);. Then in heroku dashboard, set baseURL to any URL other than localhost to see if it can retrieve the correct base url.

sfox-developer commented 3 years ago

public $baseURL = env('baseURL', 'http://localhost');

@paulbalandan this leads to a

PHP Fatal error: Constant expression contains invalid operations in /tmp/build_455805ae/app/Config/App.php on line 24

sfox-developer commented 3 years ago

Since @sfox-developer has said he's setting only baseURL as config var, app.baseURL won't be populated.

I inserted both vars recently just to be sure (before 4.1.2 i was able to use baseURL only):

I tried to remove app.baseURL right now which leads to having localhost as baseURL as defined in app/Config/App.php public $baseURL = 'http://localhost';

Having app.baseURL set in heroku, it works fine, and the correct URL is used.

paulbalandan commented 3 years ago

public $baseURL = env('baseURL', 'http://localhost');

@paulbalandan this leads to a

PHP Fatal error: Constant expression contains invalid operations in /tmp/build_455805ae/app/Config/App.php on line 24

Sorry for this dumb suggestion. I forgot scalars and arrays are only allowed there.

This seems to be a side effect of #4561 disallowing non-namespaced getenv calls (to prevent calling the PATH system variable by mistake, and others). But since we have documented this change here, I think this should be anticipated.

In order for Config** classes to get their respective properties’ values from the .env, it is now necessary to namespace the property with the name of the class. Previously, the property names are enough but now disallowed because it can get system environment variables, like PATH.