PR-5.0-dev Pending

User tests: Successful: Unsuccessful:

avatar Hackwar
Hackwar
1 Jul 2023

Summary of Changes

In UpdateModel of the Joomla Update component, we are doing some write operations on the filesystem as preparation before unpacking the update package. This code contains some unnecessary steps and is missing error handling.

Going through the changes one by one:

  • With the switch from the CMS to the framework File package, the File::delete() now checks if the file exists first and otherwise throws an exception. Thus we either have to wrap it in is_file() or catch the exception.
  • We then try to write the update package file to our filesystem, but we never check if that was successfull.
  • Trying to delete the update.php, File::delete() does the same as unlink() now and already does the opcache invalidation.
  • Since Joomla 4 doesn't have the FTP layer anymore, we can simplify the last codeblock greatly.

Testing Instructions

This code change resulted from manual code review and is not based on a specific error condition. Simply said, updating Joomla should still work the same as before.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

avatar joomla-cms-bot joomla-cms-bot - change - 1 Jul 2023
Category Administration com_joomlaupdate
avatar Hackwar Hackwar - open - 1 Jul 2023
avatar Hackwar Hackwar - change - 1 Jul 2023
Status New Pending
avatar richard67
richard67 - comment - 1 Jul 2023
  • With the switch from the CMS to the framework File package, the File::delete() now checks if the file exists first and otherwise throws an exception. Thus we either have to wrap it in is_file() or catch the exception.

@Hackwar I think the same applies to deleting files and folders on update in script.php. Of course something for another PR. Is that right?

Update: I see we have already an is_file check there for the files so that should be ok. Not sure about the folders, will check later.

avatar HLeithner HLeithner - close - 1 Jul 2023
avatar HLeithner HLeithner - merge - 1 Jul 2023
avatar HLeithner HLeithner - change - 1 Jul 2023
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2023-07-01 10:35:08
Closed_By HLeithner
Labels Added: PR-5.0-dev
avatar HLeithner
HLeithner - comment - 1 Jul 2023

Should be easier to test this when it's in nightlies

Add a Comment

Login with GitHub to post a comment