Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Refactored FileManager internals & behavior

+ Changed FileManager::expect() behavior & argument format
+ Allow restriction on mimeType
+ Allow restriction on maxFilesize
+ Add FileManager::removeFile(file, cb) => removes a file and unlinks from instance
+ Add FileManager::allow(mime1, mime2) => allows specific mimetypes (globally on instance)
+ Add FileManager::maxFilesize(size) => allows restriction on filesize (globally on instance)
+ Add FileManager::allowEmpty() => allows empty files for instance
  • Loading branch information...
commit fecfe2852df583249ff2108588d4595d9549b6a8 1 parent 22ffda3
@mendezcode mendezcode authored
View
2  .gitignore
@@ -14,3 +14,5 @@
/test/fixtures/*skeleton/public/css/*.css
/test/fixtures/*skeleton/public/css/subdir/*.css
/test/fixtures/*skeleton/log/*.log
+/test/fixtures/*skeleton/incoming/*
+
View
249 middleware/body_parser/file_manager.js
@@ -1,7 +1,8 @@
-
+
/* File Manager */
var app = protos.app,
+ _ = require('underscore'),
fs = require('fs'),
slice = Array.prototype.slice;
@@ -15,40 +16,38 @@ function FileManager(files) {
return Object.keys(this.files);
});
+ Object.defineProperty(this, 'defaults', {
+ value: {
+ maxFilesize: 0,
+ mimeTypes: [],
+ noEmptyFiles: true
+ },
+ writable: true,
+ enumerable: false,
+ configurable: false
+ });
+
+ // Removed files
+ Object.defineProperty(this, 'removed', {
+ value: [],
+ writable: true,
+ enumerable: false,
+ configurable: false
+ });
+
// Instance files
Object.defineProperty(this, 'files', {
value: protos.extend({}, files),
writable: true,
enumerable: false,
- configurable: true
+ configurable: false
});
-
+
}
/**
- Expects files to match arguments. Other files not matching the
- ones listed in the expect arguments, will be removed silently,
- logging any errors encountered removing the files.
-
- You can define specific conditions for every file:
-
- a) 'filename': Expecting 'filename'. It's ok if it doens't exist.
-
- b) '*filename': File should be present. If the condition is not satisfied, the upload
- becomes invalid and all uploaded files are removed.
-
- c) '**filename': File should be present and it should not be empty. If the condition
- is not satisfied, the upload becomes invalid and all uploaded files are removed.
-
- Examples:
-
- if ( fm.expect('fileA', 'fileB', '*fileC', '**fileD') ) {
- fm.forEach(function(file) {
- res.json(file);
- });
- } else {
- res.end('400 Bad Request');
- }
+ Expects files to match arguments. Other files not matching the ones listed in the expect arguments,
+ will be removed silently, logging any errors encountered removing the files.
Any files that are not expected will be automatically removed as a security measure.
@@ -57,133 +56,162 @@ function FileManager(files) {
@public
*/
-FileManager.prototype.expect = function() {
- var expected = slice.call(arguments, 0);
+FileManager.prototype.expect = function(defs) {
- if (expected.length === 0) return true;
+ var log = function(err) {
+ if (err) (app.errorLog || app.log)(err);
+ }
- var files = this.files,
- emptyCheck = {},
- skip = [];
-
- // Detect which files to skip
- for (var file, o, max=expected.length, i=0; i < max; i++) {
- file = expected[i];
+ // Iterate over each definition
+ for (var file in defs) {
+
+ // Requires an object
+ var opts = defs[file];
- if (typeof file == 'object') {
- o = file;
- file = o.name;
- if (o.notEmpty) {
- file = '**' + file;
- } else if (o.required) {
- file = '*' + file;
- }
+ // Set maxfilesize
+ if (!opts.maxFilesize && this.defaults.maxFilesize) {
+ opts.maxFilesize = this.defaults.maxFilesize;
+ }
+
+ // Set mimetype
+ if (typeof opts.type == 'string') {
+ opts.type = [opts.type].concat(this.defaults.mimeTypes);
+ } else if (opts.type instanceof Array) {
+ opts.type = _.unique(opts.type.concat(this.defaults.mimeTypes));
+ } else {
+ opts.type = this.defaults.mimeTypes;
}
- if (file.charAt(0) == '*' ) {
- // File is required, don't accept uploaded files
- file = file.slice(1);
+ // Process file
+ if (file in this.files) {
- // Also check if file is not empty
- if (file.charAt(0) == '*') {
- file = file.slice(1);
- emptyCheck[file] = true;
+ // File present in uploads
+
+ var f = this.files[file];
+
+ // Remove empty files if present
+ if (this.defaults.noEmptyFiles) {
+ if (f.size === 0) {
+ this.removeFile(file);
+ continue;
+ }
}
- if (! (file in files) ) {
- this.removeAll(); // Invalidate upload, remove any uploaded files
- return false; // Exit function, Fail: file not present
- } else if (emptyCheck[file] && files[file].size === 0) {
- this.removeAll(); // Invalidate upload, remove any uploaded files
- return false; // Exit function, Fail: file is empty
- } else {
- skip.push(file); // Skip file from removal
+ // If Filesize restriction not met
+ if (opts.maxFilesize && f.size > opts.maxFilesize) {
+ this.removeFile(file);
+ continue;
}
- } else {
- // File is optional
- if (file in files) {
- skip.push(file); // Skip file from removal
+
+ // If Mimetype restriction not met
+ if (opts.type.length > 0 && opts.type.indexOf(f.type || f.mime) === -1) {
+ this.removeFile(file);
+ continue;
}
+
}
+
}
- // Logging function, reports any issues encountered removing files
- var log = function(err) {
- if (err) app.log(err);
- }
-
- // Remove any files not in skip
- for (file in files) {
- if (skip.indexOf(file) >= 0) continue;
- else {
- fs.unlink(files[file].path, log);
- delete files[file];
+ // Remove any files not in definitions
+ for (file in this.files) {
+ if (!(file in defs)) {
+ this.removeFile(file);
}
}
- return true; // success
-
+ return this.files;
+
}
/**
- Gets a specific file
+ Removes a file
+ @method removeFile
@param {string} file
- @return {object} file data
- @public
+ @return {object} instance
*/
-
-FileManager.prototype.get = function(file) {
- return this.files[file];
+
+FileManager.prototype.removeFile = function(file, callback) {
+ if (file in this.files) {
+ this.removed.push(file);
+ fs.unlink(this.files[file].path, callback || logCallback);
+ delete this.files[file];
+ }
+ return this;
}
/**
- Removes any empty files that have been uploaded
+ Sets the default maxFilesize configuration
- @public
+ @method maxFilesize
+ @param {int} size Filesize to expect in bytes
+ @return {object} instance
*/
-FileManager.prototype.removeEmpty = function() {
- var files = this.files;
-
- var log = function(err) {
- if (err) app.log(err);
- }
+FileManager.prototype.maxFilesize = function(size) {
+ this.defaults.maxFilesize = size;
+ return this;
+}
+
+/*
+ Sets the filemanager configuration to allow specific mime types
- for (var file in files) {
- if (files[file].size === 0) {
- fs.unlink(files[file].path, log);
- delete files[file];
- }
- }
+ @method allow
+ @param {str} Mime type to allow (multiple args)
+ @return {object} instance;
+ */
+
+FileManager.prototype.allow = function() {
+ var args = slice.call(arguments, 0);
+ this.defaults.mimeTypes = _.unique(this.defaults.mimeTypes.concat(args));
+ return this;
+}
+
+/*
+ Sets the filemanager configuration to allow empty files
+ @method allowEmpty
+ @return {object} instance;
+ */
+
+FileManager.prototype.allowEmpty = function() {
+ this.defaults.noEmptyFiles = false;
return this;
}
/**
- Removes all files uploaded
+ Gets a specific file
+ @method get
+ @param {string} file
+ @return {object} file data
@public
*/
+
+FileManager.prototype.get = function(file) {
+ return this.files[file] || null;
+}
-FileManager.prototype.removeAll = function() {
- var files = this.files;
-
- var log = function(err) {
- if (err) app.log(err);
- }
+/**
+ Removes all files uploaded
- for (var file in files) {
- fs.unlink(files[file].path, log);
- delete files[file];
+ @method removeAll
+ @param {function} callback Callback to pass to each fs.unlink call (optional)
+ @public
+ */
+
+FileManager.prototype.removeAll = function(callback) {
+ for (var file in this.files) {
+ this.removeFile(file, callback);
}
-
return this;
}
/**
Iterates over the files uploaded
+ @method forEach
@param {function} callback
@public
*/
@@ -191,8 +219,13 @@ FileManager.prototype.removeAll = function() {
FileManager.prototype.forEach = function(callback) {
var files = this.files;
for (var key in files) {
- callback.call(this, files[key]);
+ callback.call(this, key, files[key]);
}
+ return this.files;
+}
+
+function logCallback(err) {
+ if (err) (app.errorLog || app.log)(err);
}
module.exports = FileManager;
View
7 test/fixtures/test.skeleton/app/controllers/test_controller.js
@@ -69,8 +69,11 @@ function TestController(app) {
var uploadCb;
post('/upload', uploadCb = function(req, res) {
req.getRequestData(function(fields, files) {
- if ( files.expect('**file') ) { // File should be present, and not empty
- var f = files.get('file');
+ files.expect({
+ file: {}
+ });
+ var f = files.get('file');
+ if ( f ) { // File should be present, and not empty
res.sendHeaders();
res.end(inspect(f));
files.removeAll();
View
BIN  test/fixtures/test.skeleton/incoming/199a0a7e5d6fc7cb4e359a2eb41e8b12.junk
Binary file not shown
View
381 test/middleware/body_parser-filemanager-unit.js
@@ -3,16 +3,17 @@ var app = require('../fixtures/bootstrap'),
vows = require('vows'),
fs = require('fs'),
assert = require('assert'),
- EventEmitter = require('events').EventEmitter;
+ EventEmitter = require('events').EventEmitter,
+ Multi = protos.require('multi');
var FileManager;
-var t = 100; // Time to wait for Disk I/O to complete
-
var files = {
- fileA: {path: app.fullPath('/incoming/a.txt'), size: 16},
- fileB: {path: app.fullPath('/incoming/b.txt'), size: 0},
- fileC: {path: app.fullPath('/incoming/c.txt'), size: 0},
+ alpha: {path: app.fullPath('incoming/alpha.txt'), size: 16, type: 'text/plain'},
+ beta: {path: app.fullPath('incoming/beta.jpg'), size: 16, type: 'image/jpg'},
+ gamma: {path: app.fullPath('incoming/gamma.gif'), size: 16, type: 'image/gif'},
+ epsilon: {path: app.fullPath('incoming/epsilon.txt'), size: 0, type: 'text/plain'},
+ delta: {path: app.fullPath('incoming/delta.png'), size: 5, type: 'image/png'},
}
// Simulates file uploads
@@ -30,197 +31,329 @@ vows.describe('Body Parser (middleware) » FileManager').addBatch({
topic: function() {
- app.use('body_parser');
-
+ var results = [];
+
+ createFiles();
+
+ if (!app.supports.body_parser) app.use('body_parser');
+
FileManager = app.resources.body_parser.file_manager;
+
+ // File expected not present (required)
+ var fm = new FileManager(files);
+ fm.expect({
+ other: {}
+ });
+ results.push(fm);
- loggingStatus = app.logging;
+ // File expected not present (not required)
+ createFiles();
+ fm = new FileManager(files);
+ fm.expect({
+ other: {}
+ });
+ results.push(fm);
- app.logging = false;
+ // File expected present
+ createFiles();
+ fm = new FileManager(files);
+ fm.expect({
+ alpha: {}
+ });
+ results.push(fm);
- var fm, results = [],
- promise = new EventEmitter();
+ // Expected files with wrong mimetypes and size
+ createFiles();
+ fm = new FileManager(files);
+ fm.expect({
+ alpha: {
+ maxFilesize: 10 // will fail, its size is 16 bytes
+ },
+ beta: {
+ required: true,
+ type: 'text/plain' // will fail, it's mimetype is image/jpg
+ }
+ });
+ results.push(fm);
- // Using setTimeout to compensate for Disk I/O
+ // Globally restricts on maxFilesize
+ createFiles();
+ fm = new FileManager(files);
+ fm.maxFilesize(10).expect({
+ alpha: {
+ maxFilesize: 20 // overrides restriction
+ },
+ beta: {},
+ gamma: {},
+ epsilon: {},
+ delta: {}
+ });
+ results.push(fm);
- // Returns true + removes files not expected for optional files
+ // Globally restricts on mimeType
createFiles();
fm = new FileManager(files);
- fm.__expectResult = fm.expect('fileC', 'fileX', {
- name: 'fileA',
- required: true,
- notEmpty: true
- },{
- name: 'fileB',
- required: true
+ fm.allow('image/png', 'image/jpg').expect({
+ alpha: {
+ type: ['text/javascript', 'text/plain', 'text/css']
+ },
+ beta: {},
+ gamma: {},
+ epsilon: {},
+ delta: {}
});
+ results.push(fm);
- setTimeout(function() {
- fm.__existChecks = [
- fs.existsSync(files.fileA.path),
- fs.existsSync(files.fileB.path),
- fs.existsSync(files.fileC.path)];
- results.push(fm);
-
- // Returns false + removes all files when required file not present
- createFiles();
- fm = new FileManager(files);
- fm.__expectResult = fm.expect('*fileX', 'fileA', 'fileB');
-
- setTimeout(function() {
- fm.__existChecks = [
- fs.existsSync(files.fileA.path),
- fs.existsSync(files.fileB.path),
- fs.existsSync(files.fileC.path)];
- results.push(fm);
-
- // Returns false + removes all files when required file empty
- createFiles();
- fm = new FileManager(files);
- fm.__expectResult = fm.expect('*fileA', '**fileB');
-
- setTimeout(function() {
- fm.__existChecks = [
- fs.existsSync(files.fileA.path),
- fs.existsSync(files.fileB.path),
- fs.existsSync(files.fileC.path)];
- results.push(fm);
-
- promise.emit('success', results);
- }, t);
-
- }, t);
-
- }, t);
+ // Globally restricts on mimeType and maxFilesize (only delta matches)
+ createFiles();
+ fm = new FileManager(files);
+ fm.maxFilesize(5).allow('image/png', 'image/jpg').expect({
+ alpha: {
+ type: ['text/javascript', 'text/plain', 'text/css']
+ },
+ beta: {},
+ gamma: {},
+ epsilon: {},
+ delta: {}
+ });
+ results.push(fm);
+
+ // Allows empty files if fm.config.noEmptyFiles is false
+ createFiles();
+ fm = new FileManager(files);
+ fm.allowEmpty().expect({
+ epsilon: {}
+ });
+ results.push(fm);
+
+ // Removes empty files automatically
+ createFiles();
+ fm = new FileManager(files);
+ fm.expect({
+ epsilon: {}
+ });
+ results.push(fm);
+
+ return results;
- return promise;
},
- 'Returns true + removes files not expected for optional files': function(results) {
+ 'Required File expected n/a => removes all files': function(results) {
+
var fm = results[0];
- assert.isTrue(fm.__expectResult);
- assert.equal(fm.length,3);
- assert.deepEqual(Object.keys(fm.files), ['fileA', 'fileB', 'fileC']);
- assert.deepEqual(fm.__existChecks, [true, true, true]);
+ assert.equal(fm.length, 0);
+ assert.deepEqual(fm.removed, ['alpha', 'beta', 'gamma', 'epsilon', 'delta']); // All files removed
},
- 'Returns false + removes all files when required file not present': function(results) {
+ 'Optional File expected n/a => removes all files': function(results) {
var fm = results[1];
- assert.isFalse(fm.__expectResult);
assert.equal(fm.length, 0);
- assert.deepEqual(Object.keys(fm.files), []);
- assert.deepEqual(fm.__existChecks, [false, false, false]);
+ assert.deepEqual(fm.removed, ['alpha', 'beta', 'gamma', 'epsilon', 'delta']); // All files removed
},
- 'Returns false + removes all files when required file empty': function(results) {
+ 'File expected present => removes all except required': function(results) {
var fm = results[2];
- assert.isFalse(fm.__expectResult);
+ assert.equal(fm.length, 1);
+ assert.deepEqual(fm.removed, ['beta', 'gamma', 'epsilon', 'delta']); // All files except 'alpha' removed
+ },
+
+ 'Removes files not matching filesize and mimetype requirements': function(results) {
+ var fm = results[3];
assert.equal(fm.length, 0);
- assert.deepEqual(Object.keys(fm.files), []);
- assert.deepEqual(fm.__existChecks, [false, false, false]);
+ assert.deepEqual(fm.removed, ['alpha', 'beta', 'gamma', 'epsilon', 'delta']); // All files removed
+ },
+
+ 'Successfully executes restrictions on maxFilesize': function(results) {
+ var fm = results[4];
+ assert.equal(fm.length, 2);
+ assert.deepEqual(fm.removed, ['beta', 'gamma', 'epsilon']); // All files removed except 'alpha' and 'delta'
+ },
+
+ 'Successfully executes restrictions on mimeType': function(results) {
+ var fm = results[5];
+ assert.equal(fm.length, 3);
+ assert.deepEqual(fm.removed, ['gamma', 'epsilon']); // All files removed except 'alpha', 'beta' and 'delta'
+ },
+
+ 'Successfully restricts on maxFilesize and mimeType': function(results) {
+ var fm = results[6];
+ assert.equal(fm.length, 1);
+ assert.deepEqual(fm.removed, ['alpha', 'beta', 'gamma', 'epsilon']); // All files removed except 'delta'
+ },
+
+ 'Allows empty files if config.noEmptyFiles is false': function(results) {
+ var fm = results[7];
+ assert.equal(fm.length, 1);
+ assert.deepEqual(fm.removed, ['alpha', 'beta', 'gamma', 'delta']);
+ assert.deepEqual(fm.files, {
+ epsilon: files.epsilon
+ });
+ },
+
+ 'Removes empty files by default': function(results) {
+ var fm = results[8];
+ assert.equal(fm.length, 0);
+ assert.deepEqual(fm.removed, ['epsilon', 'alpha', 'beta', 'gamma', 'delta']);
+ assert.deepEqual(fm.files, {});
+ },
+
+ 'Returns object with files (filtered)': function() {
+ var fm = new FileManager(files);
+ assert.isTrue(fm.files === fm.expect({}));
}
}
}).addBatch({
- 'FileManager::removeEmpty': {
+ 'FileManager::get': {
topic: function() {
- var promise = new EventEmitter();
+
+ var results = [];
createFiles();
var fm = new FileManager(files);
- fm.removeEmpty();
- setTimeout(function() {
- fm.__existChecks = [
- fs.existsSync(files.fileA.path),
- fs.existsSync(files.fileB.path),
- fs.existsSync(files.fileC.path) ];
- promise.emit('success', fm);
- }, t);
+ fm.expect({
+ gamma: {
+ type: 'image/gif'
+ }
+ });
+
+ return fm;
- return promise;
},
- 'Removes all empty files': function(fm) {
- assert.equal(fm.length, 1);
- assert.deepEqual(fm.__existChecks, [true, false, false]);
+ 'Returns the file object if it exists': function(fm) {
+ assert.deepEqual(fm.get('gamma'), files.gamma);
+ },
+
+ 'Returns null if file not present': function(fm) {
+ assert.isNull(fm.get('hello'));
}
}
}).addBatch({
- 'FileManager::removeAll': {
+ 'FileManager::removeFile': {
topic: function() {
+
var promise = new EventEmitter();
-
- var fm = new FileManager(files);
- fm.removeAll();
-
- setTimeout(function() {
- fm.__existChecks = [
- fs.existsSync(files.fileA.path),
- fs.existsSync(files.fileB.path),
- fs.existsSync(files.fileC.path) ];
- promise.emit('success', fm);
- }, t);
-
+ var testFilePath = app.fullPath('incoming/test-file.txt');
+
+ fs.writeFileSync(testFilePath, '', 'utf-8');
+
+ var fm = new FileManager({
+ test_file: {
+ path: testFilePath,
+ size: 16,
+ type: 'text/plain'
+ }
+ });
+
+ var results = [fm.removed.concat([])]; // Using concat to get a copy of the array
+
+ fm.removeFile('test_file', function(err) {
+
+ results.push(err);
+
+ fs.exists(testFilePath, function(exists) {
+
+ results.push(exists);
+
+ results.push(fm.removed);
+
+ promise.emit('success', results);
+
+ });
+
+ });
+
return promise;
+
},
-
- 'Removes all files': function(fm) {
- assert.equal(fm.length, 0);
- assert.deepEqual(fm.__existChecks, [false, false, false]);
- }
+ 'Successfully removes files': function(results) {
+ assert.deepEqual(results[0], []) // removed array empty
+ assert.isNull(results[1]); // null => no errors found removing file
+ assert.isFalse(results[2]); // false => file does not exist
+ assert.deepEqual(results[3], ['test_file']); // removed array contains removed file
+ }
+
}
}).addBatch({
- 'FileManager::get': {
+ 'FileManager::forEach': {
topic: function() {
- var results = [];
+
var fm = new FileManager(files);
- results.push(fm.get('fileA'));
- results.push(fm.get('fileX'));
+
+ var results = {};
+
+ fm.forEach(function(file, data) {
+ results[file] = data;
+ });
+
return results;
+
},
- 'Returns valid file object for existing files': function(results) {
- var r = results[0];
- assert.deepEqual(r, files.fileA);
- },
-
- 'Returns undefined for inexistent files': function(results) {
- var r = results[1];
- assert.isUndefined(r);
+ 'Properly iterates over files': function(results) {
+ assert.deepEqual(results, files);
}
}
}).addBatch({
- 'FileManager::forEach': {
+ 'FileManager::removeAll': {
topic: function() {
- app.logging = loggingStatus;
+ var promise = new EventEmitter();
- var results = [];
+ createFiles();
var fm = new FileManager(files);
- fm.forEach(function(file) {
- results.push(file);
+ var unlinkErrors = [];
+
+ fm.removeAll(function(err) {
+ unlinkErrors.push(err);
+ if (unlinkErrors.length == 5) {
+
+ var fsm = new Multi(fs);
+
+ for (var file in files) {
+ fsm.exists(files[file].path);
+ }
+
+ fsm.exec(function(err, results) {
+ promise.emit('success', {
+ fm: fm,
+ existResults: arguments,
+ unlinkErrors: unlinkErrors
+ })
+ });
+
+ }
});
- return results;
+
+ return promise;
+
},
- 'Iterates over the files contained': function(results) {
- assert.deepEqual(results, [files.fileA, files.fileB, files.fileC]);
+ 'Removes all files': function(results) {
+ assert.deepEqual(results.fm.files, {});
+ assert.deepEqual(results.fm.length, 0);
+ assert.deepEqual(results.fm.fileKeys, []);
+ assert.deepEqual(results.fm.removed, ['alpha', 'beta', 'gamma', 'epsilon', 'delta']);
+ assert.deepEqual(results.unlinkErrors, [null, null, null, null, null ]);
+ assert.isNull(results.existResults[0]);
+ assert.deepEqual(results.existResults[1], ['OK', 'OK', 'OK', 'OK', 'OK']); // Exists is false => no err => 'OK'
}
}
View
2  test/middleware/body_parser.js
@@ -58,7 +58,7 @@ vows.describe('Body Parser (middleware)').addBatch({
var r1 = results[0],
r2 = results[1];
-
+
// POST
assert.isTrue(r1.indexOf('HTTP/1.1 200 OK') >= 0);
assert.isTrue(r1.indexOf(expected) >= 0);
Please sign in to comment.
Something went wrong with that request. Please try again.