Files attached to issues (like patch files) which have "inc" in the filename are incorrectly served with a text/html mime type and their contents are marked up with html. This is clearly inappropriate.

See the issue below for some examples of this occurance.

http://drupal.org/node/16021

CommentFileSizeAuthor
#3 file_mimetype.patch3.81 KBtangent
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tangent’s picture

A filename with "module" in the name is also served the same way.

Perhaps we should also test for "mysql, pgsql, php, css, xtmpl, sh, pl, txt" and any other text file extensions. If this is not desirable, at least change the filter to look for these extensions *at the end* of the filename and allow .patch (or possibly .diff) files to be served as plain text.

Dries’s picture

The project module saves the mime-type sent by the browser when uploading the patch, and reuses that when serving the patch for download. It looks like some people upload patches with the wrong mime-type set.

tangent’s picture

Title: File attachments with ".inc" in filename are served as HTML » File attachments are sometimes saved with an incorrect mimetype
Project: Project » Drupal core
Component: Issues » file system
FileSize
3.81 KB

This issue is a symptom caused by faulty mimetype handling in file.inc so I'm updating the issue to the correct component. The old title was "File attachments with '.inc' in filename are served as HTML".

Apparently, the real issue is that file.inc currently stores the content-type presented by the user-agent when file is uploaded. This is not the best thing to do because a user-agent can present an incorrect content-type either inadvertantly or, when used by a malicious person, purposefully.

There are a couple of methods to determine the actual mimetype of a file but they all have drawbacks.
The Fileinfo (http://pecl.php.net/package/fileinfo) PHP extension is the recommended tool for the job but is not typically available on a typical host. The mime_content_type() function requires the "file" program to be available which is not always the case either. Then there is the fallback method of testing the filename extension against a list of internally known extensions.

I've created a patch which attempts the first 2 options and then falls back to the third. There may be a better option which I have not thought of though so feel free to offer suggestions.

tangent’s picture

Has anyone tested this patch?

I think this is an issue that needs to be addressed. I'm just not sure we want all those extension tests in there.

killes@www.drop.org’s picture

I am not sure we should try to fix browser errors. Anyway, the patch does not apply to head.

tangent’s picture

This isn't a matter of browser errors. It is a matter of the browser sending a non-standard (though possibly commonly used) mime-type in the HTTP headers for an uploaded file. If my operating system has a different mime-type assigned to files with a specific extension than your operating system then that is the one that will be used. The browser may just pass on the mime-type given to it.

I experience this problem when uploading files using WinXP and Firefox which I wouldn't consider a niche use-case. I also think that handling upload bugs should be very much handled. Assuming the user-agent will or should always do the right thing is not a wise approach in my opinion.

killes@www.drop.org’s picture

Status: Active » Fixed

We now use mime_content_type in file.inc.

Anonymous’s picture

Status: Fixed » Closed (fixed)