I don't think we have a policy on this but I would think it is better not to use 66/99 quotation marks in our strings. They dont even exist on my keyboard and it will be too easy for someone to use a regular quotation mark which will break the string. I would either use an escaped " or a single '

@tecpromotion ??

I would prefer to fix the content of latest as I have requested from @HLeithner d of the change to the language string.

I don't disagree with that. I am also aware that they are using a very old, very customised version of Akeeba Release System so I am not entirely sure if they can do that. Years ago Michael had told me some of the configuration he had to make and I had told him that's bonkers. I'll leave it at that.

I don't think we have a policy on this but I would think it is better not to use 66/99 quotation marks in our strings.

My bad, I should have changed these to « and ». Let me merge your suggestions and I'll get back to that.

FYI, you should have these quotes using the right ALT key (AltGr — Alternate Graph). It exists in all European ISO layouts but the actual glyphs may vary between keyboard layouts and OS. On Windows I believe you can press WIN+. (Windows key and dot held down together) to get the Emoji picker which also has alternate graphs in its last pages. I am on the MacBook Pro now, can't check to be 100% sure at the moment.

I obviously meant “ and ”. Brain, brain, oh where are thou?

Amazing, GitHub converts the HTML entities. Ampersand ldquo semicolon and ampersand rdquo semicolon. Hopefully you get what I mean. Sigh.

I obviously meant “ and ”. Brain, brain, oh where are thou?

\"text_inside\"

UPD
I see that they didn't work. very strange, because there are a lot of places in the admin panel where we use them.

@Kostelano I was trying to avoid using these wrong quotes but HTML entities don't work with alerts so... I have up and used the wrong quotes. Oh well...

I know how to type the ” with the keyboard ( alt 0147). I meant that its not on an actual key.

@nikosdion The javascript linter complains: https://ci.joomla.org/joomla/joomla-cms/57598/1/26

Can you fix the conflicts please and then I will test it

Done

The horrible alerts that I wish we didnt use any more do not support html

Incorrect message

incorrect message

test 2 and test 4
there is no tar only tar.gz
its not possible to select those files as the file selector is restricted to zip
so cant get the "expected result" from your instructions

Tests 2 & 4: I had rigged my local site to allow any file. You are right that in the release it won't allow you to select these files. Ignore them.

@brianteeman Try again, I fixed the JavaScript.

Also, it's worth noting that you cannot pass double or single quotes to JavaScript using \' or \" in the language string. Therefore I reverted these language strings to use calligraphic quotes.

thanks will check - hopefully with a little help from @Fedik the alerts will be getting a facelift with sweetalert2

I am done arguing with @dgrammatiko so let him figure out how to solve this on his own. If he's a know-it-all he'd better bloody prove it by writing AND maintaining code for the core.

@nikosdion you have some serious issues, how the heck am I even involved here? I haven't participated in this PR!

Dimitri, when you finally get your you know what together you'll see it's the PR for the issue you participated in, where YOU told me that the approach I proposed (and which I implemented in this PR) is wrong.

So, if you think you know better, show us your code or get lost. Kthxbye

Nicholas can you please reopen the PR.
And please exchange alert to Joomla.renderMessages

Joomla.renderMessages({error:['Errororororo message!']})

But if you do not want to bother with JavaScript part, just trash it, and leave all hard job to Server side, that also fine ?
I would really keep only a server side validation for "Package or not to Package".

@Fedik Do you have some time to do the change yourself? You SHOULD have write access as you're a repo maintainer and I've selected the GitHub option to allow y'all to make changes to the code. If not let me know and I'll try to do that over the weekend. I have paid work I need to finish.

Just a note about Joomla.renderMessages. Have you guys tried using the Upload & Update page on a small screen (13" to 15") with two or three messages about PHP upload options showing on the screen? I had to scroll about a page and a half down to find the upload field. If I use Joomla.renderMessages how would I know there's a message (if I remember correctly it doesn't make the page auto-scroll to the top)? :)

Just a note about Joomla.renderMessages. Have you guys tried using the Upload & Update page on a small screen (13" to 15") with two or three messages about PHP upload options showing on the screen?

Yeah that not very nice, but that another issue.

Do you have some time to do the change yourself?

I will do later on, PR to your branch

@nikosdion I made PR to your branch.

Note about \" ? to know:
To avoid such thing need to use Text::script('POTATO'); instead of Text::script('POTATO', true);.
$jsSafe param is a legacy thing from times when we render JS, instead of using JSON.

@Fedik Would you make a PR with a DocBlock change to explain that second parameter? Its use is very opaque. I saw we were passing it in the existing code for Joomla Update and just copied it over. Apparently I don't use it in my own software because at some point years ago I did debug that and saw what it does but I forgot in the meantime...

still getting the wrong error message

Did you run npm install? There should not be alerts anymore

i used the generated build

I hope it was not Joomla_4.2.2_to_4.2.3-dev+pr.38707 or Joomla_4.2.x_to_4.2.3-dev+pr.38707.

Please update /media/com_joomlaupdate/ files from Joomla_4.2.3-dev+pr.38707-Development-Full_Package.zip or Joomla_4.2.3-dev+pr.38707-Development-Update_Package.zip

I used

https://ci.joomla.org/artifacts/joomla/joomla-cms/4.2-dev/38707/downloads/57644/pr_list.xml

trying a clean install now

grrh - I was seeing a cached download link

ok I have the correct install now and see the messages instead of alerts.

However every test produces the same error message
COM_JOOMLAUPDATE_VIEW_UPLOAD_ERROR_NOTZIP="You need to upload the “Upgrade Package (.zip)” file of the Joomla version you want to upgrade to. You are trying to upload a file which is not in the ZIP format. The Joomla Update does not support the upgrade package files with .tar.gz and .tar.bz2 extensions."

I never get the message
COM_JOOMLAUPDATE_VIEW_UPLOAD_ERROR_FULLINSTALLATION="The ZIP file you uploaded looks like the Full Package of Joomla! which is only for creating new sites. You can only use the “Upgrade Package (.zip)” to update your site."

That strange, because it works for me.

Can you please try next: enable debug, edit media/com_joomlaupdate/js/default.js to add console.log(file);return; somwhere after here:

Should be kind of this:

Test 1
w/ log

w/o log
COM_JOOMLAUPDATE_VIEW_UPLOAD_ERROR_NOTZIP

Test 2

COM_JOOMLAUPDATE_VIEW_UPLOAD_ERROR_NOTZIP

Test 3

COM_JOOMLAUPDATE_VIEW_UPLOAD_ERROR_NOTZIP

hmhm, I wonder what is application/x-zip-compressed, should be application/zip.
what browser do you use, Safari?

google chrome on windows 11

according to python.org this is the expected mimetype on windows
https://bugs.python.org/issue41738

@brianteeman Let me set up my Windows 11 machine for core development and I'll take a look later today. I think I know exactly how to address it but I'd like to test it, not just do a blind fix :)

@nikosdion I think we can just remove "type check" and leave only ".zip" extension. The server side will throw an error if it not valid zip anyway. Or what do you think?

The PR as is works on macOS (therefore also any BSD box) and Linux. Let me get my Windows box ready so I can run more tests. I want to make an informed decision.

I have updated the PR with the following changes:

• Reinstated the checks for the Update Package and Full Package based only on their naming pattern. This is important to provide an early and helpful message to users trying to use the wrong package.
• Fixed the Windows ZIP file type detection.
• Replaced calligraphic quotes with regular double quotes using @Fedik's advice on the second parameter of Text::script().
• Updated the PR test instructions, explaining how you can drag'n'drop a .tar.gz file even though clicking on Choose File doesn't let you choose it. This also explains how people ended up trying to use these files. I only noticed because I instinctively dragged and dropped the update ZIP file when upgrading a local site earlier today on my Windows box (which I had not used for a month) at which point I went "d'oh, that's what other people must be doing, too!".

Happy testing!

TEST 1: The file DOES NOT upload. You get a message that you are using the wrong package type (Full instead of Upgrade). ✅
TEST 2: The file DOES NOT upload. You get a message that you are using the wrong format.✅
TEST 3: The file DOES NOT upload. You get a message that you are using the wrong format.❌ wrong package type message (but i think thats correct message and your expected result is wrong)
TEST 4: The file DOES NOT upload. You get a message that you are using the wrong format. ✅
TEST 5: The file DOES NOT upload. You get a message that you are using the wrong format. ❌
I d/l this file Joomla_4.2.2-Stable-Update_Package.tar.gz which i used in test 4 and renamed it to Joomla_4.2.2-Stable-Update_Package.zip
wrong package type message
TEST 6: Success!

PS your comment about needing to do drah n'drop is not required on windows

TEST 3: The file DOES NOT upload. You get a message that you are using the wrong format.❌ wrong package type message (but i think thats correct message and your expected result is wrong)

Yes, it was a typo. I fixed it in the PR description.

TEST 5: The file DOES NOT upload. You get a message that you are using the wrong format. ❌

It took me a few re-reads but I realised that what you got is actually what you should be getting now that we changed the way PR works. Even though you uploaded a file with a ZIP extension its MIME type says it's a Gzip file so, yes, it should tell you that you're using the wrong format. I need to update the PR description about that.

PS your comment about needing to do drah n'drop is not required on windows

I have yet to see a real user using that drop-down but I have seen plenty use drag'n'drop. You also missed that drop-down yesterday yourself which tells me a lot about how “invisible” it is to real users ?

There, I have edited the PR description. Now the test instructions reflect the code. If you can @brianteeman please check that the messages printed make sense for each case from a user's perspective. I am too close to the implementation now to make an objective judgement of that.

I have tested this item ✅ successfully on 1c9f344

acknowleding that the strings might be refined this is still a successful test

I used the pre build package to test. For some reason some of the language strings are not outputted. I noticed that the language strings are present in the language file com_joomlaupdate.ini. Also installing the right update package produces an error.

Here are my results:

Test 1: Error
You are trying to upload the "Full Package (.zip)" which can only be used to create new sites. You need to upload the "Upgrade Package (.zip)" file of the Joomla version you want to upgrade to.

Test 2: Error
You are trying to upload a file which is not in the ZIP format. The Joomla Update does not support the upgrade package files with .tar.gz and .tar.bz2 extensions. You need to upload the "Upgrade Package (.zip)" file of the Joomla version you want to upgrade to.

Test 3: Error
You are trying to upload the "Full Package (.zip)" which can only be used to create new sites. You need to upload the "Upgrade Package (.zip)" file of the Joomla version you want to upgrade to.

Test 4: Error
COM_JOOMLAUPDATE_VIEW_UPLOAD_ERROR_NOTZIP

Test 5: Error
COM_JOOMLAUPDATE_VIEW_UPLOAD_ERROR_FULLINSTALLATION_PREUPLOAD

Test 6: Error
COM_JOOMLAUPDATE_VIEW_UPLOAD_ERROR_FULLINSTALLATION_PREUPLOAD

@RickR2H You need to clear your browser's cache.

@brianteeman I've restored your test result in the issue tracker so it's properly counted. Changes which invalidated the count were only the language string changes which you had requested. If you know some other PR where test results need to be restored, let me know.

@RickR2H Could you try again with all kinds of cache cleared, including browser cache? Thanks in advance.

Will update the branch so we get new test packages built.

@brianteeman Could you test again? Thanks in advance.

I have tested this item ✅ successfully on 76c52d0

Tested with success the described cases and a few more.

What was not working as described was the test with the tar.gz or tar.bz2 with extension changed to zip, i.e. the test which uses the MIME type verification by the browser, in my case Firefox. The reason for this is that the browser does not really do a MIME type check based on the content of the file but on the extension. I've verified this by renaming a JPEG image to "Joomla_4.2.6-rc2-dev-Development-Update_Package.zip" and adding an alert(file.type); to the JS. The type was "application/x-zip-compressed". As said, it was a renamed JPEG ?

So the PR works, only the browser is a bit stupid :-)

One remark in addition to my test result: In my opinion the checks for the file names are too restrictive.

They check for the file names to match the right, unmodified file names.

But when I download nightly build packages, I am used to add a date part to the end of the base name, e.g. Joomla_4.2.6-rc2-dev-Development-Full_Package_2022-12-07.zip or Joomla_4.2.6-rc2-dev-Development-Update_Package_2022-12-07.zip.

Other people might do the same but use a different scheme for that date part.

I think this should still be allowed, but this PR will inhibit to use these files.

I am sure I could provide the right regular expressions to allow such suffixes in any format but still checking the constant part of the file name right.

But this might be subject of discussion.

Therefore I don't suggest a change to this PR.

This PR here is a huge improvement for keeping people from doing silly things, and I want it to come forward.

When it will be merged, I will make a follow up PR to allow modifications at the end of the file's base name, which then can be discussed.

I have tested this item ✅ successfully on 76c52d0

Works for me. Encountered similar situation with FF that Richard did, but agree this is a huge improvement that can be improved upon.

RTC

I am not going to be contributing to this project anymore. I won't be replying to any questions or making any changes until then. I will close any outstanding PRs and delete their code on December 31st, 2022. Merge whatever you want until then.

I have not tested this item.

Reverting my test result because I have investigated deeper. Will post more details later. For now just reverting my test so the PR goes back from RTC to pending so it is not merged.

Back to pending.

I have tested this item ? unsuccessfully on 76c52d0

TEST 3: Use the Full Package TAR after changing its extension to .zip fails on all browsers on all OS which I have tested.

The reason is that browsers do not really determine the MIME type for an input field of type "file" by the file's content. They set the mime type according to the file name extension, at least if there is one. So it you rename the tar.gz or tar.bz2 or any other file like e.g. a GIF image file to a zip file, you get 'application/x-zip-compressed' on Windows and 'application/zip' on Linux (or vice versa, don't remember now). And if you rename the file to something else with a different extension, it will fail the check for the extension and so not run into the check for the type. So the check for the file.type in this line here is not doing anything: https://github.com/joomla/joomla-cms/pull/38707/files#diff-550b2e9757e1fef80e3714191e4f70f3dd9bd7441ff3e7679698de031551e346R24 .

When I had noticed that first, I thought it was just a known limitation by a broswer, so I thought it was accebtable, but further investigation has shown it doesn't depend on the browser or the OS.

That's why I have to correct my test result to a negative one.

The right fix would be to change the description of the PR by removing the "Make sure the user is uploading a ZIP file based on the file type reported by the browser." point of the client-side checks, removing TEST 3: ... from the testing instructions, and removing the || (file.type !== 'application/zip' && file.type !== 'application/x-zip-compressed') from the line of code mentioned above.

As the author of this PR has stated that he won't reply to any comments and not make any changes, I don't know what to do now: Force change this PR against the will of the author, or close it and replace it by a new one?

I won't have time to work on it tomorrow, I will try to take a look on Wednesday.

I definitely don't want to see Joomla pulling in Yet Another over-engineered, third party JavaScript library to do something that simple, quite possibly cocking up the chunked download in the process. Core updates are too important and affect everyone, including myself.

I won't have time to work on it tomorrow, I will try to take a look on Wednesday.

That would be really much appreciated.

I definitely don't want to see Joomla pulling in Yet Another over-engineered, third party JavaScript library to do something that simple, quite possibly cocking up the chunked download in the process.

I fully agree. We should not use such kinds of third party libraries.

For now it would maybe be the best just to remove that type check like I suggested in my test result.

The PR still would keep people from accidentally uploading the wrong file.

Anything else, protecting against sabotage by people changing the file name extension of files, is a different task.

I got really sick, I could not take a look as I promised. The fun life of parents when the weather turns cold. Sigh. Hopefully sometime between Christmas and New Year's I will have the time and not be sick.

@richard67 I removed the MIME type check, replacing it with a signature-based ZIP file type detection which is far more reliable. I was able to successfully follow all of the tests in the PR like that on macOS. Tests with more Operating Systems are always welcome.

@nikosdion Thanks. I will test later today or tomorrow. If you would unblock Brian, he could test, too. As long as he is blocked for commenting here, he can’t submit a test result with the issue tracker.

The only people I have blocked on my personal GitHub account are those who have chronically treated Joomla contributors and/or me personally with disrespect and contempt. Interacting with them is unproductive and toxic.

Nobody has been blocked in the heat of the moment. All of them have been given multiple chances, over several years, before being blocked.

I would only unblock someone if I receive an apology and the person commits to doing better. I have plenty of contact avenues, even for people who do not know my email address. My sites are linked to my GitHub profile and they both have a contact form.

Ultimately, it's the project's responsibility for enforcing its Code of Conduct and protecting its contributors from toxic people. Asking me to unblock toxic people and forcing me to interact with them doesn't make me feel safe contributing to this project.

@nikosdion Sorry, it was me only. Blame me but not the project. I was not speaking for anybody else.

@richard67 No problem, I understood. I was just stating my point of view in more general terms so it's clear where I stand on the matter. Merry Christmas!

@nikosdion I have just tested, and I've observed that it needs to adjust the testing instructions. Now with the last change for the zip signature check, I get for test "TEST 3" and "TEST 5" the message about wrong format and not the one about wrong package type. That means the expected results of these 2 tests should just be changed back to like they were before, i.e. the stroke through text is right again. Could you do that or shall I do it for you?

P.S.: For completeness it would also need to add a 7th test with the full package zip renamed to the name of the update package zip so file type and extension are right but there is an installation/index.php inside.

P.P.S: The result of that test is the special message for this case, I've just tested that with success.

Yes, the test instructions do need an update as you said. Can you please do that? I'm helping my girls bake Christmas cookies atm :)

@nikosdion I will update them and post my test result in some 30 minutes. I wish you a merry Christmas. Hopefully you won't need to give a GDPR consent on every cookie your girls are baking.

Appropriate consent was given by all data subjects :D Merry Christmas! ???

Is there any interest in this PR or did I just waste my time?

Is there any interest in this PR or did I just waste my time?

if it's not taken by 4.3 (which I think it's too late) and 4.4 (which doesn't get new features, exceptions are possible by production I think) I would talk it for 5.0.

If it's going to 4.3 please let me know a.s.a.p. as it'll need rebasing. I have some time now but if you ask me after Easter things are getting a bit hairy.

Hi Nicholas @nikosdion, we are interested by the PR, but it's too late for 4.3, we froze beta 4 and release it on Tuesday and jumping into RC a week later. I would love to see this included into Joomla, as the uploads are prone to mistakes. Allon @laoneo, any thoughts?

I would love to see this one in to the 4.x series. As 4.3 is in RC phase, can you rebase this one to the 4.4.-dev branch?

Will do!

When this PR was made I first was very much in favour of it because I want the CMS to protect people from accidentally using the wrong file for upload & update.

But meanwhile I think it has several weak points:

1. In past, when I have downloaded nightly builds for testing purpose, I have changed their name by adding a suffix to the base name with the date when it was built, e.g. Joomla_4.2.9-rc2-dev-Development-Update_Package_2023-03-08.zip. When this PR will be merged, this is not possible anymore. I will always have to rename them again and remove that suffix before I can use them.

2. The PR does more than it should do. It shall protect people from accidentally uploading the wrong file, but the TEST 6: Rename the Full Package ZIP to the name of the Upgrade Package ZIP and use that renamed file is more than that. Such a renaming of the file would never be made by accident but by purpose. Catering for such hacking attempts make the code in this function unnecessarily complicated and so hard to maintain: https://github.com/joomla/joomla-cms/pull/38707/files#diff-617530abeea53181b5e5f641ba3906dcbf758238861e8a5281422722533d8532R1728 .

3. The code mentioned with point 2 executes after the zip has been uploaded. This is not really nice if the upload takes longer on a slow connection.

All in all, for the reasons stated with points 2 and 3, we should only do a check for the right file name in the javascript and use a regex which allows to add any kind of suffix to the base name.

This would be sufficient to protect the user from choosing the wrong file by accident, which by the way can be any file, not only the full package, The user might by accident select any file from the same folder, and this may be a gif or jpeg or txt file or whatever.

Anything else beside a check for the name in the js would be trying to protect the user from hacking themselves, or trying to protect from hacked zip files, but this would be easily to circumvent with this PR. The hacker only has to give it the right name and make sure it doesn't contain an installation/index.php. So this PR will not really protect from hacking, and as said, for protecting from mistakes it simply does too much.

if we didnt generate the useless update tar files it would be even easier and simpler

@richard67 Please start re-reading the original issue from here #38702 (comment)

Then read the history of my commits and this PR.

As you can see, my original implementation was to check if it's a ZIP file and if it has the installation folder in it, as I had already explained. More to the point, I was AGAINST matching filenames.

I added the filename check because you were all stuck on it as being a necessary feature.

Do remember that this was never about hacking yourself. It was about preventing the upload of a. a tar file which cannot be used; and b. the full installation package which leads to a broken site as soon as the first files of the installation folder are extracted.

The server-side check is only meant as a failsafe, not as the primary method of preventing the wrong upload. It's the "what if, somehow, that JavaScript failed to execute for any reason we cannot possibly imagine" failsafe. All client-side checks MUST have a server-side equivalent.

So, you are all coming back to what I was saying back in September, when you were insisting I am wrong. At which point do I get an apology? I am waiting…

So, you are all coming back to what I was saying back in September, when you were insisting I am wrong. At which point do I get an apology? I am waiting…

what did I insist you did? i must be missing something.

This is an issue which has existed for years. It wasnt until I realised the commonality in a series of forum posts that I created the original issue and then brought it to the attention of a variety of people to get their input in an open collaborative manner.

@brianteeman Why are you replying to this? I was not talking about you. I was talking about the maintainers, at-mentioning Richard.

I added the filename check because you were all stuck on it as being a necessary feature.

@nikosdion Where haven’t done that? The only change I had suggested besides code style was the removal of the not working mime type check.

@richard67 To clarify, Harald did it: #38702 (comment)

would check for the correct filename on upload, if people thinks they have to rename it then they should rename it that the update can understand for example:
Joomla_4.2.2-MYCUSTOMBUILD-Stable-Update_Package.zip or something like that

I was replying to you because you are the one who commented that the filename check is the wrong approach. I agree with that! I said so myself. Nobody in Production accepted this point in September (or at least nobody said so, which is the same for all intents and purposes), therefore my contribution had to make that change to be in line with the opinion expressed by Production.

Just to make it clear, it doesn't matter if a request for a feature or feature change comes from the department coordinator, a release lead, team lead, or team member. You are all holding an official position in the Production department. You speak for the project. You hold the power to approve or reject a contribution. When you say "jump" us volunteer contributors ask "how high", not "why".

Furthermore, you can't pretend the buck stops with the release lead because, as we've seen here, the release lead can simply ignore a PR they don't want to take responsibility for and kick it down to the next release lead. Not maliciously, but because they are overworked and, frankly, no matter what they do people are going to yell at them.

So here we are, finding myself again in the unsavoury position of having to ask for the equivalent of a hot-fix to a broken process: @laoneo @HLeithner and @bembelimen please hand me a PLT edict which describes what should and should not be in this contribution and I'll stick to it.

This pull request has been automatically rebased to 5.0-dev.