? Release Blocker ? Pending

User tests: Successful: Unsuccessful:

avatar nikosdion
nikosdion
14 Nov 2021

Summary of Changes

Makes Joomla 4 compatible with PHP 8.1.

Tagging @zero-24 @wilsonge @bembelimen and @HLeithner — this impacts all Joomla versions you are leading the release of, not just 4.0, albeit this PR only targets 4.0 to preserve my and your sanity.

Also tagging @rdeutz as the CMS maintenance liaison, @webgras as the CMS Release Team Leader and @tecpromotion as the CMS Release Team assistant leader — you guys know best if you need to bring this to the attention of other leadership members I may have missed.

Testing Instructions

Install Joomla 4.0 on PHP 7.2, 7.3, 7.4 or 8.0

Create an article — you need to do that on PHP 7.2–8.0 because this is broken on PHP 8.1

Install PHP 8.1.0-RC6 on your server (Windows builds: https://github.com/shivammathur/php-builder-windows — macOS: see https://nunomaduro.com/how-to-install-php-8-1-rc-on-mac/)

Switch your site to use PHP 8.1.0-RC6

Try to edit the article you created

Actual result BEFORE applying this Pull Request

The article title is empty, the article content is empty, you basically get the same interface as a blank article (except for tags and custom fields). Editing or creating an article is impossible. Trying to save changes doesn't work.

Expected result AFTER applying this Pull Request

Editing the article works fine; the interface is populated. Any changes you make are saved fine. Creating an article also works without a problem.

Documentation Changes Required

None

Technical details

The problem happens due to the following change noted in the official PHP 8.1 UPGRADING document:

  . When a method using static variables is inherited (but not overridden), the
    inherited method will now share static variables with the parent method.

        class A {
            public static function counter() {
                static $counter = 0;
                $counter++;
                return $counter;
            }
        }
        class B extends A {}

        var_dump(A::counter()); // int(1)
        var_dump(A::counter()); // int(2)
        var_dump(B::counter()); // int(3), previously int(1)
        var_dump(B::counter()); // int(4), previously int(2)

    This means that static variables in methods now behave the same way as
    static properties.
    RFC: https://wiki.php.net/rfc/static_variable_inheritance

The \Joomla\CMS\Table\Table::getFields() method used a method static variable to cache the fields of the table. However, since this method is inherited but not overridden in child table classes the static method variable was populated with the fields of whichever table class was loaded first — in the Joomla CMS this is always theJoomla\CMS\Table\User class for the #__users table. Since this information is used in the constructor to populate the missing table properties which are then used by the bind() method to decide which incoming source data to apply to the table object it became impossible to load any record or assign any data to any other table, causing this problem. It's actually surprising that only so little broke; we got lucky!

I went through the entire core code with a fine comb and only found another three instances of method–level static variables which would cause a problem and I fixed them.

Yes, there are dozens more method–level static variables. However, these are either keyed to a globally unique key (either a database record ID or a unique ID constructed from immutable, globally unique data such as a component name) OR they were used to cache data which is immutable throughout the life of the request.

PS: CMSObject will need an attribute come PHP 8.2 and a rewrite come PHP 9.0. I'll get back to these next year. For now, it's important to make Joomla 4.0 ready for PHP 8.1 before it's released in two weeks time.

avatar nikosdion nikosdion - open - 14 Nov 2021
avatar nikosdion nikosdion - change - 14 Nov 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 14 Nov 2021
Category Administration com_categories Libraries
avatar HLeithner
HLeithner - comment - 14 Nov 2021

Thanks for investing your the time and make joomla php 8.1 compatible. At the first glance I have one question.

Why is $lastQueryStoreId protected and $hasAssociation is private?

avatar nikosdion
nikosdion - comment - 14 Nov 2021

$lastQueryStoreId may be needed in a child class to force–clear the cached query. FWIW in the one use case I had for that I just set $this->query to NULL but I'm pretty sure someone, somewhere will ask why can't they clear the store ID. I can also see a use case where a child class can use that property to implement a different caching, e.g. caching a serialized copy of the items themselves in Redis. This is a useful key to have access to as a sneaky / enterprising 3PD.

$hasAssociation on the other hand is an internal implementation detail, a cache that's not guaranteed to be populated until $this->getAssoc() is called. Therefore you must NEVER access it directly. So I made it private to prevent anyone from naïvely doing if ($this->hasAssociation) instead of if ($this->getAssoc()) because their IDE proposed using the property instead of the method (phpStorm does that since property access is faster — in this case also wrong, because there's no guarantee it's populated!). Consider the private access childproofing ?

avatar HLeithner
HLeithner - comment - 14 Nov 2021

Make sense, thanks

avatar wilsonge
wilsonge - comment - 14 Nov 2021

Adding release blocker here as we 8.1 will be out by the time we release the next 4.0 release and we should be compatible

avatar richard67
richard67 - comment - 21 Nov 2021

@nikosdion Sorry when it’s a bit off topic because the test here tells to have a Joomla installed with older versions, but have you ever tried to make a new installation of current 4.0-dev with PHP 8.1? It fails for me here and I am not sure if it’s a mistake in my environment or if we have a problem.

avatar nikosdion
nikosdion - comment - 21 Nov 2021

@richard67 Not off–topic at all! It was indeed failing for me too. That's why my testing instructions read “Install Joomla 4.0 on PHP 7.2, 7.3, 7.4 or 8.0”. When I tried that in 8.1.0-RC4 it failed. At that point I was more interested in doing the minimum viable fix for Joomla in case someone upgrades their PHP on an existing site.

Have you tried installing Joomla 4.0-dev on PHP 8.1 after merging my PR? If you did and it still fails we may want to take another look to figure out what's failing. Next week will probably be impossible for me to do so — between Black Friday and the kid staying home from school due to a Covid-19 case in a different class — but I will probably have time to do further tests next weekend. Any preliminary discovery and further information you can give me the better. I'd include any fixes in this PR.

avatar richard67
richard67 - comment - 21 Nov 2021

@nikosdion Thanks for the quick reply. That’s very useful for me to know that it failed for you, too. I will test this PR later and will then test new install with the PR applied. I have not much hope that it helps because the error seemed to me to come from the registry class from the framework. I keep you up to date when I have some results.

avatar richard67
richard67 - comment - 21 Nov 2021

@nikosdion I've made a new installation of current 4.0-dev on an XAMPP using PHP 8.0 and then have switched on debug and error reporting to maximum. Then I have created an article and after that logged out from backend. Then I have changed my webserver to use the PHP 8.1 nightly build for that virtual host and restarted Apache. All that without your PR applied for reproducing the issue. After that I can not even login to backend. When loading the login page, I get an exception.

RuntimeException:
Failed to start the session because headers have already been sent by "C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\event\src\ListenersPriorityQueue.php" at line 17.

  at C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\session\src\Storage\NativeStorage.php:473
  at Joomla\Session\Storage\NativeStorage->start()
     (C:\xampp\htdocs\joomla-cms\libraries\src\Session\Storage\JoomlaStorage.php:305)
  at Joomla\CMS\Session\Storage\JoomlaStorage->start()
     (C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\session\src\Session.php:405)
  at Joomla\Session\Session->start()
     (C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\session\src\Session.php:332)
  at Joomla\Session\Session->has('user')
     (C:\xampp\htdocs\joomla-cms\libraries\src\Session\Session.php:201)
  at Joomla\CMS\Session\Session->get('user')
     (C:\xampp\htdocs\joomla-cms\libraries\src\Factory.php:338)
  at Joomla\CMS\Factory::getUser()
     (C:\xampp\htdocs\joomla-cms\libraries\src\Application\AdministratorApplication.php:229)
  at Joomla\CMS\Application\AdministratorApplication->getTemplate(true)
     (C:\xampp\htdocs\joomla-cms\libraries\src\Error\Renderer\HtmlRenderer.php:47)
  at Joomla\CMS\Error\Renderer\HtmlRenderer->render(object(RuntimeException))
     (C:\xampp\htdocs\joomla-cms\libraries\src\Exception\ExceptionHandler.php:128)
  at Joomla\CMS\Exception\ExceptionHandler::render(object(RuntimeException))
     (C:\xampp\htdocs\joomla-cms\libraries\src\Exception\ExceptionHandler.php:71)
  at Joomla\CMS\Exception\ExceptionHandler::handleException(object(RuntimeException))
     (C:\xampp\htdocs\joomla-cms\libraries\src\Application\CMSApplication.php:311)
  at Joomla\CMS\Application\CMSApplication->execute()
     (C:\xampp\htdocs\joomla-cms\administrator\includes\app.php:63)
  at require_once('C:\\xampp\\htdocs\\joomla-cms\\administrator\\includes\\app.php')
     (C:\xampp\htdocs\joomla-cms\administrator\index.php:32)

Deleting the session cookie did not help.

Beside these exceptions I get the following warnings:

Deprecated: Return type of Joomla\Event\ListenersPriorityQueue::getIterator() should either be compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\event\src\ListenersPriorityQueue.php on line 137

Deprecated: Return type of Joomla\Event\ListenersPriorityQueue::count() should either be compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\event\src\ListenersPriorityQueue.php on line 149

Deprecated: Return type of Joomla\Registry\Registry::jsonSerialize() should either be compatible with JsonSerializable::jsonSerialize(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\registry\src\Registry.php on line 117

Deprecated: Return type of Joomla\Registry\Registry::offsetExists($offset) should either be compatible with ArrayAccess::offsetExists(mixed $offset): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\registry\src\Registry.php on line 394

Deprecated: Return type of Joomla\Registry\Registry::offsetGet($offset) should either be compatible with ArrayAccess::offsetGet(mixed $offset): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\registry\src\Registry.php on line 408

Deprecated: Return type of Joomla\Registry\Registry::offsetSet($offset, $value) should either be compatible with ArrayAccess::offsetSet(mixed $offset, mixed $value): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\registry\src\Registry.php on line 423

Deprecated: Return type of Joomla\Registry\Registry::offsetUnset($offset) should either be compatible with ArrayAccess::offsetUnset(mixed $offset): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\registry\src\Registry.php on line 437

Deprecated: Return type of Joomla\Registry\Registry::getIterator() should either be compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\registry\src\Registry.php on line 255

Deprecated: Return type of Joomla\Registry\Registry::count() should either be compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\registry\src\Registry.php on line 103

Deprecated: Return type of Joomla\Input\Input::count() should either be compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\input\src\Input.php on line 149

Deprecated: trim(): Passing null to parameter #1 ($string) of type string is deprecated in C:\xampp\htdocs\joomla-cms\libraries\src\Application\WebApplication.php on line 355

Deprecated: trim(): Passing null to parameter #1 ($string) of type string is deprecated in C:\xampp\htdocs\joomla-cms\libraries\src\Application\WebApplication.php on line 404

Deprecated: trim(): Passing null to parameter #1 ($string) of type string is deprecated in C:\xampp\htdocs\joomla-cms\libraries\src\Application\WebApplication.php on line 355

Deprecated: trim(): Passing null to parameter #1 ($string) of type string is deprecated in C:\xampp\htdocs\joomla-cms\libraries\src\Application\WebApplication.php on line 404

Deprecated: str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated in C:\xampp\htdocs\joomla-cms\libraries\src\Uri\Uri.php on line 143

Deprecated: Return type of Joomla\Session\Handler\DatabaseHandler::open($save_path, $session_id) should either be compatible with SessionHandlerInterface::open(string $path, string $name): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\session\src\Handler\DatabaseHandler.php on line 228

Deprecated: Return type of Joomla\Session\Handler\DatabaseHandler::close() should either be compatible with SessionHandlerInterface::close(): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\session\src\Handler\DatabaseHandler.php on line 69

Deprecated: Return type of Joomla\Session\Handler\DatabaseHandler::read($session_id) should either be compatible with SessionHandlerInterface::read(string $id): string|false, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\session\src\Handler\DatabaseHandler.php on line 244

Deprecated: Return type of Joomla\Session\Handler\DatabaseHandler::write($session_id, $session_data) should either be compatible with SessionHandlerInterface::write(string $id, string $data): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\session\src\Handler\DatabaseHandler.php on line 275

Deprecated: Return type of Joomla\Session\Handler\DatabaseHandler::gc($maxlifetime) should either be compatible with SessionHandlerInterface::gc(int $max_lifetime): int|false, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\session\src\Handler\DatabaseHandler.php on line 197

Deprecated: Return type of Joomla\Session\Session::getIterator() should either be compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\session\src\Session.php on line 175

Warning: session_name(): Session name cannot be changed after headers have already been sent in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\session\src\Storage\NativeStorage.php on line 405

Deprecated: Return type of Joomla\Event\AbstractEvent::offsetExists($name) should either be compatible with ArrayAccess::offsetExists(mixed $offset): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\event\src\AbstractEvent.php on line 221

Deprecated: Return type of Joomla\Event\AbstractEvent::offsetGet($name) should either be compatible with ArrayAccess::offsetGet(mixed $offset): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\event\src\AbstractEvent.php on line 235

Deprecated: Return type of Joomla\Event\AbstractEvent::count() should either be compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\event\src\AbstractEvent.php on line 149

Deprecated: Return type of Joomla\Event\Event::offsetSet($name, $value) should either be compatible with ArrayAccess::offsetSet(mixed $offset, mixed $value): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\event\src\Event.php on line 126

Deprecated: Return type of Joomla\Event\Event::offsetUnset($name) should either be compatible with ArrayAccess::offsetUnset(mixed $offset): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in C:\xampp\htdocs\joomla-cms\libraries\vendor\joomla\event\src\Event.php on line 145

Applying this PR does not change that.

It seems to be related to the session handler.

If I was really sure that this is not related to some mistake in my environment I would open a new issue for that.

But maybe I've made a mistake with setting up my XAMPP to use different PHP versions for different virtual hosts?

avatar richard67
richard67 - comment - 21 Nov 2021

I'm almost sure there is something wrong with my setup. Will try to fix it.

avatar nikosdion
nikosdion - comment - 21 Nov 2021

Yes, I am not addressing the deprecated stuff — this also happens on PHP 8.0. All the return type deprecation notices were introduced in PHP 8.0 and require adding the PHP annotation #[ReturnTypeWillChange] in each method call. Ideally, we should be consistent. Most of that is in framework packages, not the Joomla CMS so I don't want to touch all of that.

The deprecation notice for trim requires adding a check in the WebApplication class.

None of these is a fatal yet. They will become fatal in PHP 9.0. Either way, I don't want to touch these in this PR. This PR's goal is to make the minimal changes necessary for Joomla to operate on PHP 8.1 under typical usage scenarios on live servers (no error reporting) with Joomla 4.0. Once that's merged we can continue discussing how to best address deprecation notices in Joomla 4.1.

But I digress. Since you have error_reporting=E_ALL and display_errors=On in your php.ini these deprecation notices are output to the browser before the session starts, i.e. before PHP sends the session cookie. That's why the session initialization fails.

Set Joomla's $error_reporting = 'none' OR set display_errors=Off in your php.ini. This will fix this problem.

You will then see that before the PR you have the problem I describe. After the PR that problem goes away.

avatar richard67
richard67 - comment - 21 Nov 2021

But I digress. Since you have error_reporting=E_ALL and display_errors=On in your php.ini these deprecation notices are output to the browser before the session starts, i.e. before PHP sends the session cookie. That's why the session initialization fails.

Set Joomla's $error_reporting = 'none' OR set display_errors=Off in your php.ini. This will fix this problem.

You will then see that before the PR you have the problem I describe. After the PR that problem goes away.

@nikosdion Thanks, that did it for me.

avatar richard67 richard67 - test_item - 21 Nov 2021 - Tested successfully
avatar richard67
richard67 - comment - 21 Nov 2021

I have tested this item successfully on 576805a

Hints for other testers (first 2 for people who use XAMPP):

  • I had to modify the instructions on https://github.com/shivammathur/php-builder-windows so it fetches and uses the "Get-PhpNightly.ps1" because the "Get-Php.ps1" did not work for PHP 8.1.
  • The instructions have to be run in a Windows Power Shell. You might have to run this Power Shell as Administrator if your XAMPP setup is like the default, and you might have to go to the Windows Developer settings to allow to run Power Shell scripts which are not signed.
  • You have to switch off debug in backend and set display_errors=Off in your php.ini for PHP 8.1 in order not to run into other problems, see #36041 (comment) .
    This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36041.
avatar xillibit
xillibit - comment - 21 Nov 2021

I have tested this item successfully on 576805a

Tested on WAMP with Php 8.1.0-RC6

avatar richard67
richard67 - comment - 21 Nov 2021

@xillibit Thanks for testing. But for marking the test result it's not sufficient to just post a comment with the green check mark. Please go to https://issues.joomla.org/tracker/joomla-cms/36041 and use the "Test this" button. Thanks in advance.

avatar xillibit xillibit - test_item - 21 Nov 2021 - Tested successfully
avatar xillibit
xillibit - comment - 21 Nov 2021

I have tested this item successfully on 576805a

By using "Test this" button. Tested on WAMP 3.2.5 with Php 8.1.0-RC6


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36041.
avatar richard67 richard67 - change - 21 Nov 2021
Status Pending Ready to Commit
Labels Added: ? Release Blocker
avatar richard67
richard67 - comment - 21 Nov 2021

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36041.

avatar richard67 richard67 - change - 21 Nov 2021
Labels Added: ?
avatar richard67 richard67 - alter_testresult - 21 Nov 2021 - richard67: Tested successfully
avatar richard67 richard67 - alter_testresult - 21 Nov 2021 - xillibit: Tested successfully
avatar richard67
richard67 - comment - 21 Nov 2021

For some reason drone is not started for this PR. I've tried to fix that by updating the branch, but it did not help. Reasons very likely not related to this PR.

avatar HLeithner HLeithner - change - 21 Nov 2021
The description was changed
avatar HLeithner HLeithner - edited - 21 Nov 2021
avatar HLeithner
HLeithner - comment - 21 Nov 2021

For some reason drone is not started for this PR. I've tried to fix that by updating the branch, but it did not help. Reasons very likely not related to this PR.

The reason was the simply in the description, drone doesn't like it when nicolas is funny.

avatar richard67 richard67 - alter_testresult - 21 Nov 2021 - richard67: Tested successfully
avatar richard67 richard67 - alter_testresult - 21 Nov 2021 - xillibit: Tested successfully
avatar nikosdion
nikosdion - comment - 21 Nov 2021

@HLeithner Everybody's a critic; especially those dang bots! ?

avatar wilsonge wilsonge - change - 21 Nov 2021
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2021-11-21 18:24:13
Closed_By wilsonge
avatar wilsonge wilsonge - close - 21 Nov 2021
avatar wilsonge wilsonge - merge - 21 Nov 2021
avatar wilsonge
wilsonge - comment - 21 Nov 2021

Thanks!

Add a Comment

Login with GitHub to post a comment