support optional content-disposition: attachment header about serve-static HOT 18 CLOSED

just add another middleware to set this header in your app?

from serve-static.

Good point.

from serve-static.

Wait wait wait. This is something that should be handled in this middleware. Consider the case when a 404 is returned. Only this middleware knows it's a 404 and not a 200. The header should only be set when the response is 200.

Also, only this middleware knows what filename to set for the header.

from serve-static.

All good points :) BUT this middleware is actually passing off all the resolution, filename lookup, etc. to send, so it probably needs to be an option there first so we can pass the option to send.

from serve-static.

From the user of this API's point of view, send doesn't exist, right? So it seems to me that one of these two things is true:

1. This feature request is valid and the issue should remain open until it is solved (even if it is solved in a dependency), or
2. This issue is a "won't fix" and should be closed.

from serve-static.

Sure, I mean, it seems valid for me (to serve up a bunch of files that you want to always download).

from serve-static.

Something like this would do it:

app.use(function(req, res, next) {
    var writeHead = res.writeHead
    res.writeHead = function(status) {
        res.writeHead = writeHead
        if (status === 200) {
            res.header('Content-Disposition', 'attachment; filename=' 
              + require('path').basename(req.url))
        }
        res.writeHead.apply(res, arguments)
    }
    next()
})

Monkey-patching FTW!

from serve-static.

I'd murder a school bus full of children before I'd write that code.

from serve-static.

I guess there's no accounting for taste. :)

Still, it looks better than a metric ton of different options for every use-case.

For example, if you want to serve only ".avi" files with content-disposition, would you request another option for serve-static specifying file extension? In the example above I can do it changing just one line of code:

if (status === 200 && path.extname(req.url) === '.avi') {

from serve-static.

Well...that escalated quickly.
On Apr 3, 2014 5:41 PM, "Andrew Kelley" [email protected] wrote:

I'd murder a school bus full of children before I'd write that code.

Reply to this email directly or view it on GitHubhttps://github.com//issues/5#issuecomment-39508580
.

from serve-static.

Well...that escalated quickly.

I hope it's clear I'm exaggerating in order to entertain and amuse :-)

Still, it looks better than a metric ton of different options for every use-case.

One way would be for the API user to supply a function callback.

function (response, next) {

}

from serve-static.

I'd murder a school bus full of children before I'd write that code.

lol most of the repos in this org overwrite res.writeHead() like that :(

poor children!

from serve-static.

@andrewrk you can use https://github.com/expressjs/on-headers and pretend like you are not overwriting writeHead now.

from serve-static.

I'm closing this. Adding the header won't quick work, since you can't disable the conditional GET behavior. You may be better off using send directly. In addition, the 404 argument isn't quite valid, because this middleware will next() on 404, so this should work:

var serve = require('serve-static')(__dirname + '/public')
function cdMiddleware(req, res, next){
  res.setHeader('Content-Disposition', 'attachment')
  serve(req, res, function (err) {
    res.removeHeader('Content-Disposition')
    next(err)
  })
}

from serve-static.

Ah that code snippet is beautiful. Thanks :)

Solved my issue: andrewrk/groovebasin#173

from serve-static.

I'm going to re-open this because I don't particularly like the solution I provided, haha. There should be a way to "react" to the req, res as soon as this module decides to actually serve a file instead of doing the weird wrapping thing.

from serve-static.

With the next version, the snippet becomes this:

var serve = require('serve-static')(__dirname + '/public', {
  setHeaders: function (res) { res.setHeader('Content-Disposition', 'attachment') }
})

from serve-static.

Nice, looks good!

from serve-static.