Fix upload of .srt files

[afercia]

See #45615, #45622, [44438], [44439], and [44443].

Files with .srt extension are meant for video subtitles (captions), much like .vtt files. After the changes to make the mime type check stricter in WordPress 5.0.1 (backported to 4.9.9, etc.), uploading .srt files can fail because of mismatched MIME type check. Actually, .vtt can be served as text/plain depending on the server configuration.

Before WordPress 5.0.2, .srt files could be uploaded without issues.

For example, on a standard VVV install, the upload fails. Test file from the mediaelement-files GitHub repo:
​https://github.com/mediaelement/mediaelement-files/blob/master/mediaelement.srt

[dkarfa]

​https://github.com/WordPress/WordPress/blob/master/wp-includes/functions.php#L2763

[dkarfa]

True, I try in VVV and got
`“mediaelement.srt” has failed to upload.
Sorry, this file type is not permitted for security reasons.` - Error

[killerbishop]

I've tested this specific issue using vvv - from what I can tell - the mime type on this file according to finfo is text/html. Expectation from the wp_check_filetype_and_ext() is that it should be text/plain. It's also not in the array of choices in that call. Before pushing a patch - if something comes back as text/html with extension .srt - should this be allowed or is text/html purposefully ignored for security reasons here?

[killerbishop]

I would say this might be a part of this bigger looking issue: #40175

[itowhid06]

I was able to upload .srt files without any problem. screenshot attached.

[killerbishop]

@itowhid06 - I was able to get some SRT files to work as well. The issue is with specific SRT files, such as the one @afercia posted originally:

​https://github.com/mediaelement/mediaelement-files/blob/master/mediaelement.srt

This file is failing the check from fileinfo which is why I was thinking this is part of a bigger issue that was being mentioned in #40175. The bundled magic files with PHP apparently think that file is not a valid SRT - or at least this is true in PHP 7.3.

[afercia]

I think it depends on your server's MIME types configuration. Testing should first check which is the MIME type a .srt file is served with. Re: potential fix, see [44438] which just allowed .vtt files.

[killerbishop]

@afercia - I agree that the environment variable MAGIC can cause the system version to be used, but by default, it uses the built-in magic file of PHP. On my install MAGIC is not set, so my testing is against whatever the PHP version provides.

I updated to the latest upstream/master branch and tested the change from #44438 which is merged already. On my install of vvv, it is not functioning still with the file mediaelement.srt. For PHP 5.6.40 and 7.2.21, it returns the MIME type text/html which does not appear like it will fall into the changes from #44438. The likely reason that it is calling it HTML is the fact the SRT file in question actually has HTML tags in the content.

[killerbishop]

Patch for #47752 - includes fix and unit test

[killerbishop]

I've uploaded a patch for this along with a unit test that uses a copy of the file referenced in this issue. If text/html is detected as the MIME type and the file extension is .srt then it will be allowed through.

[azaozz]

If text/html is detected as the MIME type and the file extension is .srt then it will be allowed through.

Wondering how "safe" are the .srt files that contain HTML tags.

• What happens if a user downloads such file directly in the browser?
• Shouldn't handling of .srt files match handling of other text/html files?
• By default HTML files are not allowed. If WP needs an exception for .srt files that contain tags, how can we ensure they are "safe for use"?

It seems that we shouldn't allow .srt files that contain HTML tags. Moving to future release for further review/investigation.

[afercia]

Good point. Although I'm not sure I understand how this differs from the .vtt files case: they may contain HTMl as well.