Use file mime type rather than extension to check server side if image upload is a valid file

closes #705
- uses the file type passed by express/connect
- relies on the type being set correctly by the browser upload
- doesn't reread the file to check
This commit is contained in:
jamesbloomer 2013-09-19 07:26:26 +01:00
parent 0c545d5f2e
commit c215626d2b
2 changed files with 37 additions and 9 deletions

View File

@ -73,11 +73,12 @@ adminControllers = {
'uploader': function (req, res) {
var currentDate = moment(),
month = currentDate.format("MMM"),
year = currentDate.format("YYYY"),
month = currentDate.format('MMM'),
year = currentDate.format('YYYY'),
tmp_path = req.files.uploadimage.path,
dir = path.join('content/images', year, month),
ext = path.extname(req.files.uploadimage.name).toLowerCase(),
type = req.files.uploadimage.type,
basename = path.basename(req.files.uploadimage.name, ext).replace(/[\W]/gi, '_');
function renameFile(target_path) {
@ -105,13 +106,12 @@ adminControllers = {
});
}
// TODO: is it better to use file type eg. image/png?
if (ext === ".jpg" || ext === ".jpeg" || ext === ".png" || ext === ".gif") {
if (type === 'image/jpeg' || type === 'image/png' || type === 'image/gif') {
getUniqueFileName(dir, basename, ext, null, function (filename) {
renameFile(filename);
});
} else {
res.send(404, "Invalid filetype");
res.send(404, 'Invalid filetype');
}
},
'login': function (req, res) {

View File

@ -7,6 +7,7 @@ var testUtils = require('./testUtils'),
// Stuff we are testing
admin = require('../../server/controllers/admin');
describe('Admin Controller', function() {
describe('uploader', function() {
@ -30,7 +31,8 @@ describe('Admin Controller', function() {
describe('can not upload invalid file', function() {
it('should return 404 for invalid file type', function() {
res.send = sinon.stub();
req.files.uploadimage.name = "INVALID.FILE";
req.files.uploadimage.name = 'INVALID.FILE';
req.files.uploadimage.type = 'application/octet-stream'
admin.uploader(req, res);
res.send.calledOnce.should.be.true;
res.send.args[0][0].should.equal(404);
@ -38,13 +40,25 @@ describe('Admin Controller', function() {
});
});
describe('can not upload file with valid extension but invalid type', function() {
it('should return 404 for invalid file type', function() {
res.send = sinon.stub();
req.files.uploadimage.name = 'INVALID.jpg';
req.files.uploadimage.type = 'application/octet-stream'
admin.uploader(req, res);
res.send.calledOnce.should.be.true;
res.send.args[0][0].should.equal(404);
res.send.args[0][1].should.equal('Invalid filetype');
});
});
describe('valid file', function() {
var clock;
beforeEach(function() {
req.files.uploadimage.name = "IMAGE.jpg";
req.files.uploadimage.name = 'IMAGE.jpg';
req.files.uploadimage.type = 'image/jpeg';
sinon.stub(fs, 'mkdirs').yields();
sinon.stub(fs, 'copy').yields();
sinon.stub(fs, 'unlink').yields();
@ -69,8 +83,20 @@ describe('Admin Controller', function() {
admin.uploader(req, res);
});
it('can upload jpg with incorrect extension', function(done) {
req.files.uploadimage.name = 'IMAGE.xjpg';
clock = sinon.useFakeTimers(42);
sinon.stub(res, 'send', function(data) {
data.should.not.equal(404);
return done();
});
admin.uploader(req, res);
});
it('can upload png', function(done) {
req.files.uploadimage.name = "IMAGE.png";
req.files.uploadimage.name = 'IMAGE.png';
req.files.uploadimage.type = 'image/png';
clock = sinon.useFakeTimers(42);
sinon.stub(res, 'send', function(data) {
data.should.not.equal(404);
@ -81,7 +107,8 @@ describe('Admin Controller', function() {
});
it('can upload gif', function(done) {
req.files.uploadimage.name = "IMAGE.gif";
req.files.uploadimage.name = 'IMAGE.gif';
req.files.uploadimage.type = 'image/gif';
clock = sinon.useFakeTimers(42);
sinon.stub(res, 'send', function(data) {
data.should.not.equal(404);
@ -145,6 +172,7 @@ describe('Admin Controller', function() {
});
it('should not leave temporary file when uploading', function(done) {
// Sun Sep 08 2013 10:57
clock = sinon.useFakeTimers(new Date(2013, 8, 8, 10, 57).getTime());
sinon.stub(res, 'send', function(data) {
fs.unlink.calledOnce.should.be.true;