This is essentially a duplicate of #61528: Private files show open/save prompt for PDFs, etc. while public files automatically open inline, but since that issues relates to 4.7.x-dev I'll open a new one.
Issue:
When visiting an uploaded file, the presentation in the browser depends on the storage protocol. This is especially important for PDF files as they may either be displayed in the browser or downloaded/saved/opened in a different application.
The method of specifying the desired browser action is through the Content-Disposition header which is either:
- inline, meaning showing the file in the browser, if possible; and
- attachment, meaning prompt the user to save the file.
By default, Drupal installations serve up files in two different ways, depending on the storage method:
- public://...
- These files are generally served directly by the webserver without Drupal bootstrapping. Many web servers do not send any content disposition headers and so the file is generally shown inline in the browser.
- private://...
- These files are served by Drupal. Drupal injects a Content-Disposition header which depends on MIME type. Currently file_inline_types (i.e. text/*, image/*, and *flash) are served as inline and remaining files are served as attachments.
- In the private://.. storage method, other MIME types, like PDFs, are sent as attachments making their behavior different than if they were stored using public://.
History
Upload Module
Looking through the history (via git log -Sdisposition
), it's clear that the Content-Disposition header was removed from the original upload module in #61528: Private files show open/save prompt for PDFs, etc. while public files automatically open inline in version 4.7.x.
CCK FileField
In CCK FileField, the commit
commit a7da1cd39f893cdf629f9b9a2cd423f43b06c5d3
Author: Darrel O\'Pry <dopry@22202.no-reply.drupal.org>
Date: Thu Feb 1 09:27:38 2007 +0000
backporting some fixes, untested... use at your own risk!!
copied upload_file_download()
(from a version before the upload module patch) into filefield_file_download()
.
The list of inline mime types was made configurable in #485336: Make Content-Disposition configurable for private downloads (using the variable 'filefield_inline_types').
When CCK FileField entered core (#391330: File Field for Core) this was preserved (the variable being renamed to 'file_inline_types'). The code was later split out to a separate function (file_get_content_headers) but the logic preserved in #943112: file_file_download should delegate header checks to a separate function.
Proposed Solutions
- Remove the content disposition header entirely. Is it necessary?
- Reconfigure the list of MIME types which should show up as inline. Perhaps add a configuration page, or default to a larger list of MIME types.
Comments
Comment #1
bfroehle CreditAttribution: bfroehle commentedThe attach patch removes the content disposition header, but will fail the test suite because of an assertion (that the content disposition header was sent correctly) added in #898036: Private images broken.
Comment #3
bfroehle CreditAttribution: bfroehle commentedThe attached patch removes all of Drupal handling of the Content-Disposition header. Read the issue summary for reasons why this is a good idea.
Comment #4
dvcroftarb CreditAttribution: dvcroftarb commentedThis seems to be exactly the issue I am experiencing on Drupal 7. Any chance of patching it too?
Comment #5
claudiuv CreditAttribution: claudiuv commentedI hope that this isn't off topic, but i suspect this bug is related to my problem - when downloading private files, I get a generic filename in the Save As dialogue, that is "download.pdf" instead of the original filename. I am running Drupal 7 with Filefield Sources module installed.
Comment #6
haydeniv CreditAttribution: haydeniv commentedI just manually applied this patch to D7 and it fixed my behavior problem as well. Bot's happy so I'd say this is RTBC but I don't think I have enough community mojo to flag it as such.
Comment #7
jackbravo CreditAttribution: jackbravo commentedI think this can be marked as RTBC. I also don't have community mojo =P (http://drupal.org/node/156119), but in my opinion the patch is not too complicated and the only reason against it could be that the filename is set on the Content-Disposition header (http://stackoverflow.com/questions/1741353/how-to-set-response-filename-...), but private file URLs are rightly named so I don't see any problem there.
Comment #8
bfroehle CreditAttribution: bfroehle commented#3: Content-Disposition-header-1229014-3.patch queued for re-testing.
Comment #10
jackbravo CreditAttribution: jackbravo commentedHere is a re-roll of the patch.
Comment #11
haydeniv CreditAttribution: haydeniv commentedI've been running fix on a site for a couple months now and it works well. RTBC.
Comment #12
haydeniv CreditAttribution: haydeniv commentedTagging
Comment #13
tim.plunkettFixing tags
Comment #15
jackbravo CreditAttribution: jackbravo commentedSo PATCH #10 is for D8. Has that one been applied already?
And here is a PATCH for D7.
Comment #16
tim.plunkettComment #17
catchMakes sense to just remove this. Committed/pushed to 8.x.
Comment #18
jackbravo CreditAttribution: jackbravo commentedIt is the same change for Drupal 7, and all tests pass already. Seems RTBC.
Comment #19
pattip CreditAttribution: pattip commentedHello-
Can I get help?
D7 patch (located in drupal root folder) is failing
(Drupal 7.16) (Environment: hostgator-Linux shared hosting w/SSH via PuTTY)
error:
patch -p1 < Content-Disposition-header-D7-1229014-14.patch
patching file includes/file.inc
Hunk #1 FAILED at 2478.
1 out of 1 hunk FAILED -- saving rejects to file includes/file.inc.rej
patching file modules/image/image.test
Hunk #1 FAILED at 749.
1 out of 1 hunk FAILED -- saving rejects to file modules/image/image.test.rej
I am grateful for any help..thank you!
Comment #20
pattip CreditAttribution: pattip commentedComment #21
jackbravo CreditAttribution: jackbravo commented@pattip the patch applies cleanly to drupal 7 dev version. Drupal 7.16 probably is a little behind. You could try to apply this patch by hand, it only changes to files.
Comment #22
haydeniv CreditAttribution: haydeniv commented#15: Content-Disposition-header-D7-1229014-14.patch queued for re-testing.
Comment #23
haydeniv CreditAttribution: haydeniv commentedJust applied this patch again and tested for good measure. PDFs now follow default browser handling. Works good on file downloads. If bot is happy then this is good to go.
Comment #24
haydeniv CreditAttribution: haydeniv commentedBot is happy. Hopefully this gets committed before next version.
Comment #25
chrisarusso CreditAttribution: chrisarusso commentedAs of 7.20 patch is as attached:
Comment #27
David_Rothstein CreditAttribution: David_Rothstein commentedThe patch in #15 still applies. Reuploading that and assuming it's still RTBC (haven't reviewed it myself yet though).
Comment #28
iva2k CreditAttribution: iva2k commentedminor note: line
$name = mime_header_encode($file->filename);
can be removed as $name use is removed.Comment #29
David_Rothstein CreditAttribution: David_Rothstein commentedYeah, it seems like this is the right thing to do. Committed to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/b927d87
I'll add it to the release notes also, just to be safe (in case someone out there is actually expecting this header to be there).
Maybe we can get a followup to remove the useless $name variable mentioned above (affects Drupal 8 too, I believe)?
Comment #31
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedHappened in #2853160: Variable $name is defined but not used in file_get_content_headers() for Drupal 7, and somewhere else for Drupal 8.