[#37911] - Feature/com joomlaupdate improvements
- Fixed in Code Base
- 16 Jun 2022
- Medium
- Build: 4.2-dev
- # 37911
- Diff
- nikosdion:feature/com_joomlaupdate_improvements
User tests: Successful: 0 Unsuccessful: 0
Summary of Changes
Replaces #36531. Now made against 4.2-dev since it's been nearly five months since I opened the previous one, Joomla 4.1 will be EOL in 2 ½ months and it only makes sense to include this in 4.2 so that updates to 4.3 will not have a problem. Not ideal but, hey, I've reported these issues four times since 2020 (before Joomla 4.0 was released) and done the work to fix them since January 2nd, 2022 (before Joomla 4.1 was released).
This PR addresses a number of issues regarding the way Joomla Update detects and reports the compatibility of installed extensions with the target version of Joomla.
Changes made:
- Improved language string about “Update Information Unavailable” extensions which does not falsely blame 3PDs for user actions.
- Make the confirmation checkboxes optional (controlled by component options), allowing advanced users to streamline the update process.
- Batch update checks to prevent server errors during the pre–update checks.
Pinging @roland-d and @richard67
Testing Instructions
- Find a cheap shared hosting which enforces request rate limiting and create a site there.
- Install 20-30 extensions of your choosing.
- Find any extension that comes in a package.
- Extract the package.
- Install all sub–extensions on a site manually.
- Change the update site to one created for a Joomla 4.2 PR
- Go to System, Update, Joomla and click on Check for Updates
Actual result BEFORE applying this Pull Request
The Extensions tab shows a lot of Server Error messages.
The extensions listed under “Update Information Unavailable” show a message which blame the developer for not providing an update site.
You have to check the “Acknowledge the warnings about potentially incompatible extensions and proceed with the update.” before clicking on Update. Then you have to check the “I'm prepared for the update and have made a backup.” checkbox to proceed with the update.
Expected result AFTER applying this Pull Request
The Extensions tab no longer shows a lot of Server Error messages.
The extensions listed under “Update Information Unavailable” show a message which merely states that Joomla cannot get compatibility information.
Now go to the Options page for Joomla Update and set “Disable extension version check confirmation” and “Disable backup before update confirmation” to Yes. Click on Save & Close.
You do no longer see the “Acknowledge the warnings about potentially incompatible extensions and proceed with the update.” checkbox; you can simply click on Update. You no longer see the “I'm prepared for the update and have made a backup.” checkbox in the next page. You can just click the button to update.
Documentation Changes Required
The two new options need to be documented.
Language changes required.
Modified language string COM_JOOMLAUPDATE_VIEW_DEFAULT_EXTENSIONS_UPDATE_SERVER_OFFERS_NO_COMPATIBLE_VERSION_NOTES.
Added language strings COM_JOOMLAUPDATE_CONFIG_NOBACKUPCHECK_LABEL and COM_JOOMLAUPDATE_CONFIG_NOVERSIONCHECK_LABEL.
Technical information
I will discuss each change made in detail.
Improved language string about “Update Information Unavailable” extensions
The previous language string asserted that third party developers were to blame for any extensions reported as having no compatibility information. I've already reported this is inaccurate.
The most concerning and most common problem is that a sub–extension belonging to a package may have no package_id in the #__extensions table. This happens in two surprisingly common cases:
- The user extracted the package's ZIP file and installs the sub–extensions' ZIP files separately. Extraction of the package ZIP happens automatically on macOS using Safari and its default settings.
- For whatever reason, the site owner used Discover to install sub–extensions whose files are there but they were not fully installed (no
#__extensionstable record). This happens when people restore backups — manually created, taken through their host or taken through a 3PD extension — haphazardly.
The problem is that even though the 3PD has taken care to provide an update site correctly for the package, the user's actions resulted in a messed up situation. It's not possible for 3PDs to add the package's update site as an update source for each sub–extension, therefore the 3PD has no real way to address this issue. All the while Joomla is lying to the user that the 3PD, the only innocent victim in this case, is to blame.
The new message simply states that Joomla could not determine the update status, without making any false assertions about the root cause.
Note: the user could fix the problem by installing the package type extension. Joomla will then correct the #__extensions table information. However, do remember that a. the user is oblivious to the nature of this problem and b. they likely ended up in this situation because they couldn't install the package extension either due to ignorance / Operating System lying to the user (first case described above) or server limitations (second case). Therefore expecting users to fix a problem using a method they didn't or couldn't use to begin with is... completely pointless?
Make the confirmation checkboxes optional (controlled by component options)
Several users have expressed their extreme displeasure at the two checkboxes leading to a Joomla update on the official Joomla Facebook group and the Joomla Forum. The gist is that they feel it's condescending and a waste of time. They openly admitted they are already ignoring the pre–update checks as a result — something I had already warned you about well over a year ago — and wished for a way to get rid of the checkboxes.
This PR adds two Joomla Update component options to remove each of the checkboxes i.e. the ‘Acknowledge the warnings about potentially incompatible extensions and proceed with the update.’ checkbox in the pre–update check page and the ‘I'm prepared for the update and have made a backup.’ checkbox in the main update page.
These options are disabled by default, i.e. the default state is to have the checkboxes shown and required to protect newbie users from themselves thereby staying true to the reason of existence of these checkboxes.
Batch update checks
Currently, Joomla is trying to run one XHR (AJAX request) per extension ID it needs to determine compatibility information for — and sends these requests all at once, even if there are several dozens of them!
From the server's point of view this looks like a Denial of Service attack. As a result the requests are blocked on most live servers. In some servers the user's IP is temporarily blocked. Joomla does catch that and report a “server error” but this is a cop–out solution which merely undermines the stated objective of the pre–update check feature.
The solution I implemented is fairly straightforward. Joomla Update creates a list of all extension IDs and currently installed versions which need to be checked and sends them to the server. The server will determine the update information of as many extensions as it can within 5 seconds (counted until the beginning of determining an extension's update information). It returns the bulk results and a list of the extensions it didn't have time to check yet. Rinse and repeat until there are no more extensions to check.
This solves the issue because a. there is only one request at a time hitting the server and b. there is enough time between the requests to prevent a server error. Therefore the server no longer thinks it's being hit by a DoS attack.
There is still a situation where this will fail: if an update server is unresponsive in a blocking manner for a long time. In this case all pending extensions will be reported as throwing a server error. In most cases reloading the page resolves the issue.
Further to that, Joomla Update will now convey the server–side warnings, i.e. it will inform the user if a particular update site is inaccessible.
Moreover, in case of a server error, we now call Joomla.ajaxErrorsMessages(xhr) to report the exact error condition experienced instead of only showing everything as errored out.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_joomlaupdate Language & Strings JavaScript Repository NPM Change |
| Labels |
Added:
Language Change
NPM Resource Changed
?
|
||
@Kostelano You can try again now. I addressed that change.
@nikosdion I was testing this and managed to generate an error on the XHR request and this results in an untranslated string:
We should add Text::script('JLIB_JS_AJAX_ERROR_OTHER'); to the preupdatecheck.php file. Can you add that in this PR?
All right, everywhere else we were indeed pushing the strings from the view template. So I am doing the same in the preupdatecheck.php file.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37911.
@nikosdion We have conflict(s) now in the UpdateController.php file after the last upmerge from 3.10-dev to 4.1-dev to 4.2-dev. The conflict(s) originally come from PR #36950 having been merged all the way up, but there were several iterations on solving conflicts after that upmerge.
@richard67 And done. The up merge also forgot to add correct name spacing in a type hint, I did that here (to avoid another merge conflict).
@nikosdion By review it looks ok regarding the conflict resolution. But I think it needs new tests since there has been changed so much in the base branch.
@Kostelano Could you redo your test? Thanks in advance.
@richard67 As you can see I didn't actually change anything in the code :) I only added the new method ajax() introduced in 3.10.10 and which is not actually used in this PR.
@richard67 As you can see I didn't actually change anything in the code :) I only added the new method
ajax()introduced in 3.10.10 and which is not actually used in this PR.
@nikosdion Yes, I saw that. I just like to be sure the PR still fits together with the changes from the upmerged 3.10-dev PR, just in case something got lost during the upmerge or whatever, so to have a test again would make me feel more safe somehow.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37911.
Thanks @Kostelano .
If I see this correct, we are waiting for 1 more human test or am I missing something?
If I see this correct, we are waiting for 1 more human test or am I missing something?
Yep, one more test needed. Unfortunately I am off desk.
@brianteeman Can you give it a test please?
Find a cheap shared hosting which enforces request rate limiting and create a site there.
Any recommendations on how to simulate this. I tried on my server using mod_reqtimeout but either it doesnt do what I thought or I got the syntax wrong
Please remove package-lock from the pr
@brianteeman It is tricky.
You can emulate with mod_security2, see https://johnleach.co.uk/posts/2012/05/15/rate-limiting-with-apache-and-mod-security/ A rate limit of about 10 would do it on a local server.
You can also use mod_qos with a QS_LocRequestLimit set to something really low, e.g. 3 or 5.
IIRC cPanel uses (or used to use) mod_evasive but this module has not been maintained since 2017.
This PR adds two Joomla Update component options to remove each of the checkboxes i.e. the ‘Acknowledge the warnings about potentially incompatible extensions and proceed with the update.’ checkbox in the pre–update check page and the ‘I'm prepared for the update and have made a backup.’ checkbox in the main update page.
These options are disabled by default, i.e. the default state is to have the checkboxes shown and required to protect newbie users from themselves thereby staying true to the reason of existence of these checkboxes.
You have set the default value to 0 (hide) for backupcheck. Is that correct?
COM_JOOMLAUPDATE_VIEW_DEFAULT_POTENTIALLY_DANGEROUS_PLUGIN_CONFIRM_MESSAGE="Are you sure you want to ignore the warnings about potentially incompatible extensions and proceed with the update?"
COM_JOOMLAUPDATE_VIEW_DEFAULT_NON_CORE_PLUGIN_CONFIRMATION="Acknowledge the warnings about potentially incompatible extensions and proceed with the update."
shouldnt the first one also be changed to use the word acknowledge and not ignore?
COM_JOOMLAUPDATE_VIEW_DEFAULT_POTENTIALLY_DANGEROUS_PLUGIN_CONFIRM_MESSAGE="Are you sure you want to ignore the warnings about potentially incompatible extensions and proceed with the update?"
COM_JOOMLAUPDATE_VIEW_DEFAULT_NON_CORE_PLUGIN_CONFIRMATION="Acknowledge the warnings about potentially incompatible extensions and proceed with the update."
shouldnt the first one also be changed to use the word acknowledge and not ignore?
As you may have noted, I have NOT changed the language strings in this PR. The former was last changed by @bembelimen on 13 July 2021 and the latter also changed by him on 17 October 2021. You can make a separate PR for issues unrelated to this PR.
This PR adds two Joomla Update component options to remove each of the checkboxes i.e. the ‘Acknowledge the warnings about potentially incompatible extensions and proceed with the update.’ checkbox in the pre–update check page and the ‘I'm prepared for the update and have made a backup.’ checkbox in the main update page.
These options are disabled by default, i.e. the default state is to have the checkboxes shown and required to protect newbie users from themselves thereby staying true to the reason of existence of these checkboxes.
You have set the default value to 0 (hide) for backupcheck. Is that correct?
Good catch, I changed this.
<input type="checkbox" class=" me-3" id="noncoreplugins" name="noncoreplugins" value="1" required />
Should be changed to
<input type="checkbox" class="form-check-input me-3" id="noncoreplugins" name="noncoreplugins" value="1" required />
before
after
This PR adds two Joomla Update component options to remove each of the checkboxes i.e. the ‘Acknowledge the warnings about potentially incompatible extensions and proceed with the update.’ checkbox in the pre–update check page and the ‘I'm prepared for the update and have made a backup.’ checkbox in the main update page.
These options are disabled by default, i.e. the default state is to have the checkboxes shown and required to protect newbie users from themselves thereby staying true to the reason of existence of these checkboxes.
You have set the default value to 0 (hide) for backupcheck. Is that correct?
Good catch, I changed this.
Almost. You need to update the default in the php as well.
$this->noBackupCheck = $params->get('backupcheck', 0) == 0;
<input type="checkbox" class=" me-3" id="noncoreplugins" name="noncoreplugins" value="1" required />Should be changed to
<input type="checkbox" class="form-check-input me-3" id="noncoreplugins" name="noncoreplugins" value="1" required />before
after
Once again, THIS HAS NOTHING TO DO WITH THIS PULL REQUEST. This line was last changed on 13 July 2021 by @bembelimen
Please keep your code review to what I have changed in this Pull Request and only that. Use git blame to find out which line changed when and by whom. I have deliberately kept this PR very specific in scope. It's been extremely hard fixing issues in Joomla! Update even when we have users complaining loudly about it. Therefore I try to fix one small thing at a time, hopefully fixing the problems I have been reporting the year prior to Joomla 4's release within the next 5-10 years (since nobody else can be bothered to do so and my attempts to fix what is blatantly broken are met with way too much resistance)...
Almost. You need to update the default in the php as well.
Done
Once again, THIS HAS NOTHING TO DO WITH THIS PULL REQUEST. This line was last changed on 13 July 2021 by @bembelimen
Fine I will create a pr to fix this obvious error
Once again, THIS HAS NOTHING TO DO WITH THIS PULL REQUEST. This line was last changed on 13 July 2021 by @bembelimen
Fine I will create a pr to fix this obvious error
Thank you! It bugs me too but if I start scope creeping this 6 month old PR will stay not merged for god knows how much longer :p
I have tested this item
managed to trick my server into timeout when checking for extension updates. after this pr that didnt happen.
everything else works as described
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37911.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37911.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37911.
| Labels |
Added:
?
|
||
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-06-16 09:29:18 |
| Closed_By | ⇒ | roland-d |
Thanks everybody
If the option with the checkbox is disabled, then we cannot update, because the button is inactive (relatively recently, the PR was adopted, which allows the button to become available only after the checkbox is filled).