Adding PHP 8.2 support #6173

gxgpet · 2022-11-06T14:23:44Z

This adds support for PHP 8.2.

Basically, fixing dynamic props assigning.

Khuthaily · 2022-12-08T07:20:36Z

Thank you, @gxgpet. In addition to the changes your are proposing, I had to add #[AllowDynamicProperties] in system > core > Controller.php and system > core > Router.php in one of my projects to provide support for PHP 8.2.

…roller

gxgpet · 2023-01-27T01:07:30Z

Thanks for your suggestion, @Khuthaily! I added the attribute only for CI_Controller; as for the router, the uri property was missing only, and I added it. I guess you didn't find something different for the router, right?

sneakyimp · 2023-02-06T21:00:19Z

Where do we stand on 8.2 compatibility? Issue #6192 seems to basically be a duplicate of this issue and I'm pretty eager to apply this fix to a production system I'm upgrading to PHP 8.2. Are we sure it covers all the dynamic class assignments? Are there any unit tests dedicated to checking this? Has anyone used a static analysis tool to check for undeclared properties? I've checked out PHPStan expressly to find issues like this and it apparently doesn't detect undefined property assignments on objects instantiated via a factory method.

sneakyimp · 2023-02-07T21:56:44Z

In the interest of furthering the effort for 8.2 compatibility, I have downloaded the latest CI 3.1.13 and run php stan on it:

vendor/bin/phpstan analyse -l 2 codeigniter > phpstan-output.txt

This yields about 2090 errors (see attached phpstan-output.tgz), including 688 'Access to an undefined property' errors. One example of which is:

Access to an undefined property CI_Migration::$lang.

I grepped those lines and wrote a quick PHP script to list all the distinct classes before the '::' and this is the result:

$this(CI_DB_cubrid_driver)
$this(CI_DB_mssql_driver)
$this(CI_DB_mysql_driver)
$this(CI_DB_mysqli_driver)
$this(CI_DB_oci8_driver)
$this(CI_DB_pdo_cubrid_driver)
$this(CI_DB_pdo_dblib_driver)
$this(CI_DB_pdo_ibm_driver)
$this(CI_DB_pdo_mysql_driver)
$this(CI_DB_pdo_odbc_driver)
$this(CI_DB_pdo_sqlsrv_driver)
$this(CI_DB_postgre_driver)
$this(CI_DB_sqlsrv_driver)
CI_Controller
CI_DB_cubrid_driver
CI_DB_driver
CI_DB_ibase_driver
CI_DB_ibase_forge
CI_DB_mssql_driver
CI_DB_mysql_driver
CI_DB_mysqli_driver
CI_DB_oci8_driver
CI_DB_pdo_4d_driver
CI_DB_pdo_cubrid_driver
CI_DB_pdo_dblib_driver
CI_DB_pdo_driver
CI_DB_pdo_firebird_driver
CI_DB_pdo_firebird_forge
CI_DB_pdo_ibm_driver
CI_DB_pdo_informix_driver
CI_DB_pdo_mysql_driver
CI_DB_pdo_oci_driver
CI_DB_pdo_odbc_driver
CI_DB_pdo_pgsql_driver
CI_DB_pdo_pgsql_forge
CI_DB_pdo_result
CI_DB_pdo_sqlite_driver
CI_DB_pdo_sqlsrv_driver
CI_DB_postgre_driver
CI_DB_postgre_forge
CI_DB_sqlite3_driver
CI_DB_sqlite_driver
CI_DB_sqlsrv_driver
CI_Image_lib
CI_Input
CI_Javascript
CI_Jquery
CI_Migration
CI_Router
CI_Table
CI_URI
object

I am not at all certain, but it looks like we might have a bit more work to do if we want to insure CI3 is really ready for PHP 8.2.

Anyone have thoughts on this?

EDIT: It looks like PHPStan doesn't see the CI_DB class definition because CI3 defines this class in a conditional block. I think we might be able to ignore all the CI_DB_* classes because they are either extending CI_DB_query_builder or CI_DB_driver -- surely we can put the attribute in those files abstract parent classes, right?

sneakyimp · 2023-02-15T22:00:00Z

Is there any progress on supporting PHP 8.2? I've seen a lot of people posting issues about it but they are all being closed except for this one, which seems to have the most up-to-date information and pull request. I badly need to update a server running a CI3 website and I'd very much like to have the site work with PHP 8.2 so I can avoid yet another later migration.

I don't see any declaration of AllowDynamicProperties in this pull request before the critical CI_DB declaration here. Nor do I see one applied to the CI_DB_query_builder abstract class here. I don't actually know if you can put AllowDynamicProperties in front of an abstract class, but it seems to me that we might fix most of those CI_DB_x_driver classes reported by phpstan if we were to address those two class declarations.

CI_Controller already has the dynamic properties attribute declared here in this pull request.

That would leave the following classes reported by phpstan:
CI_DB_ibase_forge
CI_DB_pdo_firebird_forge
CI_DB_pdo_pgsql_forge
CI_DB_pdo_result
CI_DB_postgre_forge
CI_Image_lib
CI_Input
CI_Javascript
CI_Jquery
CI_Migration
CI_Router
CI_Table
CI_URI

Those forge and result classes could be handled, I think, by declaring AllowDynamicProperties for their parent classes.

Curiously, CI_Javascript and CI_Jquery classes do not exist in this pull request, so I'm not sure what to do about them.

The various other classes might also be solved with an AllowDynamicProperties declaration. Personally, I see no harm in simply adding these various declarations, and then it seems we are likely to have a viable PHP 8.2 fix, no?

Do we have (or need) Unit tests to trigger the depreciated warning for these classes? What must we do to make progress on this issue?

sneakyimp · 2023-02-20T22:38:16Z

It occurred to me to run phpstan on gxgpet's pull request instead of the CI3 codebase, and PHPstan is reporting a lot fewer errors. See attached gxgpet-output.zip which I generated like so:

vendor/bin/phpstan analyse -l 2 gxgpet > gxgpet-output.txt

This is still reporting about 1000 errors, but only 54 of those refer to undefined properties:

$ grep 'undefined prop' gxgpet-output.txt
  410    Access to an undefined property object::$directory.
  412    Access to an undefined property object::$directory.
  281    Access to an undefined property CI_Input::$raw_input_stream.
  465    Access to an undefined property object::$dbdriver.
  465    Access to an undefined property object::$dbdriver.
  469    Access to an undefined property object::$dbdriver.
  182    Access to an undefined property object::$dbdriver.
  182    Access to an undefined property object::$dbdriver.
  187    Access to an undefined property object::$dbdriver.
  1480   Access to an undefined property CI_DB_driver::$qb_limit.
  474    Access to an undefined property object::$dbprefix.
  545    Access to an undefined property object::$dbprefix.
  548    Access to an undefined property object::$dbprefix.
  102    Access to an undefined property CI_DB_ibase_forge::$hostname.
  117    Access to an undefined property CI_DB_ibase_forge::$conn_id.
  123    Access to an undefined property object::$database.
  157    Access to an undefined property CI_DB_mysql_driver::$db.
  230    Access to an undefined property CI_DB_mysqli_driver::$db.
  146    Access to an undefined property CI_DB_pdo_result::$db.
  148    Access to an undefined property CI_DB_pdo_result::$db.
  88     Access to an undefined property CI_DB_pdo_firebird_forge::$hostname.
  103    Access to an undefined property CI_DB_pdo_firebird_forge::$conn_id.
  109    Access to an undefined property object::$database.
  102    Access to an undefined property CI_DB_pdo_pgsql_forge::$create_table_if.
  133    Access to an undefined property object::$database.
  162    Access to an undefined property CI_DB_postgre_driver::$db.
  90     Access to an undefined property CI_DB_postgre_forge::$create_table_if.
  119    Access to an undefined property object::$database.
  300    Access to an undefined property CI_DB_sqlsrv_driver::$_escape_like_chr.
  300    Access to an undefined property CI_DB_sqlsrv_driver::$_escape_like_str.
  558    Access to an undefined property CI_Image_lib::$dest_image.
  563    Access to an undefined property CI_Image_lib::$dest_image.
  571    Access to an undefined property CI_Image_lib::$dest_image.
  577    Access to an undefined property CI_Image_lib::$dest_image.
  145    Access to an undefined property CI_Migration::$lang.
  148    Access to an undefined property CI_Migration::$load.
  168    Access to an undefined property CI_Migration::$db.
  170    Access to an undefined property CI_Migration::$dbforge.
  174    Access to an undefined property CI_Migration::$dbforge.
  176    Access to an undefined property CI_Migration::$db.
  215    Access to an undefined property CI_Migration::$lang.
  276    Access to an undefined property CI_Migration::$lang.
  289    Access to an undefined property CI_Migration::$lang.
  294    Access to an undefined property CI_Migration::$lang.
  337    Access to an undefined property CI_Migration::$lang.
  396    Access to an undefined property CI_Migration::$lang.
  446    Access to an undefined property CI_Migration::$db.
  460    Access to an undefined property CI_Migration::$db.
  521    Access to an undefined property object::$lang.
  521    Access to an undefined property object::$lang.
  521    Access to an undefined property object::$lang.
  521    Access to an undefined property object::$lang.
  521    Access to an undefined property object::$lang.
  43     Access to an undefined property CI_TestCase::$ci_view_root.

I have started picking through some of these and believe that quiet a few might be resolved by declaring var types with php doc comments. For example, those first two complaints are in system/core/CodeIgniter.php because we don't declare a var type for the var $RTR:

/*
 * ------------------------------------------------------
 *  Instantiate the routing class and set the routing
 * ------------------------------------------------------
 */
        $RTR =& load_class('Router', 'core', isset($routing) ? $routing : NULL);

Those first two PHPStan complaints could be resolved if we add an extra asterisk and @var declaration to the comment so it declares the var type:

/**
 * ------------------------------------------------------
 *  Instantiate the routing class and set the routing
 * ------------------------------------------------------
 * @var CI_Router|NULL
 */
        $RTR =& load_class('Router', 'core', isset($routing) ? $routing : NULL);

We might similarly remedy the complaints about object::$dbdriver in system/core/Loader.php by declaring a type more specific than simply 'object.'

Numerous complaints in Loader.php are due to the bad return type declaration in this function:

        /**
         * CI Component getter
         *
         * Get a reference to a specific library or model.
         *
         * @param       string  $component      Component name
         * @return      bool
         */
        protected function &_ci_get_component($component)
        {
                $CI =& get_instance();
                return $CI->$component;
        }

This peculiar function looks suspiciously similar to a __get magic method but you'd have to expressly invoke it rather than relying on 'magic' invocation. A grep search of the code shows that this function is only invoked in ./system/core/Loader.php to retrieve a config object. It is never used in any other file and never used to retrieve anything but a config object. We can rid ourselves of several PHPStan errors simply by changing the return declaration from @return bool to @return object.

I'd be happy to try and put in a little time to try and get rid of these 54 'undefined property' errors if folks think that would be a good idea. I would hate to go through the trouble only to have my efforts ignored.

Thoughts? Anyone?

steveadamsfixedit · 2023-02-24T15:33:57Z

I'd very much like to see PHP 8.2 supported...

gxgpet · 2023-03-01T02:00:37Z

@sneakyimp

Are we sure it covers all the dynamic class assignments?

No, we are not, that's why I waited for more reports to come up... As part of CI 3's architecture, many of those can be found only if the code is actually executed.

In the interest of furthering the effort for 8.2 compatibility, I have downloaded the latest CI 3.1.13 and run php stan on it:

This is indeed a very easy step to do, and I made it too with another tool; the problem is that the output of such a tool is containing a lot of false positives, and they must be sorted out... No such tool can understand properly how the code works, it's just static analysis.

But anyway, if anyone is concerned with the file you uploaded, here's the analysis I could made for it:

We added #[AllowDynamicProperties] on CI_DB_driver, which is the root class for all implementations: CI_DB_query_builder extends CI_DB_driver, CI_DB extends CI_DB_query_builder. This can be easily found & proven if the code is opened with an IDE. So there's no issue on the DB layer and that only class which has the attribute is enough.
We are not discussing Access to an undefined property X, Method X() should return Y but return statement is missing, Constant X not found and Variable X might not be defined.. These are obvious false positives, and they are not even related to the new PHP version.
PHPDoc tag X has invalid value - nothing here
CI_Migration has the __get magic method implemented
CI_Image_lib - a commit was pushed with adding the missing property
CI_Input has the __get magic method implemented.
CI_Javascript, CI_Jquery - these were removed in 2016
CI_Router, CI_Table and CI_URI are already changed here

Sorry, but any bug/issue we might have with PHP 8.2 must be reviewed and checked by a human eye to see if that's the case. We can't change the entire code just to avoid any false positives from a static analysis tool, we won't do it.

I'd very much like to see PHP 8.2 supported...

Well, I guess we are at the end of it since nothing more popped up recently. I think we shall deploy a new version soon.

sneakyimp · 2023-03-01T02:30:39Z

As I said last week:

I'd be happy to try and put in a little time to try and get rid of these 54 'undefined property' errors if folks think that would be a good idea. I would hate to go through the trouble only to have my efforts ignored.

I went and examined every 'undefined property' complaint from PHPStan and fixed several bugs and hundreds of bad PHPDoc declarations in #6198. That PR contains every change you've made in this one, and numerous other improvements as well. I believe we should use that PR instead. Either that or merge its changes into this PR. If you don't like the change to the file system/database/DB.php, you can leave it out. Personally, I think it's weird and bad style to dynamically declare that trivial CI_DB class inside a conditional, which is inside a function. I think the reason for that kludge in the legacy CI3 code is an inept way of supporting unit testing, which is also poorly implemented in this project.

NielBuys · 2023-03-14T10:59:41Z

Can somebody give me the "composer.json" changes/steps of how I can test this Pull request. I want to test and see if my app is compatible with this pull request.

NelsonXavier30 · 2023-03-15T14:02:42Z

I couldn't figure out how to use my application without these errors

NelsonXavier30 · 2023-03-15T14:03:33Z

someone to help me?

NielBuys · 2023-03-15T14:40:33Z

I created a patch file from this pull request and merged the patch file into my fork of CodeIgniter to test my app.

I had 2 more errors I needed to fix in CodeIgniter before my app started working. I needed to add "AllowDynamicProperties" to Controller.php and Router.php. See attached patch file for my changes.

NOTE: I see some changes was not included in original patch file. The below changes is not needed to be added. I changed my branch to include all commits of this pull request.

Add AllowDynamicProperties to Controller and Router.zip

I created a test environment for myself If somebody want to use it temporarily you are welcome. I can't guarantee the branch and I don't support it. Its only for myself.

What I needed to change in my app to use it.

Change "codeigniter/framework" to "nielbuys/framework" in the "composer.json" file.
Change in main "index.php" the following line from "$system_path = 'vendor/codeigniter/framework/system';" to "$system_path = 'vendor/nielbuys/framework/system';"
Then run composer update. Then composer will install the test framework v3.1.13.2

My app works in my test environment.

Here is the patch file I created from this pull request for record purposes.
6173.zip

Thanks all

sneakyimp · 2023-03-20T19:14:45Z

I had 2 more errors I needed to fix in CodeIgniter before my app started working. I needed to add "AllowDynamicProperties" to Controller.php and Router.php. See attached patch file for my changes.

I think you may a little confused about which code you are working with. The pull request discussed in this issue is the one suggested by gxgpet, and you can see in that pull request that he has the AllowDynamicProperties declaration in Controller and in Router he added a property or two to hopefully resolve dynamic property complaints.

wagnerfnds · 2023-03-21T15:37:03Z

Honestly it is sad to see codeigniter3 dying, just now that PHP is comin to be a very modern language. Time to migrate our projects i think.

RedDragonWebDesign · 2023-03-21T15:43:26Z

What are next steps to get a code review and merge for this? Looks like it's been sitting for 4 months. Do we need to ping some additional collaborators? @gxgpet looks like you have merge access for this repo, any additional thoughts?

RedDragonWebDesign · 2023-03-21T15:44:59Z

@narfbg

NielBuys · 2023-03-22T06:51:48Z

I had 2 more errors I needed to fix in CodeIgniter before my app started working. I needed to add "AllowDynamicProperties" to Controller.php and Router.php. See attached patch file for my changes.

I think you may a little confused about which code you are working with. The pull request discussed in this issue is the one suggested by gxgpet, and you can see in that pull request that he has the AllowDynamicProperties declaration in Controller and in Router he added a property or two to hopefully resolve dynamic property complaints.

Thanks for letting me know. I have realized after I created the post, that the patch file from Github did not include those 2 changes. I added a note to my original post that I have changed my branch to exactly match what is in this pull request.

colinalbion · 2023-04-01T10:34:29Z

We have substantial CI3 applications running nicely on php 7.4 that we don't want to rewrite for CI4 just to be PHP 8x and 9.x compatible. We need a version of CI3 that is compatible with 7.4 and above (ideally php8.1 and eventually php9). I'd happily support an initiative to do this.

To rewrite for CI4 would involve upheaval to 20k+ lines of code whereas making CI 3 compatible with PHP 8+ would perhaps be just a few hundred of lines of code of change.

Anyone else in the same position?

Edit: After a few hours....added upgraded to 3.1.13, then added #[AllowDynamicProperties] to a few system files (Controler, URI etc) and a few of our own libs, we now have our app working with PHP8.2 (very pleased). I recognise that this is not the final solution, but it is reassuring that there is a path forward where we don't have to entirely rewrite tens of thousands of CI3 code into CI4 or Laraval. Again, it would be great to see a CI3 version that is compatible with PHP 8+ (perhaps dropping php5 support).

codestellar · 2023-04-16T12:10:39Z

Will this branch be merged? I am looking forward to see php 8.2 support for CI3.

mohammadhzp · 2023-04-21T12:38:26Z

@narfbg Are you able to merge this pull request?

Nedbup · 2024-02-27T21:21:56Z

Is anything happening with this? Looks like it is about ready?

any chance of having this merged in soon?

kenjis · 2024-02-27T21:46:40Z

There is a fork project:
https://packagist.org/packages/pocketarc/codeigniter

(Added)
Another fork:
https://github.com/ib3ltd/CodeIgniter/tree/master

jamieburchell · 2024-02-27T21:54:20Z

There is a fork project: https://packagist.org/packages/pocketarc/codeigniter

The releases from this fork appear to be based on the develop branch and from scanning the commits, include functionality changes. The develop branch is quite a bit different to the last released version.

IMO it would be better to cherry-pick and apply specific fixes to the latest actual release/master if you want to keep as close as possible to the last official 3.1.13 release.

kenjis · 2024-02-27T22:12:29Z

Indeed it is.
But since it is a fork, the person who forks it should do as s/he likes.

jamieburchell · 2024-02-27T22:15:37Z

Indeed it is. But since it is a fork, the person who forks it should do as s/he likes.

Of course. But people should be aware that it's not purely a bug fix fork of the latest release.

gxgpet · 2024-02-27T22:49:52Z

All contributions & ideas to this PR are welcome, but can you please stop polluting it with everything non-PHP 8.2 support-related? Every time coming here to review the PR again takes a lot of time...

jamieburchell · 2024-02-27T23:05:59Z

can you please stop polluting it with everything non-PHP 8.2 support-related?

It's hardly surprising. This PR is from 2022. I think people are bored waiting/unable to wait/have given up waiting for it to be merged or a decision made or frustrated by the lack of answers to "can we get this merged".

RedDragonWebDesign · 2024-02-27T23:14:30Z

@gxgpet Thanks for writing this patch. Is there someone you can call in a favor from in this organization to get your PR reviewed? @narfbg maybe? Or maybe you can self-merge?

pocketarc · 2024-02-28T06:38:12Z

@jamieburchell Note that this PR is -also- based on the 'develop' branch, so even if this PR gets merged eventually, you'll still need to move on from the 3.1.13 release.

One thing that I've tried to do with the fork as well is revert breaking changes made in 'develop', precisely so that it can work as a drop-in upgrade for 3.1. I'd suggest giving it a try - it should work without any changes being necessary. And if anything pops up that does break something for you, I'll happily take care of it.

jamieburchell · 2024-02-28T07:25:48Z

@jamieburchell Note that this PR is -also- based on the 'develop' branch

@pocketarc Sure, but it can be merged in to stable master/3.1.13 to form a new 3.1.14 release without the need to use the entire develop branch as a 3.2.x release. I believe this is how the 3.1.x releases are made, and why you don't see develop commits from 2015 in 3.1.x.

Ultimately everyone is free to fork and use whatever they like so for the company I work at which uses 3.1.13 I decided to fork purely for 3.1.x bug fixes and no BC. Like you, I'm not interested in new features just PHP 8.x support and beyond. I also don't want to introduce all the changes from develop. The projects I have also use DataMapper ORM and the database changes made in develop are not compatible with that. I could fix it, but I don't need to.

If a 3.1.14 is ever released (doubtful) hopefully I won't need to maintain a bug fix branch but my hand has been forced to do so because of PHP EOL.

1stwebdesigns · 2024-02-28T08:17:10Z

@jamieburchell could you explain briefly how to use this patch without forking, or using a fork, of the develop branch?

jamieburchell · 2024-02-28T09:00:34Z

@jamieburchell could you explain briefly how to use this patch without forking, or using a fork, of the develop branch?

@1stwebdesigns I ended up creating a fork of this repo purely to service my own needs. I merged the changes I wanted from the develop branch in to my master (which is basically 3.1.13). Any changes I wanted that weren't yet merged in to develop I created my own PR for and merged those in to my master too. I vetted the changes to ensure no develop things crept in, like for example the cookie helper which has different defaults for parameters. Then I pointed my composer.json at my own fork and used dev-master as the version to use.

mackieee · 2024-03-11T11:04:47Z

Does this PR generally need wider user testing before being put live? Is that's what holding this great work back?

kenjis · 2024-03-12T01:23:36Z

If you expect this to be merged, why don't you all test it yourself, review it and approve it?

manusfreedom · 2024-03-27T17:09:09Z

For your information, I have corrected the following code :

in system\database\DB_driver.php instead of using #[\AllowDynamicProperties] :

	/**
	 * List of dynamic properties
	 *
	 * @var	mixed[]
	 */
	protected $_internal_properties	= array();
	public function __set(string $name, mixed $value): void
	{
		if(isset($this->$name))
		{
			$this->$name = $value;
		}
		else
		{
			$this->_internal_properties[$name] = $value;
		}
	}
	public function &__get($name)
	{
		if(isset($this->$name))
		{
			return $this->$name;
		}
		else
		{
			return $this->_internal_properties[$name];
		}
	}
	public function __isset($name)
	{
		return isset($this->$name) || isset($this->_internal_properties[$name]);
	}
	public function __unset($name)
	{
		if(isset($this->$name))
		{
			unset($this->$name);
		}
		else
		{
			unset($this->_internal_properties[$name]);
		}
	}

to system\database\drivers\mysqli\mysqli_driver.php line 128:

		// Do we have a socket path?
		if ($this->hostname && $this->hostname[0] === '/')
		{

I don't know why, but in my projects, #[\AllowDynamicProperties] was not needed in Loader.php.
I tried to avoid #[\AllowDynamicProperties] which will be deprecated in PHP 9, as I understand it, but I can't avoid it in Controller.php because of the use of set by reference at the same time as the __set&__get magic.

pocketarc · 2024-03-27T17:15:16Z

@manusfreedom Note: Dynamic properties are what's deprecated; the attribute is not deprecated, nor is there a plan to deprecate it at the moment. The attribute exists to tell PHP "yes, we're using dynamic properties on purpose", which makes perfect sense in the case of a legacy framework built around dynamic properties.

From the RFC (emphasis mine):

The creation of dynamic properties on classes that aren't marked with the #[AllowDynamicProperties] attribute is deprecated in PHP 8.2 and becomes an Error exception in PHP 9.0.

Source: https://wiki.php.net/rfc/deprecate_dynamic_properties

mackieee · 2024-05-13T12:34:32Z

Currently in the process of updating our site to PHP 8.2 - Certainly adding #[\AllowDynamicProperties] to almost all system Controllers was required (E.g. CI_Benchmark), more so than what the pull request suggested seemingly. Once I had done this, looks to be in good shape.

jamieburchell · 2024-05-13T12:46:36Z

Currently in the process of updating our site to PHP 8.2 - Certainly adding #[\AllowDynamicProperties] to almost all system Controllers was required (E.g. CI_Benchmark), more so than what the pull request suggested seemingly. Once I had done this, looks to be in good shape.

The CI_Benchmark controller on its own doesn't appear to use dynamic properties. Perhaps you have overridden it?

mackieee · 2024-05-13T13:59:32Z

Currently in the process of updating our site to PHP 8.2 - Certainly adding #[\AllowDynamicProperties] to almost all system Controllers was required (E.g. CI_Benchmark), more so than what the pull request suggested seemingly. Once I had done this, looks to be in good shape.

The CI_Benchmark controller on its own doesn't appear to use dynamic properties. Perhaps you have overridden it?

Hi Jamie, appears that may be the case (:
I admit I've gone a little overzealous with adding the attribute where it may not be needed :)

Did you come across much else, in the way of issues in this pull request you forked from?

jamieburchell · 2024-05-13T14:08:32Z

Did you come across much else, in the way of issues in this pull request you forked from?

No issues with this PR. I have it merged in to a fork we are using in production along with some other fixes. You can see the difference between the fork and 3.1.13 here.

mackieee · 2024-05-15T21:41:11Z

I have come across an issue in my tests - although I respect this is a little difficult to replicate due to the hardware stack, thus a bit of an edge case. It's one of that which PHP >=8.1 has introduced.

I've identified the issue but would appericate input if the fix could break other aspects of the query builder.

Our site uses Oracle OCI8 & MySQL DB instances. See below for a small example:

$odbc  = $this->load->database( "oracle", true );
$mysql = $this->load->database( "mysql", true );

$products = $odbc->select([ "sku", "stock" ])->from( "products" )->get_compiled_select();
$users = $mysql->select([ "username", "dt_last_login" ])->from( "users" )->get_compiled_select();

echo PHP_VERSION . "<br />";
echo $products  . "<br />";
echo $users;

Running the above in PHP <= 8.0 would yield:

8.0.24
SELECT "sku", "stock" FROM "products"
SELECT `username`, `dt_last_login` FROM `users`

However in PHP >= 8.1:

8.1.28
SELECT "sku", "stock" FROM "products"
SELECT "username", "dt_last_login" FROM "users"

This produces a invalid MySQL query and thus a hard MySQL Error.

Note the escape_char has been modified in the second query - note this only happens if a MySQL query attempts to be built after a connection that has a different modifier.

The offending line is in DB_Driver.php - All PHP versions prior to 8.0 act in the same manner, so I'm assuming static has had some behaviour change in PHP 8.1.

CodeIgniter/system/database/DB_driver.php

Line 1417 in 6074d34

static $preg_ec = array();

Simply removing static does fix the issue, I'm assuming this was there originally as an optimisation. More than a functional use.

jamieburchell · 2024-05-15T22:04:57Z

I'm assuming static has had some behaviour change in PHP 8.1.

I think it's this

Not sure why that preg_gc couldn't be initialised when the class is instantiated and stored as its own class variable?

kenjis · 2024-05-16T02:53:40Z

This might be useful for you. codeigniter4/CodeIgniter4#5262

jamieburchell · 2024-05-16T06:33:21Z

This might be useful for you. codeigniter4/CodeIgniter4#5262

I'll open a PR for this later unless @gxgpet is planning on doing the same.

jamieburchell · 2024-05-16T09:10:07Z

@mackieee Would you mind testing this PR to see if this resolves your issue?

Please note that the PR is based from the develop branch and so merging only these changes in to the master/latest release needs to be done carefully (there will be some conflicts) so as not to copy over anything else that's changed. I'll merge this in to my fork if there are no issues found.

mackieee · 2024-05-16T09:52:01Z

Wow, thanks for your time and effort here @jamieburchell 🥇 Highly appericated. I'll give this a test now :)

Update: That worked a treat 👍

As per your note on the usage of the static variables in the final classes, doesn't appear to have been removed in @kenjis's review on this in CI4 and is still current repo. So I'd say we're safe there.

Adding PHP 8.2 support

fb1256a

tenzap mentioned this pull request Nov 6, 2022

unit tests Encryption_test::test_initialize_encrypt_decrypt fails with OpenSSL3 #6171

Closed

Merge branch 'bcit-ci:develop' into develop

ff28f9d

tenzap mentioned this pull request Jan 10, 2023

Release new version with fixes for php 8.2 #6188

Closed

Adds $uri prop in Router, and AllowDynamicProps attribute for CI_Cont…

e9ff2f2

…roller

This was referenced Jan 27, 2023

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

Closed

FIX: Fixes dynamic property deprecation error #6193

Closed

sneakyimp mentioned this pull request Mar 1, 2023

PHP 8.2 compatibility, a couple of bug fixes, move CI_DB class declaration #6198

Closed

Adding missing dest_image property in CI_Image_lib

d3c610b

jafjaf mentioned this pull request Apr 11, 2024

php8.2 core related changes - V1 #6277

Closed

This was referenced May 16, 2024

Remove static variables for PHP 8.1 #6280

Closed

Fix usage of static variables #6281

Open

jafjaf mentioned this pull request May 29, 2024

Support for PHP 8.3 #6278

Open

Adding PHP 8.2 support #6173

Are you sure you want to change the base?

Adding PHP 8.2 support #6173

Conversation

gxgpet commented Nov 6, 2022

Khuthaily commented Dec 8, 2022

gxgpet commented Jan 27, 2023

sneakyimp commented Feb 6, 2023

sneakyimp commented Feb 7, 2023 • edited

sneakyimp commented Feb 15, 2023

sneakyimp commented Feb 20, 2023 • edited

steveadamsfixedit commented Feb 24, 2023

gxgpet commented Mar 1, 2023

sneakyimp commented Mar 1, 2023

NielBuys commented Mar 14, 2023

NelsonXavier30 commented Mar 15, 2023

NelsonXavier30 commented Mar 15, 2023

NielBuys commented Mar 15, 2023 • edited

sneakyimp commented Mar 20, 2023

wagnerfnds commented Mar 21, 2023

RedDragonWebDesign commented Mar 21, 2023

RedDragonWebDesign commented Mar 21, 2023

NielBuys commented Mar 22, 2023

colinalbion commented Apr 1, 2023 • edited

codestellar commented Apr 16, 2023

mohammadhzp commented Apr 21, 2023

Nedbup commented Feb 27, 2024

kenjis commented Feb 27, 2024 • edited

jamieburchell commented Feb 27, 2024 • edited

kenjis commented Feb 27, 2024

jamieburchell commented Feb 27, 2024 • edited

gxgpet commented Feb 27, 2024

jamieburchell commented Feb 27, 2024 • edited

RedDragonWebDesign commented Feb 27, 2024

pocketarc commented Feb 28, 2024

jamieburchell commented Feb 28, 2024 • edited

1stwebdesigns commented Feb 28, 2024

jamieburchell commented Feb 28, 2024 • edited

mackieee commented Mar 11, 2024

kenjis commented Mar 12, 2024

manusfreedom commented Mar 27, 2024

pocketarc commented Mar 27, 2024

mackieee commented May 13, 2024 • edited

jamieburchell commented May 13, 2024

mackieee commented May 13, 2024

jamieburchell commented May 13, 2024 • edited

mackieee commented May 15, 2024 • edited

jamieburchell commented May 15, 2024

kenjis commented May 16, 2024

jamieburchell commented May 16, 2024

jamieburchell commented May 16, 2024 • edited

mackieee commented May 16, 2024 • edited

sneakyimp commented Feb 7, 2023 •

edited

sneakyimp commented Feb 20, 2023 •

edited

NielBuys commented Mar 15, 2023 •

edited

colinalbion commented Apr 1, 2023 •

edited

kenjis commented Feb 27, 2024 •

edited

jamieburchell commented Feb 27, 2024 •

edited

jamieburchell commented Feb 27, 2024 •

edited

jamieburchell commented Feb 27, 2024 •

edited

jamieburchell commented Feb 28, 2024 •

edited

jamieburchell commented Feb 28, 2024 •

edited

mackieee commented May 13, 2024 •

edited

jamieburchell commented May 13, 2024 •

edited

mackieee commented May 15, 2024 •

edited

jamieburchell commented May 16, 2024 •

edited

mackieee commented May 16, 2024 •

edited