Private files show open/save prompt for PDFs, etc. while public files automatically open inline

This issue points out a serious bug that affects all browsers. Both Drupal 4.7 and Drupal 5.0 have this problem: when files are public, they open in the browser, and when files are private, they download. I cannot think of any good reason to keep this behavior.

With some very limited testing, it would appear that the files open correctly if using IE7 it seems to be only IE6 which is affected by the problem, but for me at least it is a very big problem as most users seem unable to grasp they can simply download the file then open it locally.

I keep getting messages I cant open the file its corrupted

if you want a test try http://www.dibbletree.com/node/88 and try any of the pdfs on the page if you use firefox 1.5 or 2 or IE7 they open ok if you use IE6 you must download the file before opening it

Tkgafs

Can you guys please try this on a *completely clean* 4.7 install from latest 4.7 CVS that (aside from the default stuff that happens when you install Drupal) has only uid 1 created, the upload module enabled, and private file setting switched on?

I was not able to reproduce it this way, although I can reproduce it on one of our sites with a bunch of contrib and some custom modules.

Btw, here is a function I placed in our custom module that seemed to get rid of this error:

/**
 * Implementation of hook_file_download().
 *
 * For PDF and XLS files, returns custom headers so that IE
 * can open the files without an error.
 */
function custom_file_download($file) {
  $dir = variable_get('file_directory_path', 'files');
  $ext = substr(strrchr($file, "."), 1);
  if ($ext == 'pdf' || $ext == 'xls') {
    return array(
      'Cache-Control: must-revalidate, post-check=0, pre-check=0',
      'Content-Type: application/octet-stream',
      'Content-Length: ' . filesize($dir .'/'. $file),
      'Content-Disposition: attachment; filename="'.$file.'"',
    );
  }
}

This issue affects Safari, Firefox, and Internet Explorer on my Mac. I did as webchick requested (see report in issue 30525 for system information) using a fresh install of Drupal 4.7 from CVS with no contrib modules, only one user, the upload module enabled, and files set to private.

The only browser on my Mac that will open a PDF attachment on my clean Drupal site is RealPlayer.

Webchick,

in my case the upload module is not enabled, I am using private files with the attachment & filemanager modules but getting exactly this error, but for me it works ok on firefox and IE7 but not IE6.

Background to my particular case

I have only ever used 4.7 and everything was running fine until my hosting company managed to erase my home directory containing all the drupal code, but my database was left intact.

I re downloaded 4.7 and all the modules I use from the drupal site and pretty quickly had the site up and running again.

But one thing I hadn't understood was that a module you install as a 4.7 module might be different a few weeks later when you re-install it !! although its still called module xyz ver 4.7.0

since the reinstall of my code base this is the one thing that is not working as it was before and I'm a bit at a loss as to where the problem lies

hope this helps to track down problem area

tkgafs

Sorry meant to add in my case download urls look like www.example.com/filemanager/active?fid=xx where xx is the file number

tkgafs

I tracked down the problem to the Content-Disposition header that Drupal adds to any file with a type other than text/ or image/. PDF files are type application/pdf. In Drupal 4.7, the offending lines are 194 and 198 in upload.module.

Information on the Content-Disposition header is available here: http://www.ietf.org/rfc/rfc1806.txt. Browsers that do not understand this header will ignore it. That means that if IE7 and some versions of FireFox are opening private files in a browser window, it's actually a bug. In my experience, both FireFox 1.5 and FireFox 2.0 download rather than open files with this header.

You can see the difference in the headers sent when files are private here (using two Drupal Web sites I maintain):

Public setting: http://web-sniffer.net/?url=http%3A%2F%2Ftest.sidepotsinternational.com%...

Private setting: http://web-sniffer.net/?url=http%3A%2F%2Fbass.sidepotsinternational.com%...

thanks for that info in the filemanager module at approx line 580 is the following code

 $default_headers = array(
    'Content-Type: '. $file->mimetype,
    'Content-Length: '. $file->size,
    'Content-Disposition: attachment; filename="'. $file->filename .'"'
  );

If I remove the Content-Disposition line the pdf now opens inline in the browser window in both IE6 and firefox 1.5 I dont have any other browsers to test nearby.

This is what I actually want to happen and I'm happy to remove it from the code but it doesnt really answer the question as to why IE6 actually fails to correctly download and then pass on to acrobat the pdf file

as you say and I'd actually forgotten firefox used to open the file inline but with the content-disposition header it actually asks you what you want to do with the file, which is presumably the correct behaviour

thanks to you all for your help

does this also need to be raised as an issue with the filemanager module ?

Tkgafs

Yes, please open an issue with the filemanager module. This issue concerns the upload module. The problem with IE6 was fixed for upload module. See issue 30525 for details.

This patch adds application/pdf to the mime types that should be viewed inline.

Hm. I wonder if that is a proper fix though. IIRC we were also having this problem with .xls, .doc files, etc.

There's really no need to send the Content-Disposition header at all. It just makes private and public files display differently, which confuses people. The proper way to handle mime types is to let users configure their own browsers the way they want to.

There was probably a security rationale for including the header, but if uploads are handled properly, we shouldn't have to worry about malicious downloads. So I deprecate my patch and ask that the Content-Disposition header be removed.

For good measure, here is a patch to remove the header and return control of how files are displayed to viewers.

For those who are nervous about this patch:

Content-Disposition is an optional header; in its absence, the MUA may use whatever presentation method it deems suitable.
From page 2 of http://www.ietf.org/rfc/rfc1806.txt

So this issue needs a title change as this isn't about files failing to open in IE 6 as http://drupal.org/node/30525 fixed that. The issue at hand is how private files show an open/save prompt for PDFs, XLS, etc. and public files just open them inline.

+1 on this patch as it returns the control to the browser and works as expected in making public/private files download in a consistent manner.

patch works for me

Thanks for the reviews.

Usually you wait for someone else to RTBC your patch. :P

I think this needs another look as I was checking it out in IE 6 and clicking on a private PDF with this patch would open the current browser window to a blank window in IE 6 with the url of http://example.com/system/files/whatever.pdf and then popup an Acrobat reader with the file. Anyone else experiencing that behavior?

It sounds like a browser setting. How does it work when you set files to public?

Patch at #15 works and doesn't appear to introduce any side effects. I would have set this RTBC but RobRoy's comment at #20 needs looking into I guess.

In reply RobRoy's #20 comment, I don't get this effect. Just works fine.

regards,
--AjK

Committed to HEAD.

This is needed in 4.7 as well.

This reverts patch http://drupal.org/node/30525, which was aimed to fix upload.module problems with IE. Are we sure this doesn't re-introduce problems on IE?

That patch just added a comma to the end of the Content-Disposition line. The line was already there. Removing the line will not cause any problem.

Quoting drumm from issue 30525: "I checked the history before committing and a Content-disposition header has been there since the beginning of upload module."

Marked for porting to 4.7.

OK, let's remove then. Someone entered a *TAB* into the Drupal source!! Oh horror. Removed.

applied

I'm finding this exact same problem on my installation of Drupal 7.0.

History

Upload Module

Looking through the history (via git log -Sdisposition), it's clear that the Content-Disposition header was removed from the upload module in this patch.

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.

Current Status

Currently the list of MIME types which should show up as inline is configurable via 'file_inline_types'. See #1001664: Replace file_inline_types variable with a field formatter setting for an issue which would make this configurable via the UI.

But perhaps we should discuss whether this is really needed at all.

Patch attached for 7.x-dev. Now I have no idea if the state of the universe has changed such that content-disposition (while once deemed a bad idea) might now be the proper thing to do.

For the curious, the one failing test in #34 is an assertion that the content disposition header was sent correctly. (Obviously it won't be if you don't actually send it).

The assertion was added in #898036: Private images broken, but there's no mention in the issue about why it was added.

This should be created as a new issue (task or feature request). There's no good reason for reopening an issue that was closed nearly 5 years ago.

Well, it's a clear regression and the exact same as this issue so reopen IMO is ok.

Agreed. I'll open a new issue in a bit.

Discussion continued in #1229014: Content-Disposition header makes private files show open/save prompts, but public files show inline.