? Pending

User tests: Successful: Unsuccessful:

avatar nikosdion
nikosdion
4 Jan 2022

Summary of Changes

This PR fixes a caching issue which occurs when a module which defines head scripts or styles is being cached after another extension (e.g. template, plugin, or module) has already defined its own, different, head scripts or styles.

This is a type error in Joomla's cache handler which exists in Joomla 3 as well. This PR is for Joomla 4.0.

Tagging @crystalenka (reported the issue to me), @wilsonge and @HLeithner for review.

Testing Instructions

  • Install a new Joomla 4 site. Make sure you are using PHP 8.0
  • Login to the backend of your site
  • Go to System, Install, Extensions
  • Install the Foobar test plugin
  • Install the Foobar test module
  • Go to System, Manage, Plugins and enable the System - Foobar plugin
  • Go to Content, Site Modules, click on New
  • Choose the “Foobar Test Module”
  • Use the following information
    • Title: Test
    • Position: Main-top [main-top]
  • Click on Save & Close
  • Go to your site's public frontend

Checkpoint 1

  • Back in the backend of your site, go to Home Dashboard and click on Global Configuration
  • Click the System tab and set System Cache to ON - Progressive caching
  • Click on Save & Close

Checkpoint 2

Actual result BEFORE applying this Pull Request

Checkpoint 1

The site works properly and you can see the not cached Foobar module.

Checkpoint 2

Joomla shows an error page (“The requested page can't be found.”) with the following error message:

If difficulties persist, please contact the website administrator and report the error below.

0 mb_strlen(): Argument #1 ($string) must be of type string, array given

Expected result AFTER applying this Pull Request

Checkpoint 1

The site works properly and you can see the not cached Foobar module.

Checkpoint 2

The site works properly and you can see the cached Foobar module.

Documentation Changes Required

None whatsoever.

Technical information

The two extensions are dummies which add head styles in the document at different points of the application lifecycle just to trigger the error condition. It should also be possible if you have two DIFFERENT module extensions doing the same, or a module and a template, etc.

The Joomla cache handler (\Joomla\CMS\Cache\Cache) has a major type bug in the setWorkarounds method. When it compares the script and style keys of the document object's head data it incorrectly assumes that they contain string values. In actual reality they always contain EITHER an empty string OR an array.

Joomla then calls \Joomla\String\StringHelper::substr() and \Joomla\String\StringHelper::strlen() against the contents of these header keys. If they are non–empty, i.e. the contain an array, this causes a type error.

Under PHP 5 and PHP 7 the type mismatch error is a mere PHP warning. You might see it, most likely you won't (because you're likely running your site with error reporting set to None) and caching doesn't work correctly for ‘inexplicable reasons’.

Under PHP 8 type mismatch errors have been upgraded to ErrorExceptions i.e. fatal PHP errors. This is what is caught by Joomla's error handler and why you see an error page instead of the cached module.

My suspicion is that this code was copied from The Olden Days (Joomla 1.x / 2.x and possibly very early 3.x, if memory serves) when Joomla indeed stored a string in the document instead of an array. When the document object was refactored many moons ago this largely undocumented, esoteric piece of code was not updated. The kind of failure it caused was silent under PHP 5 and 7 so any caching issues it caused were attributed to the mythical ‘ghost in the machine’, the cache expired or was reset and everything was good in the world. Come PHP 8 and the root cause, the type mismatch, is now a stop error and suddenly the ghost in the machine grew fangs and bit us in the posterior.

I also suspect without being certain that some Joomla forum posts may have mistakenly blamed an innocent module for this issue, possibly claiming that the plugin is not compatible with PHP 8. Please let me say again that this issue is 100% unrelated to any third or first party extension. It is a bug in Joomla's cache handler. Fun times...

avatar nikosdion nikosdion - open - 4 Jan 2022
avatar nikosdion nikosdion - change - 4 Jan 2022
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 4 Jan 2022
Category Libraries
avatar nikosdion nikosdion - change - 4 Jan 2022
Labels Added: ?
avatar toivo
toivo - comment - 5 Jan 2022

I have tested this item successfully on a58fedc

Tested successfully in 4.0.6-dev of January 4 using PHP 8.0.13 and 8.1.0 in Wampserver 3.2.6


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

avatar toivo toivo - test_item - 5 Jan 2022 - Tested successfully
avatar crystalenka
crystalenka - comment - 5 Jan 2022

I have tested this item successfully on a58fedc


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

avatar crystalenka crystalenka - test_item - 5 Jan 2022 - Tested successfully
avatar richard67 richard67 - change - 5 Jan 2022
Status Pending Ready to Commit
avatar richard67
richard67 - comment - 5 Jan 2022

RTC


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

avatar richard67
richard67 - comment - 5 Jan 2022

RTC


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

avatar Fedik
Fedik - comment - 5 Jan 2022

The fix is incorrect, you should not use implode, but compare array instead.
(Use of Implode will have different result for the same scripts in defferent order).
it nothing with PHP. It because Joomla 4 use array to store each script, while Joomla 3 a string.

And we already have a fix for this #36068 , please confirm if it works for you.

avatar richard67 richard67 - change - 5 Jan 2022
Status Ready to Commit Pending
avatar richard67
richard67 - comment - 5 Jan 2022

Back to pending due to previous comment. Please check.


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

avatar nikosdion nikosdion - change - 5 Jan 2022
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2022-01-05 12:30:12
Closed_By nikosdion
avatar nikosdion
nikosdion - comment - 5 Jan 2022

@Fedik Yeah, thanks, I know why it happens (I actually wrote about that in length at the end of the PR).

I don't see how the scripts / styles would ever be in a different order since what this code checks is whether a script / style that wasn't there has been appended to the document's head. Joomla can only append to the document head, it cannot inject in a different order.

In any case, the point is moot. The other PR is more economical code-wise. I prefer that approach instead of mine.

avatar nikosdion nikosdion - close - 5 Jan 2022
avatar wilsonge
wilsonge - comment - 5 Jan 2022

Merged the other one

Add a Comment

Login with GitHub to post a comment