Implement Content-Disposition

i don't think you're correctly quoting the filename in all cases. non-ascii characters will need to be escaped.

I can't reproduce this – the files come home cleanly here, with both private download and non-ASCII character (æøå). I'm getting a bunch of

notice: ob_end_clean() [ref.outcontrol]: failed to delete buffer. No buffer to delete. in /srv/http/localhost/htdocs/drupal6/includes/file.inc on line 792.

though, but I don't think that's related... (I also found out that if I upload "æøå.txt", it will get renamed to "txt." – but again, this is hardly related.)

I didn't notice the URL rewrite=off. Oops. :$

Anyway, patch doesn't apply:

gentoo-vm drupal6 # patch modules/upload/upload.module http_content_disposition.patch
patching file modules/upload/upload.module
Hunk #1 FAILED at 269.
1 out of 1 hunk FAILED -- saving rejects to file modules/upload/upload.module.rej

And I don't have time to re-roll just right now.

Right, sorry I missed that. Oops. :x

Anyway, the D6 patch doesn't apply either:

gentoo-vm drupal6 # patch modules/upload/upload.module http_content_disposition_D6_v2.patch
patching file modules/upload/upload.module
Hunk #1 FAILED at 154.
1 out of 1 hunk FAILED -- saving rejects to file modules/upload/upload.module.rej

Please re-roll.

Feature requests -> 7.x-dev

Feature? I'd say usability. Receiving a file as "index.php?..." is not what a user expects, and that is what this issue is out to fix. It's also a very minor patch which does not alter, add, or remove any string or part of the API.

I'd also say that this is good for (being ported to) Drupal 5 once it's gone into D6.

freso@nayru /s/h/l/h/d/m/upload> patch -p0 < ../../upload.module_content-disposition.patch
patching file upload.module
Hunk #1 FAILED at 154.
1 out of 1 hunk FAILED -- saving rejects to file upload.module.rej

Uploaded re-rolled patch. Will test in a bit.

Works perfectly for me. Here are the steps to test:

1. Install Drupal.
2. If they're not, disable clean urls (admin/settings/clean-urls) and Apache's url rewrite (.htaccess).
3. Set file download to private (admin/settings/file-system).
4. Upload/attach a file to a node, view that node, and try to download the file. You should get a filename other than that of the uploaded file. (Note that it might only be files with spaces or other 'odd' characters that trigger this, so play about a bit.)
5. Apply patch and try to download the file again - this time it should have the same filename as that of the original fil! Yay! :D

One thing I just noticed about this patch: All uploaded files will now be downloaded, instead of possibly shown in the browser like now. (Eg., my Firefox shows XML and PNG files, while PDF and ODT files are downloaded. This would also mean that patches etc. would have to be shown outside the browser, which I would find to somewhat of an inconvenience when quickly browsing e.g. this issue queue.)

Removing the "attachment;" seems to do the trick. Same test procedure as before.

And here are some ports of the previously attached patch: One for Drupal 5 and one for Drupal 7.

Note that these haven't been tested - I'm simply assuming that since the patch is as simple as this, and it works on Drupal 6, then it'll also work there. But it would probably be proper to have them tested. Test procedure should be more or less the same as before.

Content that should be shown in the bowser should use inline;, while files that should be downloaded should use attachment;. I would think we shouldn't specify this like said in #15.

Perhaps this would gain some ground and actually have a chance of being committed if it is applied to the latest dev instead of stable? I haven't tested it on 7.x though.

freso, the standard workflow is to get a change committed to HEAD (at this point 7.x) and then backport it if it's a bug fix.

section 2 of RFC1283 provides the grammar for Content-Disposition as:

     disposition := "Content-Disposition" ":"
                    disposition-type
                    *(";" disposition-parm)

     disposition-type := "inline"
                       / "attachment"
                       / extension-token
                       ; values are not case-sensitive

extension-token is defined by RFC2045 as:

     extension-token := ietf-token / x-token

     ietf-token := <An extension token defined by a
                    standards-track RFC and registered
                    with IANA.>

     x-token := <The two characters "X-" or "x-" followed, with
                 no intervening white space, by any token>

     token := 1*<any (US-ASCII) CHAR except SPACE, CTLs,
                 or tspecials>

So, in my opinion, to follow the standard, we can't just provide a filename. We need to specify either 'inline', 'attachment', or 'x-something'.

Couldn't we call the token simply x-drupal, or, for kicks, use x-attachment? My guess is that either of those would trigger the same course of action as having no disposition type defined. I'll try and get around to it (making a patch and testing it) sometime this week. We'll see. (If someone feels like beating me to it, I won't hold any grudges, and I promise to test the patch - against any of the three versions patches are provided for! :))

@thePanz: The RFC states that you can use "inline", "attachment", or an extension-token. The extension-token can be one defined by the IETF, a local one (x-foo), or a generic token (which I don't really get, but something like 1*foo I think). Using an "x" token would thus not be non-standard and would (probably) work around buggy browsers like Firefox, no matter the sent MIME type.

Thanks for the patch! Now my private downloads are working again!

This issue is fixed in the CCK filefield module! So for everyone who doesn't want to patch use the filefield of CCK instead of the core upload module!

marked #258723: Filenames (Content Disposition Header) for Private Downloads as a duplicate

Thanks a lot every one! I was having this problem and instituted a workaround with webfm, but this is much better.

Why is the patch (#15) not yet committed one year after it is solved?
I am tired modifying core files each time a new D6 is released.
D7 is not there yet, D5 and D6 shall still be maintained - I just speak for D6, but I suppose D5 hasn't be ported too.

It can't be committed to 6.x, if it isn't committed to HEAD first. And the issue from drewish's comment #19 hasn't been dealt with yet, which is why it still "needs work".

OK, I'll forget Upload.
I already switched most of me sites to File Framework, I'll proceed with the rest.
Interesting that dealing with files seems not of high priority in Drupal...

upload module is no more as of 7.x. file.module is the new happening spot.

jvielle, there's been a lot of hard work on files in 7.x sorry we're all not jumping up and down to solve the single issue that bothers you.

Only feature requests and tasks can be assigned to D8 for now, because the Drupal 8 tree is not opened yet.

+1