Need to set Filename / File type in Mime Headers when downloading files

This is indeed an old issue. I use the filefield of CCK instead of of the upload module. At least in Drupal 5 the filefield module doesn't have this issue.

I'm setting this to status: needs review, as it looks likely that the patch you've located is to the point.

I'm also renaming this issue to more clearly describe the problem.

Anyone reviewing this should look at #180778: Implement Content-Disposition comments #15 and #16 for the appropriate patches, which are quick to understand and apply.

Tell me if I'm wrong, but I figure that needs review means it's not a case of the bug needing to be found so much as the solution needs to be checked for correctness. I'm new to the drupal project's workflow though, and I'm quite ready to be corrected.

OK, so here's some review.

I've just taken a look at RFC 2183. The header format implemented in the patch is not RFC compliant.

1/ It needs the disposition-type to be specified. Looks like 'attachment' is the apropriate value So, the added line would now look something like:

+         'Content-Disposition: attachment; filename="'. mime_header_encode($file->filename) .'"',

A significant effect of this is that it forces a browser to download the file. eg a .pdf file will not be displayed in a window care of a user's Adobe acrobat plugin. That's probably a good thing.

2/ RFC 2183 stipulates that the filename MUST be encoded as specified in RFC 2184. However, It looks like Drupal's mime_header_encode function documentation refers to RFC 2047. I'm not familiar with all of what has changed, but given the dates of the documents, I'd say that unicode would be the big one. RFC 2047 dates back to 1996 when unicode had little to no support in mail and web software. Unlike RFC 2047, The code of the drupal function refers to UTF-8, so there's a documentation bug at best. My reading of the encoding function says that it doesn't necessarily escape 'tspecials' (see RFC 2045), including characters like ',', ':', ' ', '=' and ';' which MIME uses to delimit fields in the header, and which would create problems if the header line were to wrap (valid MIME behaviour, but presumably uncommon in web browsers). One would hope that web browsers don't trust server headers enough for this to be exploitable.

At this moment, mime_header_encode does not provide a suitable encoding function for the proposed purpose. That should be fixed.

I'd agree with you in regards to my second point, but not in regards to my first point.

Corrected Patches attached. These are untested, but should be fine. Let me know how you go with them.

The fix to the mime header encoding should be treated as a separate issue. I've been meaning to file a bug report, but haven't had much time.

Can someone commit this please?

Drupal 7 goes first.

New patch versions.

love you!! It works!!

albertoperego, could you clarify which drupal version you've tested this with?

Patch files were produced with file names relative to the wrong working directory, so the automated testing system complained.

These new versions of the patch files should hopefully keep it happy.

[Note - it seems I can edit the comment, but not mark the D6 patch as being incorrect. See #21 below]

The correct line to add is:

'Content-Disposition: attachment; filename="'. mime_header_encode($file->filename) .'"'

instead of

'Content-Length: attachment; filename="'. mime_header_encode($file->filename) .'"'

This works for me on Drupal-6.13

Greetings.

Our set up uses the private file download option.
After upgrading to D6 from D5, prior to putting in this hack, excel files on our system were being re-named with a 'doc' suffix and would not display or save . With the hack, while an excel file was displayed in the download dialog in IE6 as excel, and saved ok, in Firefox on the mac it was still displaying as a word document for download, and the headers indicated a Content type of 'application/msword'. With the latest upgrade to 6.14, the hack didn't seem to work it's usual magic.

I disabled the contributed module imce, and the problem went away.

Might the difficulty generally lie in contributed modules that use hook_file_download possibly over-riding the Content type set by the Upload module?

After copious drupal_set_message tests (there is probably an easier way, but I ain't no coder!) I found in my case it was the use of finfo_file at line 92 in imce.module that set the header Content type to "application/msword", which then overrode the original upload module type header of "application/vnd.ms-excel".

Below was my temporary solution for imce which may help as an illustration if there are other modules involved (will also post to the imce issues)

Swap the order of the 'finfo_file' test that is causing errors, with the db test that works ok.

Original (at line 91 in imce.module) -

     else if (function_exists('finfo_file') && $finfo = @finfo_open(FILEINFO_MIME)) {
       $type = finfo_file($finfo, $path);
       finfo_close($finfo);
     }
     else if ($result = db_result(db_query("SELECT filemime FROM {files} WHERE filepath = '%s'", $path))) {
       $type = $result;
      }

Change to -

    else if ($result = db_result(db_query("SELECT filemime FROM {files} WHERE filepath = '%s'", $path))) {
      $type = $result;
    }
    else if (function_exists('finfo_file') && $finfo = @finfo_open(FILEINFO_MIME)) {
      $type = finfo_file($finfo, $path);
      finfo_close($finfo);
    }

Used a swap rather than a comment out, as the least disruptive option. And it's good to have the option of viewing in the browser again.

Hope this is of some help, I've set it back to active (with the greatest trepidation) because I'm not sure this would resolve the original difficulty that @jvieille was having ... but I think the problem may not be in core.

Cliona

Corrected D6 version of the patch attached. I believe the ones for D5 and D7 are correct.

Can people who use these successfully (or not) please say so on this page. It's a bit frustrating to have fixed the problem but have noone stepping in to commit it. Perhaps some feedback will help that process.

Yes, worked for mee to.
Drupal 6.14

Thanks,

Wims

I have similar problem, except that I don't use upload module (uploading files via FTP).

- Drupal 6.15
- private file system
- FCKEditor + IMCE as file browser

File names are totally messed up. Switching between private and public file access don't help.
What to do if I don't use the upload module?

Andrzej

Has this been fixed in the latest D6?