New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix mime checkes #16091

Merged
merged 14 commits into from May 19, 2017

Conversation

@zero-24
Contributor

zero-24 commented May 17, 2017

Pull Request for Issue #16086

Summary of Changes

If you have installed 3.7.1 and try to upload using com_media a file which is not a image you got an error that you can't upload it.

I have now:

  • Improved the checks
  • show the failing mime to the use
  • make sure that images are checked with the image functions and other files with the other checks
  • implemented a fallback for images
  • fixed a longstanding issuse with finfo_open (FILEINFO_MIME vs. FILEINFO_MIME_TYPE)

Testing Instructions

  • install 3.7.1
  • try to upload a PDF file.
  • it fails
  • try to upload a jpg file
  • it works
  • apply this patch
  • try the PDF and the image again
  • it works now

Now go to the options and don't allow uploading jpg images and PDF files (by removing the jpg & PDF mime from the allowed mime types

  • try to upload a jpg again
  • it should not work but now show the failing mime type. (image/jpg)
  • do the same for a PDF
  • it should also show the failing mime.

Expected result

If allowed the upload should work if not it should show the mime we detected to the user.

Actual result

non image files can't be uploaded using the media manger

Documentation Changes Required

none.

zero-24 added some commits May 17, 2017

return false;
}
}
// We can't detect the mime type so it looks like a invalid image'

This comment has been minimized.

@Quy

Quy May 17, 2017

Contributor

Change a to an and remove '

return false;
}
}
// We can't detect the mime type so it looks like a invalid image'

This comment has been minimized.

@Quy

Quy May 17, 2017

Contributor

Change a to an and remove '.

return false;
}
// If we cant detect the mime try it again

This comment has been minimized.

@Quy

Quy May 17, 2017

Contributor

Change cant to can't.

@@ -199,10 +213,24 @@ public function canUpload($file, $component = 'com_media')
// If tmp_name is empty, then the file was bigger than the PHP limit
if (!empty($file['tmp_name']))
{
$result = $this->checkMimeType($file['tmp_name'], $component, true);
// Get the mime type this is a image file

This comment has been minimized.

@Quy

Quy May 17, 2017

Contributor

Change a to an. Add ; after type.

This comment has been minimized.

@Quy

Quy May 18, 2017

Contributor

Change.

@zero-24

This comment has been minimized.

Contributor

zero-24 commented May 17, 2017

Should be fixed now thanks @Quy !

@PhilETaylor

This comment has been minimized.

Contributor

PhilETaylor commented May 17, 2017

@rdeutz Joomla 3.7.2 please

@Quy

This comment has been minimized.

Contributor

Quy commented May 17, 2017

Uploaded a PDF file and got the error Invalid mime type detected. and in the PHP error log:

PHP Notice:  Undefined variable: mime in C:\xampp\htdocs\joomla371\libraries\cms\helper\media.php on line 92
PHP Notice:  Undefined variable: mime in C:\xampp\htdocs\joomla371\libraries\cms\helper\media.php on line 98
$finfo = finfo_open(FILEINFO_MIME_TYPE);
$mime = finfo_file($finfo, $file);
finfo_close($finfo);
}

This comment has been minimized.

@wilsonge

wilsonge May 17, 2017

Contributor

Looks like from the test we have cases where none of these things exist!? Probably having a final else statement here with return false to reject everything if we can't detect things is going to be important

This comment has been minimized.

@zero-24

zero-24 May 18, 2017

Contributor

Fixed with: 298959e

@N6REJ

This comment has been minimized.

Contributor

N6REJ commented May 18, 2017

following

elseif ($isImage && function_exists('getimagesize'))
{
$imagesize = getimagesize($file);
$mime = (isset($imagesize['mime'])) ? $imagesize['mime'] : false;

This comment has been minimized.

@Quy

Quy May 18, 2017

Contributor

c/s remove outside () since not required.

finfo_close($finfo);
$imagesize = getimagesize($file);
$mime = (isset($imagesize['mime'])) ? $imagesize['mime'] : false;

This comment has been minimized.

@Quy

Quy May 18, 2017

Contributor

c/s remove outside () since not required.

This comment has been minimized.

@zero-24
@infograf768

This comment has been minimized.

Member

infograf768 commented May 18, 2017

Except for the cs #16091 (comment) this works for me

@franz-wohlkoenig

This comment has been minimized.

franz-wohlkoenig commented May 18, 2017

I have tested this item successfully on efc334f


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16091.

@franz-wohlkoenig

This comment has been minimized.

franz-wohlkoenig commented May 18, 2017

set RTC, @infograf768?

@rdeutz rdeutz added this to the Joomla 3.7.2 milestone May 18, 2017

@infograf768

This comment has been minimized.

Member

infograf768 commented May 18, 2017

Waiting for
$mime = (isset($imagesize['mime'])) ? $imagesize['mime'] : false;
to be corrected to
$mime = isset($imagesize['mime']) ? $imagesize['mime'] : false;

@wilsonge

This comment has been minimized.

Contributor

wilsonge commented May 18, 2017

Quy's php notices need to be investigated before this can go RTC - clearly in some cases this isn't working fully

@rdeutz rdeutz removed the release-blocker label May 20, 2017

@zero-24

This comment has been minimized.

Contributor

zero-24 commented May 20, 2017

@EndeavorC ok i have finaly found something that could be the issue.

Please run the following SQL

select * from `#__extensions` where `element` = `com_media`;

And post the result here.

@EndeavorC

This comment has been minimized.

EndeavorC commented May 20, 2017

Hello,

When I run the above SQL query in myPHPAdmin I get the following:

Error
SQL query: Documentation

select * from #__extensions where element = com_media LIMIT 0, 25

MySQL said: Documentation

#1146 - Table '.#__extensions' doesn't exist

Sorry this is not much help


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16091.

@zero-24

This comment has been minimized.

Contributor

zero-24 commented May 20, 2017

Yes you need to replace #__ with your database prefix

@EndeavorC

This comment has been minimized.

EndeavorC commented May 20, 2017

Error
SQL query: Documentation

select * from <prefix>_extensions where element = com_media LIMIT 0, 25
MySQL said: Documentation

#1054 - Unknown column 'com_media' in 'where clause'


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16091.

@EndeavorC

This comment has been minimized.

EndeavorC commented May 20, 2017

If I view the com_media table I have the following, not sure if this is what you are looking for:

name: com_media

type: component

element: com_media

manifest_cache:

{"name":"com_media","type":"component","creationDate":"April 2006","author":"Joomla! Project","copyright":"(C) 2005 - 2017 Open Source Matters. All rights reserved.","authorEmail":"admin@joomla.org","authorUrl":"www.joomla.org","version":"3.0.0","description":"COM_MEDIA_XML_DESCRIPTION","group":"","filename":"media"}

params:

{"upload_extensions":"bmp,csv,doc,gif,ico,jpg,jpeg,odg,odp,ods,odt,pdf,png,ppt,swf,txt,xcf,xls,BMP,CSV,DOC,GIF,ICO,JPG,JPEG,ODG,ODP,ODS,ODT,PDF,PNG,PPT,SWF,TXT,XCF,XLS","upload_maxsize":"10","file_path":"images","image_path":"images","restrict_uploads":"1","check_mime":"1","image_extensions":"bmp,gif,jpg,png","ignore_extensions":"","upload_mime":"image/jpeg,image/gif,image/png,image/bmp,application/x-shockwave-flash,application/msword,application/excel,application/pdf,application/powerpoint,text/plain,application/x-zip","upload_mime_illegal":"text/html"}


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16091.

@zero-24

This comment has been minimized.

Contributor

zero-24 commented May 20, 2017

This is what I was looking for thanks. Can you please go to the com_media options and save them again. And than try uploading a image from the frontend again?

If this still don't work please send me a backup + the file you try to upload to tobias.zulauf[at]community.joomla.org as i'm running out of ideas what the issue could be. And I can not reproduce the problems.

@EndeavorC

This comment has been minimized.

EndeavorC commented May 20, 2017

Saving the options did not solve the issue, what type of backup are you asking for?

@zero-24

This comment has been minimized.

Contributor

zero-24 commented May 20, 2017

file and database you can clear the content table if needed. So I can debug the issue.

@gwsdesk

This comment has been minimized.

gwsdesk commented May 22, 2017

If I understand all properly I can mention that I have tested this item successfully on 672e183

@EndeavorC

This comment has been minimized.

EndeavorC commented May 22, 2017

What permissions are you testing with for the user account? I am using an account with publisher. If I replace the media.php file with the one from 3.7.0 it works but not the updated one here or the one that was installed with 3.7.1.

I have tried with included editor (TinyMCE) and Ark Editor

@sedonasky

This comment has been minimized.

sedonasky commented May 23, 2017

I updated to 3.7.2 but still cannot upload PDFs. I cleared the cache, etc. and tried to upload with Super Admin permissions. I still get this error (so clients are still very unhappy).

Error
Invalid mime type detected.

@zero-24

This comment has been minimized.

Contributor

zero-24 commented May 23, 2017

@sedonasky

Error
Invalid mime type detected.

this means that finfo_open and mime_content_type did not work or are not available. Can you please check that they are installed and enabled on your server?

@gwsdesk

This comment has been minimized.

gwsdesk commented May 23, 2017

I cannot replicate this on our servers https://gws-host.com/systems-technology

Imho this is a server issue/settings and not related to Joomla

@Quy

This comment has been minimized.

Contributor

Quy commented May 23, 2017

this means that finfo_open and mime_content_type did not work or are not available. Can you please check that they are installed and enabled on your server?

Maybe include in the error message that finfo_open and mime_content_type could not be found and to contact their hosting provider.

@gwsdesk

This comment has been minimized.

gwsdesk commented May 23, 2017

@Quy which would mean that for every specific hosting config with disabled functions we would have to create exemptions/messages? Does that make sense?

@jen4web

This comment has been minimized.

jen4web commented May 23, 2017

I am having the same problem. Site is hosted with Rochen, PHP 7.0.19. Site is upgraded to 3.7.2, and I am uploading a PDF (131K) as a super user. JPG and PNG files upload without issue, so it's just PDF that's a problem.

@zero-24

This comment has been minimized.

Contributor

zero-24 commented May 23, 2017

Yes please check that one or both php functions are enabled.

@fperillo

This comment has been minimized.

fperillo commented May 23, 2017

I confirm this patch is not working on 3.7.1 and 3.7.2, due to mime_content_type

PHP Fatal error: Call to undefined function mime_content_type() in - on line 1

OpenSuse 12.3 has php5-fileinfo extension module that is not installed on the server... fileinfo was not a pre-requisite when installing or updating.

@zero-24

This comment has been minimized.

Contributor

zero-24 commented May 23, 2017

I hope The mime type fatal error is not shown to the user?

But yes if both methods are not there there is no upload allowed as we cant check the mime types which is required for Security reasons.

@fperillo

This comment has been minimized.

fperillo commented May 23, 2017

I understand, and I really don't know Joomla developers guidelines and, believe me, I don't want to be rude, but after a hot phone call with one user...
according to
https://downloads.joomla.org/technical-requirements
there is no need to install fileinfo package and adding this requirements you added external dependencies that not everybody can fulfill.
I and others installed php as required, and now it is not enough...

PS: I found a similar problem with gzdecode... in this case package php5-zlib is installed but this function is not present..

@brianteeman

This comment has been minimized.

Contributor

brianteeman commented May 23, 2017

@fperillo fileinfo - This extension is enabled by default as of PHP 5.3.0. (http://php.net/manual/en/fileinfo.installation.php)

Joomla cannot be expected to tell you every php extension that needs to be enabled especially if it is enabled by default.

@EndeavorC

This comment has been minimized.

EndeavorC commented May 23, 2017

@brianteeman

This comment has been minimized.

Contributor

brianteeman commented May 23, 2017

We are all volunteers as well

I see from the system info report that you posted before that you are using the ark editor on your site. If I understand your reports correctly you are able to upload images through the administrator but your editors can not at the front end. Are the editors using the ARK editor or the default tinymce editor? If they are using ARK can you try with the TinyMCe editor. I am not passing any blame here i am just trying to gather all the facts so that we can try to help you

@EndeavorC

This comment has been minimized.

EndeavorC commented May 24, 2017

@jen4web

This comment has been minimized.

jen4web commented May 24, 2017

I have been uploading files through the Media Manager. JPG and PNG work, while PDF does not.

I reported this to Rochen, who confirmed that finfo_open and mime_content_type were installed, available, and working correctly.

Their response was this: "Well there seems to be compatibility issue with mime_content_type on Joomla version and PHP v7.0, If I switch the PHP version to v5.6 it is working fine.

It appears the patch provided isn't working on 3.7.1 and 3.7.2, due to mime_content_type. Thanks!"

Previously I was running PHP 7.0.19.

@brianteeman

This comment has been minimized.

Contributor

brianteeman commented May 24, 2017

Thanks for the info about php 7.0 - that will hopefully help track it down

But as this issue is closed it wont be seen by most people so can I please ask you to open a new issue with this information

@gwsdesk

This comment has been minimized.

gwsdesk commented May 24, 2017

I am sorry but I cannot replicate this on our servers (PHP 7.1.14). Neither in default media manager nor in /TinyMCEJCE. I can upload pdf's either from frontend or admin without problems and if pdf's disabled (in media options) I do get the proper message (extension not supported)

However: The pdf's show in the file system with FTP (after upload in the media manager but they do not show after upload in the media manager itself in the administrator (they are uploaded as stated since they show in the file manager with ftp) That only happens when uploaded in this case with Tiny/JCE but they show when uploaded with/in the admin panel in the media manager

so by using the media manager upload they show but when upload with any editor they don't show despite being listed in the file system (FTP)

@zero-24

This comment has been minimized.

Contributor

zero-24 commented May 24, 2017

Thanks. The PDFs are not supported in the frontend image form. (different issue).

As stated above and here #16238 it looks like a server / hosting issue.

Here you can find a PR with a better message incase your hosting did disable both checkers:
#16246

I'm going to lock this issue now. If there are still issues please see #16238 or open a new issue. Thanks.

@joomla joomla locked and limited conversation to collaborators May 24, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.