User tests: Successful: Unsuccessful:
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.
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
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.
Editing the article works fine; the interface is populated. Any changes you make are saved fine. Creating an article also works without a problem.
None
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.
Status | New | ⇒ | Pending |
Category | ⇒ | Administration com_categories Libraries |
$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
Make sense, thanks
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
@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.
@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.
@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.
@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?
I'm almost sure there is something wrong with my setup. Will try to fix it.
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.
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 setdisplay_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.
I have tested this item
Hints for other testers (first 2 for people who use XAMPP):
@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.
I have tested this item
By using "Test this" button. Tested on WAMP 3.2.5 with Php 8.1.0-RC6
Status | Pending | ⇒ | Ready to Commit |
Labels |
Added:
?
Release Blocker
|
RTC
Labels |
Added:
?
|
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.
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.
@HLeithner Everybody's a critic; especially those dang bots!
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 |
Thanks!
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?