Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "accept" attribute to the upload file input element of com_joomlaupdate's Upload & Update tab #29877

Conversation

richard67
Copy link
Member

@richard67 richard67 commented Jul 1, 2020

Pull Request for Issue #29763 (partly).

Summary of Changes

This Pull Request (PR) adds the "accept" attribute to the file field of the Joomla Update Component's Upload & Update tab so that only zip files with mime type "application/zip" are selectable.

Only zip because currently the Joomla Update Component only supports that packing format for Upload & Update, see also the discussion in comments below. No idea what the other update packages (tar.gz, tar.bz2) are good for. There is no update from folder option or update channel for which they could be used.

Important: This is NOT a security fix, it only shall make it harder to accidently select the wrong file for upload.

See the following description on https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/accept:

The accept attribute doesn't validate the types of the selected files; it simply provides hints for browsers to guide users towards selecting the correct file types. It is still possible (in most cases) for users to toggle an option in the file chooser that makes it possible to override this and select any file they wish, and then choose incorrect file types.

Because of this, you should make sure that expected requirement is validated server-side.

I will work on these server-side validations and provide a separate PR.

Browser support see https://caniuse.com/#feat=input-file-accept.

Testing Instructions

  1. On a clean current staging or recent 3.9 nightly build or a 3.9.19, login to backend and go to "Components -> Joomla Update".

  2. Go to the "Upload & Update" tab and use the button right beside "Joomla package file" to select a file for upload.
    Result: See section "Actual result BEFORE applying this Pull Request" below.

  3. Apply the patch of this PR.

  4. Repeat step 2.
    Result: See section "Expected result AFTER applying this Pull Request" below.

Actual result BEFORE applying this Pull Request

A browser dialogue opens which allows you to select a file. It shows all kinds of files in the currently active folder. There is no filter for zip files only.

E.g. on Firefox 77.0.1 (64-Bit) for Windows:
j3-pr29877_actual

Expected result AFTER applying this Pull Request

A browser dialogue opens which allows you to select a file. Depending on your browser it limits the files being shown to zip files.

E.g. on Firefox 77.0.1 (64-Bit) for Windows:
j3-pr29877_expected

Documentation Changes Required

None, I think.

@richard67 richard67 marked this pull request as ready for review July 1, 2020 14:56
@Quy
Copy link
Contributor

Quy commented Jul 1, 2020

Update from Joomla! 3.9.18 .tar.bz2 | .tar.gz | .zip

You have to include the other 2 options found here:

https://github.com/joomla/joomla-cms/releases

@richard67
Copy link
Member Author

richard67 commented Jul 1, 2020

@Quy No. These don't work with Upload & Update. They are meant to be used for manual upload and unpack into an empty folder and use the folder update channel. Update: That was for the extension installer. The Joomla Update Component currently can't use tar.gz or a tar.bz2 files in any way.

@richard67
Copy link
Member Author

@Quy Just try it with current staging or 4.0-dev. In both cases update with a tar.gz or a tar.bz2 fails when using "Upload & Update".

@richard67
Copy link
Member Author

@Quy On J3:
snap-1

On J4:
snap-2

@Quy
Copy link
Contributor

Quy commented Jul 1, 2020

Thanks. Why offer those other formats?

@richard67
Copy link
Member Author

Thanks. Why offer those other formats?

This is a question someone else has to answer, but it is a good question ;-)

@Quy
Copy link
Contributor

Quy commented Jul 1, 2020

I have tested this item successfully on f30a197


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29877.

@degobbis
Copy link
Contributor

degobbis commented Jul 1, 2020

I have tested this item successfully on f30a197


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29877.

@zero-24 zero-24 added this to the Joomla! 3.9.20 milestone Jul 1, 2020
@zero-24
Copy link
Member

zero-24 commented Jul 1, 2020

Merging here thanks @richard67 and @Quy and @degobbis for testing.

@zero-24 zero-24 merged commit 8273a92 into joomla:staging Jul 1, 2020
6 checks passed
@richard67
Copy link
Member Author

Yes, thanks @Quy and @degobbis for testing.

@richard67 richard67 deleted the staging-use-accept-attribute-for-file-input-joomlaupdate branch July 1, 2020 20:32
@Quy Quy removed the PR-staging label Jul 2, 2020
@joomla-cms-bot joomla-cms-bot removed this from the Joomla! 3.9.20 milestone Jul 2, 2020
@Quy Quy added this to the Joomla! 3.9.20 milestone Jul 2, 2020
@joomla-cms-bot joomla-cms-bot removed this from the Joomla! 3.9.20 milestone Jul 2, 2020
@Quy Quy added this to the Joomla! 3.9.20 milestone Jul 2, 2020
Reconix pushed a commit to Reconix/joomla-cms that referenced this pull request Aug 31, 2020
Reconix added a commit to Reconix/joomla-cms that referenced this pull request Aug 31, 2020
Reconix added a commit to Reconix/joomla-cms that referenced this pull request Aug 31, 2020
Reconix added a commit to Reconix/joomla-cms that referenced this pull request Aug 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants