Skip to content
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

Closed
zanderwar opened this issue Jan 15, 2023 · 9 comments
Closed

Creation of dynamic property CI_URI::$config is deprecated #6192

zanderwar opened this issue Jan 15, 2023 · 9 comments

Comments

@zanderwar
Copy link

zanderwar commented Jan 15, 2023

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

@t1xart
Copy link

t1xart commented Jan 15, 2023

<unrelated, non-English web page link removed>

@zanderwar
Copy link
Author

@t1xart SSL is broken on that page and Cloudflare firewall says it is dangerous and blocks my access 🤷

@sneakyimp
Copy link

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 __get and __set methods or, alternatively, we could use a PHP Attribute for classes to which CI3 assigns dynamic properties.

// 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.

Creation of dynamic property CI_URI::$config is deprecated
Creation of dynamic property CI_Router::$uri is deprecated
Creation of dynamic property Home::$benchmark is deprecated
Creation of dynamic property Home::$hooks is deprecated
Creation of dynamic property Home::$config is deprecated
Creation of dynamic property Home::$log is deprecated
Creation of dynamic property Home::$utf8 is deprecated
Creation of dynamic property Home::$uri is deprecated
Creation of dynamic property Home::$exceptions is deprecated
Creation of dynamic property Home::$router is deprecated
Creation of dynamic property Home::$output is deprecated
Creation of dynamic property Home::$security is deprecated
Creation of dynamic property Home::$input is deprecated
Creation of dynamic property Home::$lang is deprecated
Creation of dynamic property Home::$db is deprecated
Creation of dynamic property CI_DB_mysqli_driver::$failover is deprecated
Creation of dynamic property Home::$session is deprecated
Creation of dynamic property Home::$alerts is deprecated
Creation of dynamic property CI_Cache::$file is deprecated
Creation of dynamic property Home::$cache is deprecated
Creation of dynamic property CI_Cache::$dummy is deprecated
Creation of dynamic property Home::$agent is deprecated
Creation of dynamic property CI_Loader::$load is deprecated
Creation of dynamic property CI_Loader::$language is deprecated
Creation of dynamic property CI_Loader::$view_admin_menu is deprecated
Creation of dynamic property CI_Loader::$paywall_routing is deprecated
Creation of dynamic property CI_Loader::$no_index_no_follow is deprecated
Creation of dynamic property CI_Loader::$site_version is deprecated
Creation of dynamic property CI_Loader::$benchmark is deprecated
Creation of dynamic property CI_Loader::$hooks is deprecated
Creation of dynamic property CI_Loader::$config is deprecated
Creation of dynamic property CI_Loader::$log is deprecated
Creation of dynamic property CI_Loader::$utf8 is deprecated
Creation of dynamic property CI_Loader::$uri is deprecated
Creation of dynamic property CI_Loader::$exceptions is deprecated
Creation of dynamic property CI_Loader::$router is deprecated
Creation of dynamic property CI_Loader::$output is deprecated
Creation of dynamic property CI_Loader::$security is deprecated
Creation of dynamic property CI_Loader::$input is deprecated
Creation of dynamic property CI_Loader::$lang is deprecated
Creation of dynamic property CI_Loader::$db is deprecated
Creation of dynamic property CI_Loader::$session is deprecated
Creation of dynamic property CI_Loader::$alerts is deprecated
Creation of dynamic property CI_Loader::$cache is deprecated
Creation of dynamic property CI_Loader::$agent is deprecated
Creation of dynamic property CI_Loader::$typography is deprecated
Creation of dynamic property Home::$typography is deprecated

@ubeljan
Copy link

ubeljan commented Jan 19, 2023

With the next changes the welcome page appears without errrors.
Add the public statements to the mentioned files.

/system/core/URI.php

class CI_URI {

public $config;

/system/core/Router.php

class CI_Router {

public $uri;

/system/core/Loader.php

class CI_Loader {

public $load;
public $benchmark;
public $config;
public $log;
public $hooks;
public $utf8;
public $uri;
public $router;
public $exceptions;
public $output;
public $security;
public $input;
public $lang;  

/system/core/Controller.php

class CI_Controller {

public $benchmark;    
public $config;
public $log;    
public $hooks;  
public $utf8;    
public $uri;
public $router;    
public $exceptions;    
public $output;    
public $security;
public $input;    
public $lang;  

If this is a bad idea, please let me know.
I will now import an existing project in the above environment and see what happens.
I will let you know.

Ubel Jan

@ubeljan
Copy link

ubeljan commented Jan 19, 2023

A few additions after asking a question to the database and showing the answer.

I give you all my adjustments.
The adjustments in /system/core/Controller.php are coming from my /config/autoload.php file.
The section libraries and model have to be declared in /system/core/Controller.php
I have them commented them.

/system/core/URI.php

class CI_URI {

public $config;

/system/core/Router.php

class CI_Router {

public $uri;

/system/core/Loader.php

class CI_Loader {

public $load;
public $benchmark;
public $config;
public $log;
public $hooks;
public $utf8;
public $uri;
public $router;
public $exceptions;
public $output;
public $security;
public $input;
public $lang;  

/system/core/Controller.php

class CI_Controller {

public $benchmark;    
public $config;
public $log;    
public $hooks;  
public $utf8;    
public $uri;
public $router;    
public $exceptions;    
public $output;    
public $security;
public $input;    
public $lang;  
// coming from autoload.php libraries and model
public $db;      // libraries   
public $email;   // libraries
public $WorldM;  // model      

/system/core/DB_driver.php

abstract class CI_DB_driver {

public $failover; 

Greetings,
Ubel Jan

@sneakyimp
Copy link

@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
Add the $dynamic_properties var and these methods to every class that has dynamic properties set

    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
Use the PHP AllowDynamicProperties attribute to tell PHP you'd like these classes to permit dynamic attributes:

#[\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.

@ubeljan
Copy link

ubeljan commented Jan 20, 2023

For now I think the shortest way is approach 2.
To implement #[\AllowDynamicProperties] in the classes.
For now I have seen 5 places where to add #[\AllowDynamicProperties] to get a working website.

/system/core/URI.php

#[\AllowDynamicProperties]

class CI_URI {

/system/core/Router.php

#[\AllowDynamicProperties]

class CI_Router {

/system/core/Loader.php

#[\AllowDynamicProperties]

class CI_Loader {

/system/core/Controller.php

#[\AllowDynamicProperties]

class CI_Controller {     

/system/core/DB_driver.php

#[\AllowDynamicProperties]

abstract class CI_DB_driver {

@sneakyimp
Copy link

For now I have seen 5 places where to add #[\AllowDynamicProperties] to get a working website.

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?

@gxgpet
Copy link
Contributor

gxgpet commented Jan 27, 2023

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.

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?

Exactly what's happening on #6173.

It would be helpful to have a complete list of all classes to which CI3 attempts to assign dynamic properties.

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.

class CI_URI {
public $config;

Covered #6173.

class CI_Router {
public $uri;

Covered.

class CI_Loader {

Covered.

class CI_Controller {

Covered.

abstract class CI_DB_driver {

Covered.

Approach 1: __get and __set methods
Approach 2: AllowDynamicProperties attribute

Once again, please follow up on #6173... If you find other places, just point them out on that PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants