Make Content-Disposition configurable for private downloads
| Project: | FileField |
| Version: | 6.x-3.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
FileField currently sets the Content Disposition header to "attachment" for most MIME types when a file is downloaded from a filefield. This same behavior broke SWF content uploaded through a filefield and was fixed by adding flash as hard-coded special case that is dowloaded as "inline" rather than "attachment" in #325795: Content-Disposition: attachment breaks swf content and #483232: Content-Disposition header may be incorrect for Flash files.
The MIME type 'audio/midi' could be added to the special cases by adding "|| ereg('^(audio/midi)', $file->filemime)" to the line that sets $disposition, analogously to the previous patch for flash.
But the fact that this is a problem for a rare file type like MIDI is making me think that we may want an administrative interface to create an "exception list" of MIME types that should always be sent with the "inline" disposition rather than following the automatic rule. Then administrators could add a filetype to this list if the need came up, but would feel free to ignore it if they did not see what it was for.
I'd be glad to do the coding on this, if we agree that it is the correct way to go.
#1
Here is the change I have in mind to solve this issue. Please be gentle -- this is my first post of a patch. It is rolled against the current dev version.
#2
subscribe
#3
#4
I fixed the patch after testing revealed a namespace collision. Here is the improved patch.
#5
Thanks for the patch crotown. I've thought about adding a settings page several times before, especially to help centralize some settings that are currently hidden. However adding a settings page for this particular feature doesn't seem appropriate. The filefield_file_download() function is only used if using private files, which is by far the minority of Drupal sites out there. Making this a setting that is only applicable to some Drupal sites seems excessive.
#6
Nate, what do you suggest as an alternative for sites that do use private downloads and want to control download behavior of filefields?
#7
To satisfy my indecisive nature regarding a FileField settings page, I'd be happy to have a temporary solution of making the setting but not giving it a UI.
<?php$inline_types = variable_get('filefield_inline_types', array('image/*', '*/flash'));
?>
You can then set this setting manually in your settings.php file or using the devel variable editor.
We also need a way to make easy conversions between what user wants to enter (something like "image/*") and what FileField needs to actually do the matching. We're currently using ereg() but if we can use preg_match() I'd be happy with that change. Either way we need to make sure that we maintain the current functionality of images and Flash files being displayed inline by default.
#8
Subscribe, I'm interested too in this discussion on Content-Disposition handling, let me know when you ageree on a solution
#9
quicksketch, thanks for the wise opinion and help on this. (And thanks for all the education you've provided through the Lullabot podcast!) I agree that a settings page is not called for with this fix, now that I see how to do without it.
I have redone the fix so that the current functionality is preserved if the variable 'filefield_inline_types' is not set, and the array of strings provided in 'filefield_inline_types' is followed exclusively if it is present.
That means that one has to provide all inline types if one wants to add one. For my MIDI case, I do this via the Devel module's "Execute PHP Code" function with the code:
variable_set('filefield_inline_types',array('image\/*', 'text\/*', 'flash$', 'audio\/midi'));to add the inline type audio/midi.
Thanks in advance for your thoughts on this patch.
#10
Nice, this looks great. Only a couple things:
- Let's switch the regex delimiter to a character instead of "/" that isn't allowed in mime-types. "!" is another popular delimiter. That will make it so that the inline types can just be
array('image/*', 'text/*', 'flash$').- Is
strlen($inline_type)>0necessary?#11
I changed the pattern delimiter to "!" and removed unnecessary backslashes. The strlen() check was not necessary so I removed that as well.
Now I can add the audio/midi and audio/mid types with the PHP code:
variable_set('filefield_inline_types',array('image/*', 'text/*', 'flash$', 'audio/midi?'));#12
Thanks again crotown, I've modified the patch slightly to trim it up a bit and make the comments fit within the 80 character wrap. Committed the attached patch.
#13
#14
Automatically closed -- issue fixed for 2 weeks with no activity.