Project properties - mp3 upload problem

[FayCross]

When trying to upload an mp3 file via the media & quota tab of the project properties panel I get the error 'Upload: Invalid file type - audio/mp3'. The same mp3 files will upload ok via the editor & there's nothing odd in the file names.

I can upload other file types via the project properties window without any errors - it seems to be just mp3s that won't work. Any ideas?

[torinfo]

@FayCross What version is this on? I merged in a pull request of John Horne yesterday dealing with mime types. It should not have changed functionality (by default the more thorough check is switched off). But it is only on develop.

[FayCross]

UoN install is v3.5 & it's also a problem on my xampp version running develop code but about a week behind so isn't caused by John's changes

[ronm123]

@FayCross @torinfo I just pulled down the latest develop and tested this and sure enough as reported by Fay upload of mp3 via properties doesn't work. However I've tested this via xampp on a long standing develop install rather than a recent new install so I'm not sure if the default mime type list has changed in more recent install sql?

Basically the mime type list in the develop installation I've tested had audio/mpeg, but not audio/mp3. Adding the latter enabled mp3 to be uploaded via properties. However two issues/questions with this:

1. trying to add audio/mp3, to the list via management failed so I had to add this directly to the database.

2. I know this is an omission that we want to rectify but if a mime type is excluded from this list via management, that is to say deliberately, shouldn't the same file type be excluded via the editor too? Not sure if the changed John has made has impact on both upload methods?

[ronm123]

@FayCross @torinfo ok quick update...

1. the change via management wasn't working because although I had pulled down latest develop I hadn't re-run upgrade.php. After running that I can add remove audio/mp3 via management.

After running upgrade I now see a new text area via management labelled: Whether the MIME file type check should be applied to file uploads This has a default value of False and is greyed out saying The MIME check requires the PHP 'mime_content_type' function. However adding/removing audio/mp3 does control the ability to add mp3 via properties so is this about checking uploads via the editor too? Is the 'mime_content_type' a php extension?

[ronm123]

@FayCross @torinfo ok another update:

Via xampp extension=php_fileinfo.dll has to be enabled for the PHP 'mime_content_type' function to be enabled.

I changed that and was then able to change the option from false to true in management but that didn't seem to have any impact on the file types that can be uploaded via the editor and the check on mime types was working via properties regardless of this true/false value.

[ronm123]

Ok just read an earlier note from John:

This collection of commits allows the admin user to enable or disable the MIME type file check (via a new field in the database). The current list of allowed mime types in the database is also augmented a little bit (when upgrade is run). By default the check is disabled for backwards compatibility. An option for this appears in the admin interface (under the server section). It will be greyed out if the relevant php/mime function is not present. At the moment this check is not run as the code to run it has yet to be committed to github/xerte.

[ronm123]

Looking at the notes and changes from John upgrade.php does add audio/mp3 to the default list of allowed mime types. (Can't believe that wasn't already included!)

I'm not sure if the upgrade code detects whether someone has already edited the default list e.g. I could see some organisations wanting to prevent direct upload of video files in promoting use of a streaming server instead. So if someone had edited the default list to exclude mp4 and/or other video types before this upgrade does re-running upgrade.php add those back in? I know it doesn't once it has been run once because it triggers the upgrade number but what about the first time it is run?

Also after looking at this do we need to discuss the default list of mime types and what should/shouldn't be included in the defaults?

[FayCross]

Thanks for looking into this @ronm123. I've added mp3 via the management pages & it's working now.

As you say, we probably do need to look at the default list & stop users uploading types that shouldn't be allowed through the editor

[jayaich]

Hi, Just noticed this issue. The original 'mimetypes' list in the database did not include 'audio/mp3'. We had it as a local patch for our users, but it went into the updated list (via the mimetypes check pull request) because I could see no reason for not allowing it. However, the code (line 24 in website_code/php/import/fileupload.php) referenced the mimetypes list directly from the DB - and it currently still does. Hence uploads with audio/mp3 type would not work by default. I did not notice this till later, and after the pull request for updating the mimetype check had been merged. That is, file uploads of audio/mp3 type will still fail unless modified via the admin user interface (and hence the DB). I have corrected the code in 'fileupload.php' but it is in the 'file_ext_check' repository which is used by the 'file extension check' pull request. The new code only executes the mime type check (in fileupload.php) if the relevant check has been enabled in the admin user interface. Hope that makes sense! :-)

[jayaich]

@ronm123

I'm not sure if the upgrade code detects whether someone has already edited the default list e.g. I could see some organisations wanting to prevent direct upload of video files in promoting use of a streaming server instead.

Yes, the upgrade change only 'adds' new mime types if they are not already present. It does not 'reset' the mime list back to some default. In terms of video types upgrade will add 'video/mpeg', so someone wanting to remove all 'video' types would need to modify the list after upgrading.

We could modify 'upgrade.php' to say which types were added whilst the upgrade was running. That way at least the user could see if they needed to change the list afterwards. It should be easy enough - let me know if you want that done.

[ronm123]

@jayaich thanks for the clarification. Displaying what types were added during upgrade sounds like a good idea. Not sure what @torinfo and @FayCross think about that?

I think its probably a good idea to include this and other upgrade/management related topics for discussion during our next online meeting on 18th October. Hope you can join us for that John?

[jayaich]

@ronm123 Meeting 18 Oct - sorry, but we have an all IT staff meeting (90mins) that afternoon. Missing these things is not recommended :-)

[JohnSmith-LT]

This is fixed right? I am able to upload mp3svia both media and quota and via editor just fine.