Download & Extend
Issues

Non-standard and pointless name attribute added to Content-Type header

Posted by Jorrit on September 4, 2012 at 7:11am
Project:Drupal core
Version:7.x-dev
Component:file.module
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed (fixed)
Issue tags:7.17 release notes, needs backport to D7

Issue Summary

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.

Login or register to post comments

Comments

#1

Posted by Jorrit on September 4, 2012 at 7:15am
Status:active» needs review

The patch.

AttachmentSizeStatusTest resultOperations
remove-content-type-name-attr-1773032-1.patch2.04 KBIdlePASSED: [[SimpleTest]]: [MySQL] 41,084 pass(es).View details

#2

Posted by quicksketch on September 4, 2012 at 7:51am
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.

#3

Posted by webchick on September 4, 2012 at 9:30pm
Version:8.x-dev» 7.x-dev
Status:reviewed & tested by the community» patch (to be ported)

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.

#4

Posted by Jorrit on September 5, 2012 at 6:44am
Status:patch (to be ported)» needs review

Here you go. Thanks for the quick commit!

AttachmentSizeStatusTest resultOperations
remove-content-type-name-attr-1773032-1-d7.patch1.76 KBIdlePASSED: [[SimpleTest]]: [MySQL] 39,380 pass(es).View details

#5

Posted by dcam on September 14, 2012 at 11:26pm
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."

#6

Posted by David_Rothstein on September 15, 2012 at 9:48pm
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.

#7

Posted by David_Rothstein on September 15, 2012 at 9:50pm

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.

#8

Posted by System Message on September 29, 2012 at 9:51pm
Status:fixed» closed (fixed)

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

#9

Posted by David_Rothstein on November 6, 2012 at 5:08am
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.

nobody click here