-
Notifications
You must be signed in to change notification settings - Fork 7.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Creation of dynamic property CI_URI::$config is deprecated #6192
Comments
|
@t1xart SSL is broken on that page and Cloudflare firewall says it is dangerous and blocks my access 🤷 |
I have a CI3 site that's been running production on a PHP 7 server. PHP 7 is now EOL so we are migrating the server to PHP 8. It seems to work for 8.0 and 8.1, but in 8.2 dynamic property assignment has been deprecated so a single page load results in tons of warnings. I believe this problem still persists in the very latest CI 3.1.13. It looks like there's a fairly small number of classes to which CI3 attempts to assign dynamic properties so perhaps this can be quickly fixed to make CI3 compatible with PHP 8.2? I see that supermod InsiteFX posted the ci forum with a semi-helpful post. If I understand correctly, it could be fixed by implementing // this is an attribute
#[\AllowDynamicProperties]
class Post
{
}
$post = new Post();
$post->name = 'Name'; // All fine Each warning throws an exception with its call stack but that's a LOT of text so I've just included the messages below. There are a lot of messages but it's only referring to about six classes here. It would be helpful to have a complete list of all classes to which CI3 attempts to assign dynamic properties.
|
With the next changes the welcome page appears without errrors. /system/core/URI.php class CI_URI {
/system/core/Router.php class CI_Router {
/system/core/Loader.php class CI_Loader {
/system/core/Controller.php class CI_Controller {
If this is a bad idea, please let me know. Ubel Jan |
A few additions after asking a question to the database and showing the answer. I give you all my adjustments. /system/core/URI.php class CI_URI {
/system/core/Router.php class CI_Router {
/system/core/Loader.php class CI_Loader {
/system/core/Controller.php class CI_Controller {
/system/core/DB_driver.php abstract class CI_DB_driver {
Greetings, |
@ubeljan it's good to see what you've suggested here, but I'm not sure adding some collection of public properties to these classes is going to solve the problem in the general sense. I vaguely recall that CI3 assigns a LOT of dynamic properties to the controller. I can think of two other ways that we might fix this, but would like to hear from the CI devs and/or community. Approach 1: __get and __set methods protected $dynamic_properties = [];
function __set($prop, $val) {
$this->dynamic_properties[$prop] = $val;
}
function __get($prop) {
if (array_key_exists($prop, $this->dynamic_properties)) {
return $this->dynamic_properties[$prop];
} else {
throw new Exception("Property $prop does not exist");
}
} Approach 2: AllowDynamicProperties attribute #[\AllowDynamicProperties]
class ClassAllowsDynamicProperties {
// blah blah blah
}
$foo = new ClassAllowsDynamicProperties();
$foo->some_new_property = 42; To be clear, I have often lamented that CI uses undeclared properties so frequently -- IDEs don't know what kind of object you might be referring to, so all the code hinting features are disabled -- I just think one of these two approaches would be a more comprehensive and reasonable solution. |
For now I think the shortest way is approach 2. /system/core/URI.php
/system/core/Router.php
/system/core/Loader.php
/system/core/Controller.php
/system/core/DB_driver.php
|
This is helpful information! I wonder if perhaps @narfbg might sound off on our suggested solution here, and let us know what other classes have dynamic properties? |
The described problem in the first post of this issue is already fixed by #6173... It's just in the first PRs if you open the Pull Requests page.
Exactly what's happening on #6173.
That's why I waited for more incoming issues and pull requests on this matter. And thanks for your list of them. I compared with what we already in the previously mentioned PR and we covered all the cases.
Covered #6173.
Covered.
Covered.
Covered.
Covered.
Once again, please follow up on #6173... If you find other places, just point them out on that PR. |
A stackoverflow question a user had sent me on the path of discovering why this was happening and noticed it wasn't patched in any branch or tag yet.
https://github.com/bcit-ci/CodeIgniter/blob/3.1.13/system/core/URI.php#L102
The text was updated successfully, but these errors were encountered: