diff --git a/lib/storage-handler.js b/lib/storage-handler.js index 96080ea..52733a2 100644 --- a/lib/storage-handler.js +++ b/lib/storage-handler.js @@ -204,16 +204,6 @@ exports.upload = function(provider, req, res, options, cb) { }); }; -function handleError(res, err) { - if (err.code === 'ENOENT') { - res.type('application/json'); - res.status(404).send({error: err}); - return; - } - res.type('application/json'); - res.status(500).send({error: err}); -} - /** * Handle download from a container/file. * @param {Object} provider The storage service provider @@ -248,7 +238,7 @@ exports.download = function(provider, req, res, container, file, cb) { provider.getFile(params.container, params.remote, function(err, stats) { if (err) { - return handleError(res, err); + return cb(processError(err, params.remote)); } setupPartialDownload(params, stats, res); @@ -261,11 +251,15 @@ exports.download = function(provider, req, res, container, file, cb) { res.type(fileName); reader.pipe(res); + reader.on('error', function onReaderError(err) { - handleError(res, err); + cb(processError(err, params.remote)); + cb = function() {}; // avoid double-callback }); + reader.on('end', function onReaderEnd() { cb(); + cb = function() {}; // avoid double-callback }); } }; @@ -287,3 +281,13 @@ function setupPartialDownload(params, stats, res) { res.set('Accept-Ranges', 'bytes'); res.set('Content-Length', chunksize); }; + +function processError(err, fileName) { + if (err.code === 'ENOENT') { + err.statusCode = err.status = 404; + // Hide the original message reported e.g. by FS provider, as it may + // contain sensitive information. + err.message = 'File not found: ' + fileName; + } + return err; +} diff --git a/test/upload-download.test.js b/test/upload-download.test.js index a7d3728..7bc16e6 100644 --- a/test/upload-download.test.js +++ b/test/upload-download.test.js @@ -314,6 +314,24 @@ describe('storage service', function() { }); }); + it('should run a function after a download failed', function(done) { + var hookCalled = false; + var Container = app.models.Container; + + Container.afterRemoteError('download', function(ctx, cb) { + hookCalled = true; + cb(); + }); + + request('http://localhost:' + app.get('port')) + .get('/containers/album1/download/does-not-exist') + .expect(404, function(err, res) { + if (err) return done(err); + assert(hookCalled, 'afterRemoteEror hook was not called'); + done(); + }); + }); + it('should delete a file', function(done) { request('http://localhost:' + app.get('port')) .del('/containers/album1/files/test.jpg')