CSV files are prevented in PHP 8.1. due to wp_check_filetype_and_ext()

[bradshawtm]

finfo_file() gives different results for the same file in different PHP versions:

PHP 7.4: text/plain
PHP 8.0: application/csv
PHP 8.1: text/csv

The first two are currently allowed by WP, but the latter is not.

[SergeyBiryukov]

Hi there, welcome to WordPress Trac! Thanks for the ticket.

Just noting that this previously came up in comment:49:ticket:50913, and [49049] / #50913 added application/csv to the list of supported types in wp_check_filetype_and_ext(), so that it works as expected on PHP 8.0.

While the text/csv type is technically allowed as of [44438] / #45615, it looks like the function may need a new conditional to check for it properly, similar to the one added for application/csv in [49049].

The unit tests might need some updates too if there is currently an issue with text/csv that they don't catch.

[bradshawtm]

While the text/csv type is technically allowed

Yes, it's recognised in the code as a valid mime type, but the piece that's missing is that it needs a similar check to the one added in [49049], line 2947, since finfo_file() gives text/csv in PHP 8.1 and thus doesn't match any of the other checks.

Thanks!

Include text/csv to the list of supported types in wp_check_filetype_and_ext().

Track ticket: https://core.trac.wordpress.org/ticket/58094

[oglekler]

It looks like a pure bug fix but still needs a unit test, it can be added to data provider data_wp_check_filetype_and_ext(), but it can require checking PHP version.

[felipeelia]

I tried to reproduce the problem but I could not (sorry if I missed something :-) )

Summarizing my findings, in wp_check_filetype_and_ext() we deal with two mime types: one from wp_check_filetype() and another from finfo_file().

As @bradshawtm pointed out, finfo_file() will indeed return 'text/csv' but so will wp_check_filetype(), and adaptations are only needed when those two don't match.

Happy to work on the unit tests if I missed something and can start reproducing the problem.

[oglekler]

Due to lack of unit test update and proper testing, I am moving this into 6.4 milestone.

[SergeyBiryukov]

Replying to felipeelia:

As @bradshawtm pointed out, finfo_file() will indeed return 'text/csv' but so will wp_check_filetype(), and adaptations are only needed when those two don't match.

Thanks for testing! Same here, I can't seem to reproduce the issue.

In my testing on PHP 8.1+:

• Both finfo_file() and wp_check_filetype() return text/csv. The function then proceeds to check whether the type matches the real type​, and whether it is allowed. Both of these conditions are satisfied, so the correct values are returned (the proper_filename key is not relevant here, it's only used for images):

  array(3) {
    ["ext"]=>
    string(3) "csv"
    ["type"]=>
    string(8) "text/csv"
    ["proper_filename"]=>
    bool(false)
  }
  

• This is confirmed by a unit test for wp_check_filetype_and_ext(), which has a test case for 'text/csv'​ that currently passes on PHP 8.1+.
• This can also be confirmed by uploading a CSV file in the admin, which works as expected in my testing.

Note that text/csv is an allowed type as of WordPress 3.0: [13962] / #12757.

So I don't see any issues here at the moment. The steps to reproduce on a clean install would be helpful if any changes are still needed.

Thanks for the PR! Closing this for now, as the related issue cannot be reproduced, see comment 9 on #58094.

[hellofromTonya]

Changing the keywords as the patch was closed and the issue has not been reproducible.

• Removed has-patch as the patch was closed.
• Added reporter-feedback and needs-testing-info as the steps to produce are needed to continue investigating.

I did not add php-compatibility focus to denote an incompatibility, as currently the code seems correct and compatible.