The Content-Type header set by file_get_content_headers() adds a name attribute, that isn't standard or documented anywhere on the Internet. Although it doesn't cause any problems, leaving it in might confuse people (like me) when looking for bugs in their code.

I'll add a patch to remove it from Drupal 8. I'll be happy to port it to Drupal 7 and perhaps 6 if needed.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Jorrit’s picture

Status: Active » Needs review
FileSize
2.04 KB

The patch.

quicksketch’s picture

Status: Needs review » Reviewed & tested by the community

I researched this yesterday and came up with the following conclusions in an e-mail to Jorrit:

So that patch was (more or less) a direct port of the code from FileField. So I went back through the history of FileField to find out where that came from.

Going back in time, the last time that line was changed was here, when we added quotes around the file name.

http://drupalcode.org/project/filefield.git/blobdiff/1231621ba11da531cd2...

However that doesn't explain why it's there, since like you say it doesn't seem to be standard. So the time before that was here, when we ported the Drupal 5 version of FileField (v. 2.0):

http://drupalcode.org/project/filefield.git/blobdiff/490f07d90cadbc820f3...

However that was just a code restructuring.

My git foo going back in time this much gets pretty weak, but I'm *pretty* sure this code goes all the way back to the original versions of ImageField (on which FileField was later based).

Hilariously, this goes back *even further*, since ImageField was based on *Upload* module in Drupal 5. Which, going back far enough, you can find that it had the same problem! It was fixed in http://drupal.org/node/103164.

Here's the commit diff that fixed it in Upload module: http://drupalcode.org/project/drupal.git/blobdiff/677746341403edf159381e...

However this change was never made to ImageField in Drupal 5, so it propagated through all the incarnations all the way up into the File module we have today.

Anyway, the presence of the "; name: filename" property apparently goes ALL THE WAY back to the very first commit of upload.module, back in August 2004. You can see its presence here:

http://drupalcode.org/project/drupal.git/blob/78b052a6af5fc7c87c807821bf...

Anyway, funny thing. A single mistake from 8 years ago is still with us in Drupal 8. ;)

TL;DR: We already fixed this in core once, but the FileField port into D7 reintroduced the problem. See http://drupal.org/node/103164.

webchick’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)
Issue tags: +Needs backport to D7

LOL that's awesome. Awesome find Jorrit, and awesome sleuthing too, quicksketch!

Committed and pushed to 8.x. We should fix this in 7.x, too.

Jorrit’s picture

Status: Patch (to be ported) » Needs review
FileSize
1.76 KB

Here you go. Thanks for the quick commit!

dcam’s picture

Status: Needs review » Reviewed & tested by the community

I'm not certain how to test this one since the bug isn't causing a problem for me with any browser that I've tried (using a private file field in FF, Chrome, and IE9). It's basically the same patch that got committed to D8 and the same fix that was introduced in #103164: Internet Explorer Not Opening Attachments Correctly-double window issue. The patch applied correctly and file downloads continued to work afterward. So I'm marking it RTBC. If anyone thinks this should be tested more thoroughly, go ahead and set the status back to "Needs review."

David_Rothstein’s picture

Status: Reviewed & tested by the community » Fixed

Wow, with a history like that it is almost sad to remove it :) Like, it's been through so much, it doesn't deserve to die. Or maybe we are tempting fate, and it will be back again later, only this time more powerful than before...

Anyway, it seems to me like it's been tested and reviewed well enough, so I committed this to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/e218eff

As far as I can tell from reading the issue, there actually isn't anything to fix in Drupal 6 here, but feel free to reopen and backport if I'm wrong.

David_Rothstein’s picture

Oh by the way, I did a little Googling and I got the impression that maybe this attribute once did exist, but was deprecated sometime in the early 1990's :)

http://www.imc.org/ietf-822/old-archive2/msg02121.html

Though I wasn't 100% sure that was referring to the same thing.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

David_Rothstein’s picture

Issue tags: +7.17 release notes

Although this is a pretty trivial change, it did break a core test, so on second thought I'm adding this to the release notes and CHANGELOG.txt just in case someone is relying on it somewhere.

neRok’s picture

Just found the following related issue where it has been found the 'name' part of the return array should have been called 'filename'.
#2010360: file_get_content_headers should add a Content-Disposition header with "filename" in it