[#48368] - [6.2] InstallerScriptTrait::allowDowngrades not working
- Pending
- Medium
- Build: 6.1-dev
- # 48368
- Diff
- n3t:n3t-patch-InstallerScriptTraitgetOldManifest
User tests: Successful: Unsuccessful:
Function getOldManifest searched invalid path. It relied on extension_administrator path, which is available only for components. Changed to extension_root, which is available for all extensions, and in components installer fallback to extension_administrator.
Moreover path manifest contains complete path to manifest not only filename, therefore the result path was always invalid.
- 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
- changed the InstallerScriptTrait::getOldManifest function to search in correct path
- added parameters to error message (current and new version)
Testing Instructions
Use any extension using InstallerScriptTrait with disallowed downgrades (that is by default) and try to install an old version over existing newer version.
Actual result BEFORE applying this Pull Request
The extension is installed anyway,
Expected result AFTER applying this Pull Request
The extension is not installed, error message is displayed,
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 | ⇒ | Libraries |
@richard67 This is just Joomla 6.x related, as the InstallerScriptTrait was introduced in Joomla 6.0, should be replacement for InstallerScript class.
I have tested this item ✅ successfully on 191b7a3
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/48368.
I have tested this item ✅ successfully on 191b7a3
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/48368.
I have tested this item ✅ successfully on 191b7a3
Code review only. If code review is not accepted, @n3t will need to provide sample extensions for testing (two for each type, mean many extensions)
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/48368.
I have tested this item ✅ successfully on 191b7a3
Code review only. If code review is not accepted, @n3t will need to provide sample extensions for testing (two for each type, mean many extensions)
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/48368.
I have tested this item ✅ successfully on 191b7a3
Code review only. If code review is not accepted, @n3t will need to provide sample extensions for testing (two for each type, mean many extensions)
@tecpromotion Was you test also based on code review like @joomdonation 's test? Or were you ablr to test with a real extension?
I have tested this item ✅ successfully on 191b7a3
Code review only. If code review is not accepted, @n3t will need to provide sample extensions for testing (two for each type, mean many extensions)@tecpromotion Was you test also based on code review like @joomdonation 's test? Or were you ablr to test with a real extension?
Real extensions.
| Status | Pending | ⇒ | Ready to Commit |
| Labels |
Added:
bug
PR-6.1-dev
|
||
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/48368.
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/48368.
This pull request has been automatically rebased to 6.2-dev. Joomla! 6.1 is now EOL.
| Title |
|
||||||
| Labels |
Added:
RTC
PR-6.2-dev
Removed: PR-6.1-dev |
||
Tested unsuccesfully with libraries, packages and files
| Status | Ready to Commit | ⇒ | Pending |
Back to pending
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/48368.
Back to pending
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/48368.
Ok, Installer adapters for libraries, files and packages (especially for libraries) handle manifest different (hordcoded) way. There are few options how to handle this:
- hardcode in InstallerScriptTrait::getOldManifest (doesn't seem to be good option to me, as files and packages use one way, but libraries another, special, way)
- add Database query (as in InstallerScript class - using getItemArray function) - this will work, but I'm not sure, if one of the point of this trait wasn't to remove this query
- add some extra function to Adapter, which will be overwritten in descendants for libarary, files and packages to provide correct path
- add extra path to paths array in Installer itself. Again Adapters will set this. Name 'manifest' is already used, so maybe something like 'target_manifest'?
Last option seems the best to me, but it would mean to change bit more than this single file. Using DB query to load the manifest path from database would be more simple, but it is an extra DB query. Not sure which way to choose to comply with philosophy of the Installer, could someone review this and give advice?
| Labels |
Added:
Updates Requested
Removed: RTC |
||
@n3t Is it a bug which happens also in 5.4? Or does it only happen in 6.x? If only in 6.x, the 6.1-dev branch is ok, but if the issue also happens in 5.4, the PR should be made for the 5.4-dev branch. See also https://github.com/joomla/joomla-cms#which-branch-should-my-pull-request-target .