Skip to content

Commit

Permalink
Allow options without filename in res.download
Browse files Browse the repository at this point in the history
  • Loading branch information
@dougwilson
dougwilson committed Mar 25, 2022
1 parent f739b16 commit 03dc367
Show file tree
Hide file tree
Showing 3 changed files with 206 additions and 62 deletions.
1 change: 1 addition & 0 deletions History.md
View file
Expand Up @@ -2,6 +2,7 @@ unreleased
==========

* Add "root" option to `res.download`
* Allow `options` without `filename` in `res.download`
* Deprecate string and non-integer arguments to `res.status`
* Ignore `Object.prototype` values in settings through `app.set`/`app.get`
* Support proper 205 responses using `res.send`
Expand Down
7 changes: 7 additions & 0 deletions lib/response.js
View file
Expand Up @@ -561,6 +561,13 @@ res.download = function download (path, filename, options, callback) {
opts = null
}

// support optional filename, where options may be in it's place
if (typeof filename === 'object' &&
(typeof options === 'function' || options === undefined)) {
name = null
opts = filename
}

// set Content-Disposition when file is sent
var headers = {
'Content-Disposition': contentDisposition(name || path)
Expand Down
260 changes: 198 additions & 62 deletions test/res.download.js
View file
Expand Up @@ -86,46 +86,12 @@ describe('res', function(){
})
})

describe('.download(path, filename, fn)', function(){
it('should invoke the callback', function(done){
var app = express();
var cb = after(2, done);

app.use(function(req, res){
res.download('test/fixtures/user.html', 'document', cb)
});

request(app)
.get('/')
.expect('Content-Type', 'text/html; charset=UTF-8')
.expect('Content-Disposition', 'attachment; filename="document"')
.expect(200, cb);
})
})

describe('.download(path, filename, options, fn)', function () {
it('should invoke the callback', function (done) {
var app = express()
var cb = after(2, done)
var options = {}

app.use(function (req, res) {
res.download('test/fixtures/user.html', 'document', options, cb)
})

request(app)
.get('/')
.expect(200)
.expect('Content-Type', 'text/html; charset=UTF-8')
.expect('Content-Disposition', 'attachment; filename="document"')
.end(cb)
})

describe('.download(path, options)', function () {
it('should allow options to res.sendFile()', function (done) {
var app = express()

app.use(function (req, res) {
res.download('test/fixtures/.name', 'document', {
res.download('test/fixtures/.name', {
dotfiles: 'allow',
maxAge: '4h'
})
Expand All @@ -134,51 +100,124 @@ describe('res', function(){
request(app)
.get('/')
.expect(200)
.expect('Content-Disposition', 'attachment; filename="document"')
.expect('Content-Disposition', 'attachment; filename=".name"')
.expect('Cache-Control', 'public, max-age=14400')
.expect(utils.shouldHaveBody(Buffer.from('tobi')))
.end(done)
})

describe('when options.headers contains Content-Disposition', function () {
it('should be ignored', function (done) {
describe('with "headers" option', function () {
it('should set headers on response', function (done) {
var app = express()

app.use(function (req, res) {
res.download('test/fixtures/user.html', 'document', {
res.download('test/fixtures/user.html', {
headers: {
'Content-Type': 'text/x-custom',
'Content-Disposition': 'inline'
'X-Foo': 'Bar',
'X-Bar': 'Foo'
}
})
})

request(app)
.get('/')
.expect(200)
.expect('Content-Type', 'text/x-custom')
.expect('Content-Disposition', 'attachment; filename="document"')
.end(done)
.get('/')
.expect(200)
.expect('X-Foo', 'Bar')
.expect('X-Bar', 'Foo')
.end(done)
})

it('should be ignored case-insensitively', function (done) {
it('should use last header when duplicated', function (done) {
var app = express()

app.use(function (req, res) {
res.download('test/fixtures/user.html', 'document', {
res.download('test/fixtures/user.html', {
headers: {
'content-type': 'text/x-custom',
'content-disposition': 'inline'
'X-Foo': 'Bar',
'x-foo': 'bar'
}
})
})

request(app)
.get('/')
.expect(200)
.expect('Content-Type', 'text/x-custom')
.expect('Content-Disposition', 'attachment; filename="document"')
.end(done)
.get('/')
.expect(200)
.expect('X-Foo', 'bar')
.end(done)
})

it('should override Content-Type', function (done) {
var app = express()

app.use(function (req, res) {
res.download('test/fixtures/user.html', {
headers: {
'Content-Type': 'text/x-custom'
}
})
})

request(app)
.get('/')
.expect(200)
.expect('Content-Type', 'text/x-custom')
.end(done)
})

it('should not set headers on 404', function (done) {
var app = express()

app.use(function (req, res) {
res.download('test/fixtures/does-not-exist', {
headers: {
'X-Foo': 'Bar'
}
})
})

request(app)
.get('/')
.expect(404)
.expect(utils.shouldNotHaveHeader('X-Foo'))
.end(done)
})

describe('when headers contains Content-Disposition', function () {
it('should be ignored', function (done) {
var app = express()

app.use(function (req, res) {
res.download('test/fixtures/user.html', {
headers: {
'Content-Disposition': 'inline'
}
})
})

request(app)
.get('/')
.expect(200)
.expect('Content-Disposition', 'attachment; filename="user.html"')
.end(done)
})

it('should be ignored case-insensitively', function (done) {
var app = express()

app.use(function (req, res) {
res.download('test/fixtures/user.html', {
headers: {
'content-disposition': 'inline'
}
})
})

request(app)
.get('/')
.expect(200)
.expect('Content-Disposition', 'attachment; filename="user.html"')
.end(done)
})
})
})

Expand All @@ -187,15 +226,15 @@ describe('res', function(){
var app = express()

app.use(function (req, res) {
res.download('name.txt', 'document', {
res.download('name.txt', {
root: FIXTURES_PATH
})
})

request(app)
.get('/')
.expect(200)
.expect('Content-Disposition', 'attachment; filename="document"')
.expect('Content-Disposition', 'attachment; filename="name.txt"')
.expect(utils.shouldHaveBody(Buffer.from('tobi')))
.end(done)
})
Expand All @@ -204,15 +243,15 @@ describe('res', function(){
var app = express()

app.use(function (req, res) {
res.download('fake/../name.txt', 'document', {
res.download('fake/../name.txt', {
root: FIXTURES_PATH
})
})

request(app)
.get('/')
.expect(200)
.expect('Content-Disposition', 'attachment; filename="document"')
.expect('Content-Disposition', 'attachment; filename="name.txt"')
.expect(utils.shouldHaveBody(Buffer.from('tobi')))
.end(done)
})
Expand All @@ -224,7 +263,7 @@ describe('res', function(){
var p = '..' + path.sep +
path.relative(path.dirname(FIXTURES_PATH), path.join(FIXTURES_PATH, 'name.txt'))

res.download(p, 'document', {
res.download(p, {
root: FIXTURES_PATH
})
})
Expand All @@ -240,7 +279,7 @@ describe('res', function(){
var app = express()

app.use(function (req, res) {
res.download('../name.txt', 'document', {
res.download('../name.txt', {
root: FIXTURES_PATH
})
})
Expand All @@ -254,6 +293,103 @@ describe('res', function(){
})
})

describe('.download(path, filename, fn)', function(){
it('should invoke the callback', function(done){
var app = express();
var cb = after(2, done);

app.use(function(req, res){
res.download('test/fixtures/user.html', 'document', cb)
});

request(app)
.get('/')
.expect('Content-Type', 'text/html; charset=UTF-8')
.expect('Content-Disposition', 'attachment; filename="document"')
.expect(200, cb);
})
})

describe('.download(path, filename, options, fn)', function () {
it('should invoke the callback', function (done) {
var app = express()
var cb = after(2, done)
var options = {}

app.use(function (req, res) {
res.download('test/fixtures/user.html', 'document', options, cb)
})

request(app)
.get('/')
.expect(200)
.expect('Content-Type', 'text/html; charset=UTF-8')
.expect('Content-Disposition', 'attachment; filename="document"')
.end(cb)
})

it('should allow options to res.sendFile()', function (done) {
var app = express()

app.use(function (req, res) {
res.download('test/fixtures/.name', 'document', {
dotfiles: 'allow',
maxAge: '4h'
})
})

request(app)
.get('/')
.expect(200)
.expect('Content-Disposition', 'attachment; filename="document"')
.expect('Cache-Control', 'public, max-age=14400')
.expect(utils.shouldHaveBody(Buffer.from('tobi')))
.end(done)
})

describe('when options.headers contains Content-Disposition', function () {
it('should be ignored', function (done) {
var app = express()

app.use(function (req, res) {
res.download('test/fixtures/user.html', 'document', {
headers: {
'Content-Type': 'text/x-custom',
'Content-Disposition': 'inline'
}
})
})

request(app)
.get('/')
.expect(200)
.expect('Content-Type', 'text/x-custom')
.expect('Content-Disposition', 'attachment; filename="document"')
.end(done)
})

it('should be ignored case-insensitively', function (done) {
var app = express()

app.use(function (req, res) {
res.download('test/fixtures/user.html', 'document', {
headers: {
'content-type': 'text/x-custom',
'content-disposition': 'inline'
}
})
})

request(app)
.get('/')
.expect(200)
.expect('Content-Type', 'text/x-custom')
.expect('Content-Disposition', 'attachment; filename="document"')
.end(done)
})
})
})

describe('on failure', function(){
it('should invoke the callback', function(done){
var app = express();
Expand Down

0 comments on commit 03dc367

Please sign in to comment.