Define file upload type by sample

kkarpieszuk commented 2 years ago

It was reported here and there and today I got down to investigate the issue: many people are reporting even though they registered custom file type to be "uploadable" the upload still fails.

After long investigation I found different tools reports different mime type for the same file.

I checked some .tar.gz file and it's mime type is:

in online tool https://www.htmlstrip.com/mime-file-type-checker it is application/x-gzip
in linux command line xdg-mime query filetype arch.tar.gz it is application/x-compressed-tar
if I run PHP script which is the same as in WP function wp_check_file_type_and_ext() then it is application/gzip (the script below)

Users don't know which mime type they should specify when they are defining their rules in the plugin (and it should be the one which WordPress uses, the last, the script I wrote below is code I extracted from WP function to check mime type, see \wp_check_filetype_and_ext)

My propositions, two approaches:

Idea 1: More descriptive "file upload failed" messages

I propose to modify all "file upload failed" messages to show the reason in case it was mime. Something like "file upload failed, mime blah/x-blah is not supported.

User, when see such info, will go to file upload types configuration and will adjust his definitions to cover such mime.

Drawback: It is probably not possible t modify all error messages as they can come from different places:

when user uploads file to standard Media library, WP will print the error
when user uploads file with WPForms, WPForms rpints the error (side note: user will see it, not site admin so site admin will not be aware there is any issue; we can overcome this problem by adding some error logging)
any other plugin can have their own logic to handle failed upload

Idea 2: Add file type tester (or allow to define file upload type by sample file)

Now the site admin defines allowed custom files by manually typing the extension and mime type. My idea is to add below that form some row with label "Define allowed file type by uploading sample file" and button to upload file. When site admin uploads file, we could define fetch its extension and mime type (with the script at the bottom of this post) and automatically fill extension and mime type in the form which we already have)

Bear in mind, we don't need to save that file anywhere, script which will be processing the file will just check extension and mime and do not proceed. So it is safe (I am not 100% sure of that)

Drawback: Actually none (except security concerns). It will need some coding, some UI design work but we always do the right things

p.s. just a hint: we can apply both ideas at the same time ;)

checktype.php:

<?php
$finfo     = finfo_open( FILEINFO_MIME_TYPE );
$real_mime = finfo_file( $finfo, $argv[1] );
finfo_close( $finfo );

echo $real_mime . PHP_EOL;

How to use this script:

place the file checktype.php on your server
add execution chmod for the file chmod +x checktype
place in the same directory the file you want to check mime (for example font.otf)
run in console ./checktype.php font.otf

kkarpieszuk commented 1 year ago

@ClassicLauren that is the ticket about problem with mime types. It does not have any real solution yet as it requires some discussion how to solve it.

cheh commented 1 year ago

@kkarpieszuk the second approach looks good. But before implementing it (I'm still thinking about it), I'm confused about the reported issue related to uploading JSON files - https://github.com/awesomemotive/file-upload-types/issues/61#issuecomment-1385611191.

According to the provided PHP script, my test JSON file (one of the imported form) is text/html. But I wasn't able to upload this file - https://a.supportally.com/v/a8Bkn9

Could you please investigate it?

kkarpieszuk commented 1 year ago

@cheh you have text/plain defined for .html files but not for .json files.

WordPress is checking it the whole pair: for .json files it looks for mimes defined for .json files and you don't have such mime for it

cheh commented 1 year ago

@kkarpieszuk oh, I see. So if one of JSON mime types was text/plain, it would work, right? Actually, the same reason for OTF - there is missed application/vnd.ms-opentype.

I see the support team usually uses and suggests the web tool that you mentioned - https://www.htmlstrip.com/mime-file-type-checker, and it works for my cases. But it seems it doesn't work properly for .tar.gz file. Is it a reported issue? Or you just decided to check on this file created by yourself?

My concern is how much time you need to implement the second approach. And looks like Matt will need to be involved. Maybe the QA team as well, but I am not sure.

Do we maybe have not relevant mime types collection, and should just update it? So, I'm considering updating the current file-types-list-v2.json file and putting correct mime types where needed:

.json - [ ..., text/plain ]
.otf - [ ..., application/vnd.ms-opentype ]
etc.

Please, let me know your thoughts.

kkarpieszuk commented 1 year ago

and suggest this web tools that you mentioned - https://www.htmlstrip.com/mime-file-type-checker, and it works for my cases.

It always works like that:

You have a file which you want to check
You use some tool to check: online, local CLI command, PHP script, Python script
The tool replies like "according to my mime types database* defined on my system, this is mime foo/bar"

*) I call it database, it is just some data stored on system, idk what is the format

The point 3 is the most important.

If this online tool is running using a tool which has the same records in its database that the server where your site with WordPress is running, then that online tool will be working for you (will give the same results).

But if it has different data, then it won't work.

I just checked some .json file on my system:

konrad@konrad-HP-Laptop-15s-eq1xxx:/var/www/html$ php info.php cookiecutter.json 
application/json
konrad@konrad-HP-Laptop-15s-eq1xxx:/var/www/html$ xdg-mime query filetype cookiecutter.json 
application/json

and that online tool says it is text/plain

Do we maybe have not relevant mime types collection and should update it?

@cheh we could but that does not solve the problem completely.

Problem 1

We don't know all possible mime types and it means every now and then we will have to update that list again and release a new version of plugin. I already did this for some MS files (merged but not released yet) https://github.com/awesomemotive/file-upload-types/pull/40/files and we can expect we will be doing such PRs often

Problem 2

Users also have problem with files which are not in our assets/file-types-list-v2.json

They go to plugin settings screen
They choose option to add a new supported file
They specify file extension and mime
-> it does not work for them because it is not the mime which system expects

cheh commented 1 year ago

Thanks for the details, @kkarpieszuk!

Could you please estimate how much time you need for this checker implementation? Will you need a mockup?

Also, I'd ask you to prepare a separate small PR that partly resolves #61 - update the assets/file-types-list-v2.json file.

kkarpieszuk commented 1 year ago

Hi @cheh what have to be done:

GUI mockup or at least some decisions how it should look and behave. We probably should consult it with @mattbrett . I can't estimate that part.
time to implement the GUI, at most one day, I think (depends on design)
some logic to upload file with ajax must be written. At most it would take one day
the logic to check file mime type is already written so a few minutes
time to test and review

mattbrett commented 1 year ago

@kkarpieszuk These messages are just being displayed in the admin? Are we altering the standard WP messages or adding a secondary message? In either case, I think we should simply use standard WP notices, but please let me know if there's something more that's required.

kkarpieszuk commented 1 year ago

These messages are just being displayed in the admin? Are we altering the standard WP messages or adding a secondary message?

HI @mattbrett , I am not talking about the design of the message (they can be outputted in a different places in wp-admin and front-end be coming from different plugins or wp core).

Please check here:

https://a.supportally.com/i/sSAF19

This is File Upload Types configuration screen. At the bottom (a bit over the red rectangle) we already have 3 fields where user can add a new file type by specifying File description, MIME Type and Extension

I would like to add - in the place of red rectangle (or any better place which you would suggest) - additional method to add a new file type. User should see here "Upload file" button and when he uploads a file, File description, MIME Type and Extension automatically are filled (file is checked what extension it has and what mime type).

What I need is:

where to place the button: in the place of red rectangle | to the left/right of the row above | any other place
what label should we add and where. I am not perfect in English, I would propose "Define custom file type by sample" but idk if this is ideal.

mattbrett commented 1 year ago

@kkarpieszuk Nice idea to solve this problem! Unfortunately, the design team is bogged down at the moment with a couple of very large, high priority projects. I've been told we'll circle back to this when we have more bandwidth.

kkarpieszuk commented 1 year ago

So, I'm considering updating the current file-types-list-v2.json file and putting correct mime types where needed:

@cheh heads up: I am doing this but in the span of #15 ticket which is exactly about this.

Edit: #15 is very big task and I don't want to postpone upcoming release. I will add only extensions mentioned here and in #55 . For this I created #62 ticket and will handle it there

kkarpieszuk commented 1 year ago

note to myself: in this clask thread https://awesomemotive.slack.com/archives/C03CUEEDX3Q/p1676288733476539 me and @cheh found the result of proposed script might be different if script is run form command line or in-broswer.

But it does not affect the idea: script will be run in browser so will be returning the same result when checking mime as Wordpress gets.

for the reference, this is script which can be used in-browser (place it in the path accessible by browser, place file you want to check next to it and execute cript in browser):

<?php

$file = glob( '*' );

if ( ! $file ) {
    echo 'No files found.' . PHP_EOL;
    exit;
}

foreach ( $file as $f ) {

    echo 'checking ' . $f . '...<br>';

    $finfo     = finfo_open( FILEINFO_MIME_TYPE );
    $real_mime = finfo_file( $finfo, $f );
    finfo_close( $finfo );

    echo 'the mime type is <pre>' . $real_mime . '</pre>' . PHP_EOL;
}

kkarpieszuk commented 1 year ago

@cheh just to proove myself it might be working, I created a proof of concept (code is ugly and a lot of things are hardcoded, s don't read it much)

https://github.com/awesomemotive/file-upload-types/pull/64

@mattbrett this is how it works https://d.pr/v/g4H8Qv I hope now it would be easier for you to get what I am asking for here :) That "Sample" button is most likely in wrong place, with wrong description, but video shows what it does.

mattbrett commented 1 year ago

Thanks @kkarpieszuk! Nice prototype and this will solve a lot of headaches for people. I have some ideas on how we should implement this, but not sure when I'll be able to get to it. I'll add it to the design queue and will follow-up when I have an ETA for you. Thanks for your patience!

kennymacharia commented 1 year ago

Hey @kkarpieszuk just noting this down here that we have one additional report of this issue here: https://wordpress.org/support/topic/wif-files-no-longer-supported/ WIF files don't seem to work and all the suggested MIME types don't seem to work.

kkarpieszuk commented 1 year ago

and we have one more report about this here #67

mattbrett commented 1 year ago

@kkarpieszuk Sorry it's taken me so long to get to this. Between Payments v2 and Easy WP SMTP, I haven't had a whole lot of time for other projects. Anyway, I've mocked up a solution that I believe would be the best approach. Basically, making file upload for automatic detection the default behaviour, and manually adding file types secondary. If we could do a drag-and-drop file uploader, that would be even better. Please have a look at this screencast I've prepared, showing my concept. If you'd like to take a closer look, you can jump into the Figma file.

kkarpieszuk commented 1 year ago

I love it Matt, completely doable. I will check with team when I can dedicate some time and finally implement it that way.

mattbrett commented 1 year ago

@kkarpieszuk Awesome! I've made a bunch of other changes to the colour scheme to bring it more inline with WP standards. Just CSS changes. I've love to have those taken care of as well if you have time. I'll finish the mockups and add the right column. We should update the brands we're promoting to include all WPForms Collective products.

mattbrett commented 1 year ago

@kkarpieszuk Just a quick follow-up to let you know that I've finished the refresh. I made a bunch of changes across the board. Basically just cleaned things up and used WP native colours instead of our custom colours. I also updated the sidebar to reflect mostly our own products. Here's a link to the design in Figma. And I recorded a screencast to go over all of the changes.

kkarpieszuk commented 3 months ago

@joannafoss could someone from content team provide a new message for the tooltip?

Now it says:

Enter the description, MIME type and extension of the file. Multiple MIME types for a single extension can be separated by a comma.

It refers to the old UI where we had only the fields to provide description, mime type and extension manually. We should mention in some way the fact now the main way to add custom file type is by dragging/selecting the example file into this area and optionally, the user can still provide it manually. I presented above on the screenshot interface with lower panel open after clicking on the link inside dropzone, but after page load it looks like this:

If something is unclear I suggest to check Matt's figma design and screencast in comment above

kkarpieszuk commented 3 months ago

I am working on the File By Sample implementation in #64

The part about a deeper redesign I will handle separately in #75

joannafoss commented 3 months ago

@mattbrett could you please supply updated text for the tooltip message here?

mattbrett commented 3 months ago

@kkarpieszuk Does this work? I don't think we need to mention in the tooltip that files can be added manually, since that's explained below.

Upload files and have their MIME type detected automatically. Multiple MIME types for a single extension can be separated by a comma.

kkarpieszuk commented 3 months ago

thanks Matt, changed

awesomemotive / file-upload-types