Skip to content

Commit 03dc367

Browse files
dougwilsondougwilson
committed
Allow options without filename in res.download
1 parent f739b16 commit 03dc367

File tree

3 files changed

+206
-62
lines changed

3 files changed

+206
-62
lines changed

‎History.md

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ unreleased
22
==========
33

44
* Add "root" option to `res.download`
5+
* Allow `options` without `filename` in `res.download`
56
* Deprecate string and non-integer arguments to `res.status`
67
* Ignore `Object.prototype` values in settings through `app.set`/`app.get`
78
* Support proper 205 responses using `res.send`

‎lib/response.js

+7
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,13 @@ res.download = function download (path, filename, options, callback) {
561561
opts = null
562562
}
563563

564+
// support optional filename, where options may be in it's place
565+
if (typeof filename === 'object' &&
566+
(typeof options === 'function' || options === undefined)) {
567+
name = null
568+
opts = filename
569+
}
570+
564571
// set Content-Disposition when file is sent
565572
var headers = {
566573
'Content-Disposition': contentDisposition(name || path)

‎test/res.download.js

+198-62
Original file line numberDiff line numberDiff line change
@@ -86,46 +86,12 @@ describe('res', function(){
8686
})
8787
})
8888

89-
describe('.download(path, filename, fn)', function(){
90-
it('should invoke the callback', function(done){
91-
var app = express();
92-
var cb = after(2, done);
93-
94-
app.use(function(req, res){
95-
res.download('test/fixtures/user.html', 'document', cb)
96-
});
97-
98-
request(app)
99-
.get('/')
100-
.expect('Content-Type', 'text/html; charset=UTF-8')
101-
.expect('Content-Disposition', 'attachment; filename="document"')
102-
.expect(200, cb);
103-
})
104-
})
105-
106-
describe('.download(path, filename, options, fn)', function () {
107-
it('should invoke the callback', function (done) {
108-
var app = express()
109-
var cb = after(2, done)
110-
var options = {}
111-
112-
app.use(function (req, res) {
113-
res.download('test/fixtures/user.html', 'document', options, cb)
114-
})
115-
116-
request(app)
117-
.get('/')
118-
.expect(200)
119-
.expect('Content-Type', 'text/html; charset=UTF-8')
120-
.expect('Content-Disposition', 'attachment; filename="document"')
121-
.end(cb)
122-
})
123-
89+
describe('.download(path, options)', function () {
12490
it('should allow options to res.sendFile()', function (done) {
12591
var app = express()
12692

12793
app.use(function (req, res) {
128-
res.download('test/fixtures/.name', 'document', {
94+
res.download('test/fixtures/.name', {
12995
dotfiles: 'allow',
13096
maxAge: '4h'
13197
})
@@ -134,51 +100,124 @@ describe('res', function(){
134100
request(app)
135101
.get('/')
136102
.expect(200)
137-
.expect('Content-Disposition', 'attachment; filename="document"')
103+
.expect('Content-Disposition', 'attachment; filename=".name"')
138104
.expect('Cache-Control', 'public, max-age=14400')
139105
.expect(utils.shouldHaveBody(Buffer.from('tobi')))
140106
.end(done)
141107
})
142108

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

147113
app.use(function (req, res) {
148-
res.download('test/fixtures/user.html', 'document', {
114+
res.download('test/fixtures/user.html', {
149115
headers: {
150-
'Content-Type': 'text/x-custom',
151-
'Content-Disposition': 'inline'
116+
'X-Foo': 'Bar',
117+
'X-Bar': 'Foo'
152118
}
153119
})
154120
})
155121

156122
request(app)
157-
.get('/')
158-
.expect(200)
159-
.expect('Content-Type', 'text/x-custom')
160-
.expect('Content-Disposition', 'attachment; filename="document"')
161-
.end(done)
123+
.get('/')
124+
.expect(200)
125+
.expect('X-Foo', 'Bar')
126+
.expect('X-Bar', 'Foo')
127+
.end(done)
162128
})
163129

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

167133
app.use(function (req, res) {
168-
res.download('test/fixtures/user.html', 'document', {
134+
res.download('test/fixtures/user.html', {
169135
headers: {
170-
'content-type': 'text/x-custom',
171-
'content-disposition': 'inline'
136+
'X-Foo': 'Bar',
137+
'x-foo': 'bar'
172138
}
173139
})
174140
})
175141

176142
request(app)
177-
.get('/')
178-
.expect(200)
179-
.expect('Content-Type', 'text/x-custom')
180-
.expect('Content-Disposition', 'attachment; filename="document"')
181-
.end(done)
143+
.get('/')
144+
.expect(200)
145+
.expect('X-Foo', 'bar')
146+
.end(done)
147+
})
148+
149+
it('should override Content-Type', function (done) {
150+
var app = express()
151+
152+
app.use(function (req, res) {
153+
res.download('test/fixtures/user.html', {
154+
headers: {
155+
'Content-Type': 'text/x-custom'
156+
}
157+
})
158+
})
159+
160+
request(app)
161+
.get('/')
162+
.expect(200)
163+
.expect('Content-Type', 'text/x-custom')
164+
.end(done)
165+
})
166+
167+
it('should not set headers on 404', function (done) {
168+
var app = express()
169+
170+
app.use(function (req, res) {
171+
res.download('test/fixtures/does-not-exist', {
172+
headers: {
173+
'X-Foo': 'Bar'
174+
}
175+
})
176+
})
177+
178+
request(app)
179+
.get('/')
180+
.expect(404)
181+
.expect(utils.shouldNotHaveHeader('X-Foo'))
182+
.end(done)
183+
})
184+
185+
describe('when headers contains Content-Disposition', function () {
186+
it('should be ignored', function (done) {
187+
var app = express()
188+
189+
app.use(function (req, res) {
190+
res.download('test/fixtures/user.html', {
191+
headers: {
192+
'Content-Disposition': 'inline'
193+
}
194+
})
195+
})
196+
197+
request(app)
198+
.get('/')
199+
.expect(200)
200+
.expect('Content-Disposition', 'attachment; filename="user.html"')
201+
.end(done)
202+
})
203+
204+
it('should be ignored case-insensitively', function (done) {
205+
var app = express()
206+
207+
app.use(function (req, res) {
208+
res.download('test/fixtures/user.html', {
209+
headers: {
210+
'content-disposition': 'inline'
211+
}
212+
})
213+
})
214+
215+
request(app)
216+
.get('/')
217+
.expect(200)
218+
.expect('Content-Disposition', 'attachment; filename="user.html"')
219+
.end(done)
220+
})
182221
})
183222
})
184223

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

189228
app.use(function (req, res) {
190-
res.download('name.txt', 'document', {
229+
res.download('name.txt', {
191230
root: FIXTURES_PATH
192231
})
193232
})
194233

195234
request(app)
196235
.get('/')
197236
.expect(200)
198-
.expect('Content-Disposition', 'attachment; filename="document"')
237+
.expect('Content-Disposition', 'attachment; filename="name.txt"')
199238
.expect(utils.shouldHaveBody(Buffer.from('tobi')))
200239
.end(done)
201240
})
@@ -204,15 +243,15 @@ describe('res', function(){
204243
var app = express()
205244

206245
app.use(function (req, res) {
207-
res.download('fake/../name.txt', 'document', {
246+
res.download('fake/../name.txt', {
208247
root: FIXTURES_PATH
209248
})
210249
})
211250

212251
request(app)
213252
.get('/')
214253
.expect(200)
215-
.expect('Content-Disposition', 'attachment; filename="document"')
254+
.expect('Content-Disposition', 'attachment; filename="name.txt"')
216255
.expect(utils.shouldHaveBody(Buffer.from('tobi')))
217256
.end(done)
218257
})
@@ -224,7 +263,7 @@ describe('res', function(){
224263
var p = '..' + path.sep +
225264
path.relative(path.dirname(FIXTURES_PATH), path.join(FIXTURES_PATH, 'name.txt'))
226265

227-
res.download(p, 'document', {
266+
res.download(p, {
228267
root: FIXTURES_PATH
229268
})
230269
})
@@ -240,7 +279,7 @@ describe('res', function(){
240279
var app = express()
241280

242281
app.use(function (req, res) {
243-
res.download('../name.txt', 'document', {
282+
res.download('../name.txt', {
244283
root: FIXTURES_PATH
245284
})
246285
})
@@ -254,6 +293,103 @@ describe('res', function(){
254293
})
255294
})
256295

296+
describe('.download(path, filename, fn)', function(){
297+
it('should invoke the callback', function(done){
298+
var app = express();
299+
var cb = after(2, done);
300+
301+
app.use(function(req, res){
302+
res.download('test/fixtures/user.html', 'document', cb)
303+
});
304+
305+
request(app)
306+
.get('/')
307+
.expect('Content-Type', 'text/html; charset=UTF-8')
308+
.expect('Content-Disposition', 'attachment; filename="document"')
309+
.expect(200, cb);
310+
})
311+
})
312+
313+
describe('.download(path, filename, options, fn)', function () {
314+
it('should invoke the callback', function (done) {
315+
var app = express()
316+
var cb = after(2, done)
317+
var options = {}
318+
319+
app.use(function (req, res) {
320+
res.download('test/fixtures/user.html', 'document', options, cb)
321+
})
322+
323+
request(app)
324+
.get('/')
325+
.expect(200)
326+
.expect('Content-Type', 'text/html; charset=UTF-8')
327+
.expect('Content-Disposition', 'attachment; filename="document"')
328+
.end(cb)
329+
})
330+
331+
it('should allow options to res.sendFile()', function (done) {
332+
var app = express()
333+
334+
app.use(function (req, res) {
335+
res.download('test/fixtures/.name', 'document', {
336+
dotfiles: 'allow',
337+
maxAge: '4h'
338+
})
339+
})
340+
341+
request(app)
342+
.get('/')
343+
.expect(200)
344+
.expect('Content-Disposition', 'attachment; filename="document"')
345+
.expect('Cache-Control', 'public, max-age=14400')
346+
.expect(utils.shouldHaveBody(Buffer.from('tobi')))
347+
.end(done)
348+
})
349+
350+
describe('when options.headers contains Content-Disposition', function () {
351+
it('should be ignored', function (done) {
352+
var app = express()
353+
354+
app.use(function (req, res) {
355+
res.download('test/fixtures/user.html', 'document', {
356+
headers: {
357+
'Content-Type': 'text/x-custom',
358+
'Content-Disposition': 'inline'
359+
}
360+
})
361+
})
362+
363+
request(app)
364+
.get('/')
365+
.expect(200)
366+
.expect('Content-Type', 'text/x-custom')
367+
.expect('Content-Disposition', 'attachment; filename="document"')
368+
.end(done)
369+
})
370+
371+
it('should be ignored case-insensitively', function (done) {
372+
var app = express()
373+
374+
app.use(function (req, res) {
375+
res.download('test/fixtures/user.html', 'document', {
376+
headers: {
377+
'content-type': 'text/x-custom',
378+
'content-disposition': 'inline'
379+
}
380+
})
381+
})
382+
383+
request(app)
384+
.get('/')
385+
.expect(200)
386+
.expect('Content-Type', 'text/x-custom')
387+
.expect('Content-Disposition', 'attachment; filename="document"')
388+
.end(done)
389+
})
390+
})
391+
})
392+
257393
describe('on failure', function(){
258394
it('should invoke the callback', function(done){
259395
var app = express();

0 commit comments

Comments
 (0)