I have tested this item ✅ successfully on a017d12

I've successfully reproduced the issue and tested the fix as described.

In addition, I have successfully tested that when a file which shall be extracted already exists - this can be easily tested by just trying to extract the zip file a 2nd time - then the extraction fails with error alerts "Some files with the same name already exist." and "Failed to extract the archive file.".

@joomdonation Could you add that to the testing instructions so other testers will test that, too? Thanks in advance.

I have tested this item ✅ successfully on a017d12

I've successfully reproduced the issue and tested the fix as described.

In addition, I have successfully tested that when a file which shall be extracted already exists - this can be easily tested by just trying to extract the zip file a 2nd time - then the extraction fails with error alerts "Some files with the same name already exist." and "Failed to extract the archive file.".

@joomdonation Could you add that to the testing instructions so other testers will test that, too? Thanks in advance.

I have tested this item ✅ successfully on a017d12

I've successfully reproduced the issue and tested the fix as described.

In addition, I have successfully tested that when a file which shall be extracted already exists - this can be easily tested by just trying to extract the zip file a 2nd time - then the extraction fails with error alerts "Some files with the same name already exist." and "Failed to extract the archive file.", and nothing is extracted from that archive, also no other files.

@joomdonation Could you add that to the testing instructions so other testers will test that, too? Thanks in advance.

Could you add that to the testing instructions so other testers will test that, too

@richard67 This is existing behavior and not being changed by this PR, so I do not mention it in the testing instructions.

Could you add that to the testing instructions so other testers will test that, too

@richard67 This is existing behavior and not being changed by this PR, so I do not mention it in the testing instructions.

@joomdonation Yes, but you have fixed that, too. Before it would have failed, too, just it would not be testable because of the first error. But now with your PR it works. So I thought it could be worth to be mentioned.

Anyway, I am ok with it.

P.S.: In addition, I have also tested that trying to upload a crafted Zip file which contains a file with a relative path "../../escaped.txt" (path traversal) is not possible (exception thrown by Path::check).

I have tested this item ✅ successfully on a017d12

tested with a zip file containing files and folders

I have tested this item ✅ successfully on a017d12

tested with a zip file containing files and folders

RTC

RTC

what is the expected behaviour when you try to upload a zip file with php files inside it

what is the expected behaviour when you try to upload a zip file with php files inside it

@brianteeman Currently it shows red alert messages "No file was found." and "Failed to upload file.". That can be improved for sure, but right now that's the expected behavior.

@joomdonation Thank you for your speedy fix 👍 and I am wondering that I get success and error message:

Tested with JBT, PHP 8.5 in creating simplest ZIP with one image:

cd images
zip ../media/templates/site/cassiopeia/images/test.zip powered_by.png
zipinfo ../media/templates/site/cassiopeia/images/test.zip powered_by.png 
-rw-r--r--  3.0 unx     3197 bx stor 26-Sep-08 17:25 powered_by.png

'File Content Preview' lists the one image file name.
'Extract Here' extracts the image and I am getting success message Archive file extracted., but additional an error message Archive file not found.?

@muhme From the code, I could not guess how you get both success and error message like that. Can you test it again with a different zip file, maybe with a fresh Joomla 5.4 installation?

@muhme From the code, I could not guess how you get both success and error message like that. Can you test it again with a different zip file, maybe with a fresh Joomla 5.4 installation?

It was the second ZIP file and it was fresh 5.4-dev branch installation. Used Patch Tester. I will try full installation from PR's Download and test ZIP with multiple files ...

Can you upload the zip file which you are using for testing so that I can try to test it myself with your file, too ?

@joomdonation Same result (success + error message) with PR's full packages installation, different ZIP containing multiple images, using 7-Zip instead Apple-compiled zip, Joomla debug disabled or using Safari instead of Firefox. No Joomla or PHP log entries.

Can you upload the zip file which you are using for testing so that I can try to test it myself with your file, too ?

test.zip

I tried with your file (which is a very simple zip file) but for some reasons, I could not replicate the errors you are having here, so strange. Is your test site public accessible? If Yes, can you give me access so that I can try it directly on your test site. Will send you a message on Mattermost.

Set back to Pending as there is an error message in sub-folders

Set back to Pending as there is an error message in sub-folders

I just tried with your zip. Uploading it into various different folders and then extracting it - no problem here

Turned out it is another bug. The error @muhme found happens when we extract a file in media folder. It should be fixed now with my last commit. Could you please test it again? Thanks

@joomdonation how do i replicate the problem?

@brianteeman Please upload the zip file to media/templates/site/cassiopeia , then select that file and extract it from there

I have tested this item ✅ successfully on 3641e47

I have verified the additional error reported by @muhme and confirm that the latest addition to this PR fixes it

I have tested this item ✅ successfully on 3641e47

I have verified the additional error reported by @muhme and confirm that the latest addition to this PR fixes it

I have tested this item ✅ successfully on 3641e47

In addition to my previous test, I've successfully verified the issue found by @muhme and tested the fix.

I have tested this item ✅ successfully on 3641e47

In addition to my previous test, I've successfully verified the issue found by @muhme and tested the fix.

RTC

RTC

✅ Final test before merge with JBT

• Seen the two red error messages in extracting ZIP before PR
• Applied PR with Patch Tester
  • Extracting ZIP with one image is working in templates and also in media folder
  • Extracting ZIP with multiple files and subdirectories is working in templates and also in media folder
  • Extraction of file-system copied ZIP archive with relative paths ../../../../tmp/hello-world.php fails with exception Joomla\Filesystem\Path::check() - Use of relative paths not permitted
  • Extracting archive with already existing files fails with error Some files with the same name already exist.
  • No unusual Joomla or PHP log messages except the named exception and
    • Not related to this PR: On the attempt to upload a ZIP archive with relative paths fails with red error message and there are:
      • PHP Warning: Trying to access array offset on null in /var/www/html/administrator/components/com_templates/src/Model/TemplateModel.php on line 1389
      • PHP Deprecated: transliterator_transliterate(): Passing null to parameter # 2 ($string) of type string is deprecated in /var/www/html/libraries/vendor/joomla/filesystem/src/File.php on line 95

Thank you very much @joomdonation for your speedy fix. Thanks to @brianteeman and @richard67 for testing multiple times.

Thanks