#45615 closed defect (bug) (fixed)
CSV Mime Type fails upload
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.0.3 | Priority: | normal |
Severity: | major | Version: | 5.0.1 |
Component: | Upload | Keywords: | has-unit-tests |
Focuses: | Cc: |
Description
CSV is an allowed upload type, however since WP 5.0.1 the mime type is now checked against the file. CSV files can however have text/csv as well as text/plain mime types, WP only caters for the first.
Attachments (11)
Change History (75)
#3
@
6 years ago
Has mime type checking been retrospectively patched into older versions of WP?
Two sites running 4.8.8 and 4.9.9 both seem to have this issue
#4
@
6 years ago
Yep, as part of the security release. See https://make.wordpress.org/core/2018/12/13/backwards-compatibility-breaks-in-5-0-1/
#6
@
6 years ago
- Keywords has-unit-tests added; needs-testing removed
45615-2.diff adds a test with a sample.csv
with sample data from https://www.w3.org/TR/2016/NOTE-tabular-data-primer-20160225/
This ticket was mentioned in Slack in #forums by ipstenu. View the logs.
6 years ago
#9
@
6 years ago
- Component changed from Filesystem API to Upload
- Milestone changed from Awaiting Review to 5.0.2
This ticket was mentioned in Slack in #forums by clorith. View the logs.
6 years ago
#13
follow-up:
↓ 14
@
6 years ago
for those in need of a solution NOW, like, everyone, because, well, we DEPEND on this;
https://gist.github.com/rmpel/e1e2452ca06ab621fe061e0fde7ae150
#14
in reply to:
↑ 13
@
6 years ago
Replying to clearsite:
for those in need of a solution NOW, like, everyone, because, well, we DEPEND on this;
https://gist.github.com/rmpel/e1e2452ca06ab621fe061e0fde7ae150
Thanks a ton! Please send feedback for those of you implementing this. Just tested on my staging server and it worked like a charm as an interim solution!!!
Thanks Team!!
#16
in reply to:
↑ 11
@
6 years ago
Replying to pento: Hi there! Are these actually available yet buddy?
#17
@
6 years ago
For those using or trying at least to use my mu-plugin option, please join the discussion on GitHub, I'll try to answer questions as quickly as possible.
@linuxhombr3 ; I'm not affiliated with any 'team' other than my co-workers, in any case not with the WordPress team.
This mu-plugin is not created, endorsed or maintained by Automattic, or any other WordPress affiliated development team other than myself and my co-workers at Internetbureau Clearsite.
This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.
6 years ago
#22
@
6 years ago
- Owner set to joemcgill
- Status changed from new to assigned
I'm assigning myself, but am coordinating with @tellyworth on this and any tickets marked as duplicates to ensure we cover as many reported use cases affected as possible.
#23
@
6 years ago
attachment:45615-tests.diff incorporates unit tests taken from some of the other tickets, plus a couple more, to cover various text/* file types. It also includes a couple of extra cases for misnamed files.
#24
@
6 years ago
I have a problem with XLS, mime file is correct: application/vnd.ms-excel but keeps saying "not allowed for security reasons"
No problem with XLSX.
WP Multisite (Directory): 5.0.2
#25
@
6 years ago
Getting different REAL MIME type from function finfo_file()
. Looks like this is the PHP version bug. PHP bug ticket id 75380 All other related of MIME.
Testing MIME of file with PHP Code:
echo 'PHP Version: ' . phpversion() . "<br/><br/>"; echo 'file.vtt | ' . mime_content_type( 'file.vtt') . "<br/>"; echo 'file.xml | ' . mime_content_type( 'file.xml') . "<br/>";
On Localhost:
PHP Version: 7.2.4 file.vtt | text/plain file.xml | text/xml
On Live
PHP Version: 7.0.32-4+ubuntu16.04.1+deb.sury.org+1 file.vtt | text/plain file.xml | application/xml
#26
@
6 years ago
@shodan01 mentioned that .gpx
also stopped working.
#27
@
6 years ago
As I understand it:
If we associate a given file extension to multiple mime types in wp_get_mime_types()
, then wp_check_filetype()
will only pick up the first match, because of the break in the foreach loop when a match is confirmed (src).
So having e.g.
'txt|asc|c|cc|h|srt|vtt' => 'text/plain',
before
'vtt' => 'text/vtt',
will mean that the .vtt
file type will only be expected to have the text/plain
mime type, regardless of other possible matches.
It also seems that finfo_file()
is not consistent determining the real mime type info depending on the system setup.
So simply assigning a file extension to multiple mime types in wp_get_mime_types()
will not solve the issue here. An actual support for multiple mime types or mime type "fallbacks" seems to be needed.
In the meanwhile here's a suggestion for a plugin workaround to support a fallback mime type of a given file extension:
/** * Support for 'text/plain' as the secondary mime type of .vtt files, * in addition to the default 'text/vtt' support. */ add_filter( 'wp_check_filetype_and_ext', 'wpse323750_secondary_mime', 99, 4 ); function wpse323750_secondary_mime( $check, $file, $filename, $mimes ) { if ( empty( $check['ext'] ) && empty( $check['type'] ) ) { // Adjust to your needs! $secondary_mime = [ 'vtt' => 'text/plain' ]; // Run another check, but only for our secondary mime and not on core mime types. remove_filter( 'wp_check_filetype_and_ext', 'wpse323750_secondary_mime', 99, 4 ); $check = wp_check_filetype_and_ext( $file, $filename, $secondary_mime ); add_filter( 'wp_check_filetype_and_ext', 'wpse323750_secondary_mime', 99, 4 ); } return $check; }
and it's possible to extend this further for multiple fallbacks, see here.
This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.
6 years ago
#34
follow-up:
↓ 36
@
6 years ago
attachment:45615.3.diff includes a mildly hacky fix for a few file types like CSV that are often misidentified by fileinfo as text/plain
.
It also includes a test and partial fix for #45633 - when coupled with the patch on that ticket I believe it should be fixed also. (The patch on that ticket alone did not fix the problem in my testing, see the unit tests in this diff).
This does not fix other file types that have been reported as problematic, like aac and fonts. I haven't yet been able to reproduce those, most likely because I don't have sample files that are specific enough. A similar approach to the text/plain
fix might work for those, but we need sample files in order to test.
This ticket was mentioned in Slack in #core-media by tellyworth. View the logs.
6 years ago
#36
in reply to:
↑ 34
@
6 years ago
Replying to tellyworth:
The approach in 45615.3.diff is ok for 5.0.3, and then we can follow with a broader refactor to make this less "hacky" as you put it. I don't think we should take this occasion to add support for .json files, even though the workaround for #45633 is mildly annoying for now. I've modified your approach slightly in that regard and to also add back support for vtt
files when they're reported as text/plain
by the server, which should fix #45622.
I think we can take a similar approach to add support for other miscellaneous types that are commonly misidentified.
#37
@
6 years ago
Any chance that the fix for 5.0.3 can also re-add gpx file upload support again? We are currently getting lots of tickets because of this issue. A possible workaround for us would be to change the registered mimetype in our plugin from
application/gpx+xml
to
application/gpx
but we would rather like to prevent this, as formerly uploaded gpx files would not be selectable via media library filter anymore for example.
#38
@
6 years ago
@harmr can you elaborate on this please? Your plugin is, what, filtering mime_types
to add 'gpx' => 'application/gpx+xml'
? At what point does that go wrong now, and how? Can you provide a minimal sample of a gpx file that we can test with?
#39
@
6 years ago
attachment:45615.5.diff moves the unit tests for vtt to a separate function, and adds tests for dfxp and json. Those types are unsupported by default, but can be added by filtering upload_mimes
- see test_wp_check_filetype_and_ext_unsupported_filtered()
for a trivial example. I think this is the right approach for obscure file types that don't need to be supported by core by default.
However, note that the test.dfxp and test.json cases currently fail their filtered tests, meaning adding those types to upload_mimes
via a filter is not sufficient to allow them. We'd need either less strict logic or specific exceptions for them in the text/plain block in wp_check_filetype_and_ext()
. I haven't found a simple solution.
#40
@
6 years ago
Hello,
Just to say that I tested 45615.5.diff on 5.0.2 with several different files and it's ok on my side. Thanks for the patch!
#41
@
6 years ago
When will this fix be available in the next core update? Is this slotted for version 5.0.3?
#42
@
6 years ago
For 5.0.3, I don't think we should remove vtt and dfxp files from the list of supported mime types. 45615.6.diff leaves those file types in the array of supported mime types, removes the tests requiring a filter for these types. I've also adjusted the json test to expect a text/plain
file type because that test was failing for me. I don't think we even really need that test for 5.0.3, though I agree with the idea to include tests for a suggested filter overrides for this functionality, generally.
45615.7.diff is the same as 45615.6.diff but removes the unsupported file type unit tests for now. I think this is a good iterative approach for 5.0.3 that fixes a few of the common errors for files being misidentified as text/plain
.
This ticket was mentioned in Slack in #core by webdevmattcrom. View the logs.
6 years ago
#45
@
6 years ago
@tellyworth: We use the following code to register the GPX mimetype in our plugin Maps Marker Pro:
add_filter('upload_mimes', array($this, 'add_upload_mime_types')); public function add_upload_mime_types($mimes) { $mimes['gpx'] = 'application/gpx+xml'; return $mimes; }
filtering for gpx files in the media library is added via
add_filter('post_mime_types', array($this, 'add_post_mime_types')); public function add_post_mime_types($post_mime_types) { $post_mime_types['application/gpx+xml'] = array( __('GPX tracks', 'mmp'), __('Manage GPX tracks', 'mmp'), _n_noop('GPX track <span class="count">(%s)</span>', 'GPX tracks <span class="count">(%s)</span>', 'mmp') ); return $post_mime_types; }
A minimal sample GPX file can be found at https://demo.mapsmarker.com/wp-content/uploads/2019/01/gpx-sample.zip
thanks a lot!
#49
@
6 years ago
45615.diff moves the new tests to the section that only runs when not in multisite, since these file types are not usually allowed in multisite.
This ticket was mentioned in Slack in #core by ocean90. View the logs.
6 years ago
#53
@
6 years ago
- Milestone changed from 5.0.3 to 5.1
WordPress 5.0.3-RC (and the final release, pending no further changes are needed) will contain a partial fix for these issues and we'll pick up additional fixes in 5.1. I'm updating the milestone to reflect this plan.
The current changes in r44443 should fix most cases where users were seeing text files like CSV, RTF, and VTT getting blocked.
#54
@
6 years ago
- Keywords needs-patch added; has-patch removed
- Severity changed from normal to major
In 5.0.3, will this be fixed for CSV product file imports in WooCommerce or not?
Also, WHEN is the release scheduled?
#55
follow-up:
↓ 56
@
6 years ago
- Keywords needs-patch removed
- Milestone changed from 5.1 to 5.0.3
- Resolution set to fixed
- Status changed from assigned to closed
@linuxhombr3: WordPress 5.0.3 is due to be released tomorrow.
I'm moving this ticket back to 5.0.3, we can open new tickets for followup fixes in 5.1.
#56
in reply to:
↑ 55
@
6 years ago
Replying to pento:
@linuxhombr3: WordPress 5.0.3 is due to be released tomorrow.
I'm moving this ticket back to 5.0.3, we can open new tickets for followup fixes in 5.1.
Great news. Thank you much and I'll be standing-by, considering I'm sure this has been battle-tested prior to release.
Thank you and maybe you guys can write a simple 'uploader unit tester' in PHP to attempt to use the CLI to upload CSVs as well as automate plugin-side imports so that if that fails you know you have an issue.
My two cents having required such a scope of testing for a platform as dynamic as WordPress.
Thanks!
@Linuxhombr3
PS: I'd be happy to consult.
#57
@
6 years ago
Unfortunately GPX upload is still broken with 5.0.3 - do I understand the comments right here that a broader fix is planned for 5.1?
This ticket was mentioned in Slack in #core-media by desrosj. View the logs.
6 years ago
#60
follow-up:
↓ 61
@
6 years ago
.m4a
still seems to be broken on WordPress 5.0.3. Trying to understand if this is being planned in a separate fix, or what the status is. Thanks for checking!
#61
in reply to:
↑ 60
@
6 years ago
Replying to arunsathiya:
.m4a
still seems to be broken on WordPress 5.0.3. Trying to understand if this is being planned in a separate fix, or what the status is. Thanks for checking!
I too have this question. I reported this with bug #45665, still not working.
#62
in reply to:
↑ description
@
6 years ago
Replying to Kloon:
CSV is an allowed upload type, however since WP 5.0.1 the mime type is now checked against the file. CSV files can however have text/csv as well as text/plain mime types, WP only caters for the first.
What is the status of this bug in relation to adding a filter for additional mime types? We're on 5.0.3 and we add .dwg and .skp files and these are still failing. New ticket for each mime-type now or what? Is there a way to easily see what mime type WP thinks it's seeing? It would be nice if the mime type were shown to the user in the error message...
Add csv to text/plain mime types