[#47208] - [5.4] Prevent recursions in loadposition/loadmodule plugin
- Pending
- Medium
- Build: 5.4-dev
- # 47208
- Diff
- Hackwar:5.4-loadmodule
User tests: Successful: Unsuccessful:
Pull Request resolves #46899 .
- I read the Generative AI policy and my contribution is either not created with the help of AI or is compatible with the policy and GNU/GPL 2 or later.
Summary of Changes
When you have an article which loads a module position which loads a module which in itself again contains the same article, you can run into an endless recursion, killing the site. This PR introduces a recursion block to prevent stuff like this. If the plugin is run with the same configuration more than once, it simply skips the output again.
Testing Instructions
- Create an article which contains a
{loadposition }command to load a module position. - Create an article module, which loads the article you previously created.
Actual result BEFORE applying this Pull Request
The server runs into a 500 error due to infinite recursion (The module loads the article, which loads the module, which loads the article, which...)
Expected result AFTER applying this Pull Request
The module loads the article, but then the loadmodule plugin stops at that point.
Link to documentations
Please select:
-
Documentation link for guide.joomla.org:
-
No documentation changes for guide.joomla.org needed
-
Pull Request link for manual.joomla.org:
-
No documentation changes for manual.joomla.org needed
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Front End Plugins |
I have tested this item ✅ successfully on 0592c3a
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47208.
I have tested this item ✅ successfully on 0592c3a
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47208.
| Status | Pending | ⇒ | Ready to Commit |
| Labels |
Added:
RMDQ
PR-5.4-dev
|
||
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47208.
@mariantanase I think you did not produce a recursion.
This PR needs some more testing. Please don't merge.
I have not tested this item.
Needs more tests
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47208.
| Status | Ready to Commit | ⇒ | Pending |
Back to pending due to reverted 2nd human test.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47208.
I have tested this item ✅ successfully on 0592c3a
Re-tested with different modules and articles. All fine.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47208.
| Status | Pending | ⇒ | Ready to Commit |
RTC again
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47208.
| Status | Ready to Commit | ⇒ | Pending |
Back to pending due to requested changes. See previous code reviews. @Hackwar Could you check? Thanks in advance.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47208.
| Labels |
Added:
Updates Requested
bug
Removed: RMDQ |
||
It is good now :)
@chmst @ChristineWk If you can find the time, could you test this PR again with the latest changes? That would be really great. Thanks for the previous tests, and thanks in advance for new ones.
| Labels |
Removed:
Updates Requested
|
||
I have tested this item ✅ successfully on 4f3f220
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47208.
I have tested this item ✅ successfully on 4f3f220
I used {loadposition main-top} in articles and selected same position in the articles module and have seen the site 'endless loading' and crashing
Aplying the PR it is working
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47208.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47208.
| Labels |
Added:
RTC
|
||
I have tested this item 🔴 unsuccessfully on 5a85c05
Final test before merge using JBT
- Seen site crashing with OutOfMemory with article with
{loadmodule mod_articles}and site module Articles, with articles category and Introtext enabled - Applied PR with Patch Tester, Site is no more crashing, but has PHP warnings and deprecations:
Warning: Undefined variable $match in /var/www/html/plugins/content/loadmodule/src/Extension/LoadModule.php on line 145
Warning: Trying to access array offset on null in /var/www/html/plugins/content/loadmodule/src/Extension/LoadModule.php on line 145
Deprecated: Using null as an array offset is deprecated, use an empty string instead in /var/www/html/plugins/content/loadmodule/src/Extension/LoadModule.php on line 145
Warning: Undefined variable $match in /var/www/html/plugins/content/loadmodule/src/Extension/LoadModule.php on line 149
Warning: Trying to access array offset on null in /var/www/html/plugins/content/loadmodule/src/Extension/LoadModule.php on line 149
Deprecated: Using null as an array offset is deprecated, use an empty string instead in /var/www/html/plugins/content/loadmodule/src/Extension/LoadModule.php on line 149
Warning: Undefined variable $match in /var/www/html/plugins/content/loadmodule/src/Extension/LoadModule.php on line 177
Warning: Trying to access array offset on null in /var/www/html/plugins/content/loadmodule/src/Extension/LoadModule.php on line 177
- This was seen with PHP 8.5 (used as default), but is the same situation for PHP 8.4
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47208.
| Status | Ready to Commit | ⇒ | Pending |
Back to 'Pending', @Hackwar could you check please?
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47208.
| Labels |
Removed:
RTC
|
||
Fixed
@ThomasFinnern and @ChristineWk may I ask you to test again? It would be best to test loadposition and loadmodule.
I followed the testing instructions, but I don't see that behavior. It works correctly without the PR.