From 6543a87b11036a27dc32e58e6016625179ba279c Mon Sep 17 00:00:00 2001 From: Bobby Wibowo Date: Thu, 29 Nov 2018 00:52:12 +0700 Subject: [PATCH] Updates Reworked unique name generator to prevent the same unique identifier from being used if it was already used with a different extension (e.i. If a file named aBcD.jpg already exists, then files such as aBcD.png or aBcD.txt may not exist). This is mainly to deal with the fact that thumbnails are only being saved as PNG, so if the same unique name is being used by multiple image/video extensions, then only one of them will have the proper thumbnail. If you already have existing files with matching unique name but varying extensions, unfortunately you can only deal with them manually for now (either allocating new unique names or deleting them altogether). Added a new config option to filter files with no extension. Files with no extensions will no longer have their original name appended to the allocated random name (e.i. A file named "textfile" used to become something like "aBcDtextfile", where "aBcD" was the allocated random name. Now it will only just become "aBcD"). In relation to that, utils.extname() function will now always return blank string if the file name does not seem to have any extension. Though files such as '.DS_Store' (basically anything that starts with a dot) will still be accepted. Examples: .hiddenfile => .hiddenfile .hiddenfile.sh => .sh .hiddenfile.001 => .hiddenfile.001 .hiddenfile.sh.001 => .sh.001 Simplified error messages of /api/upload/finishchunks. Most, if not all, of the error responses for /api/upload* will now have HTTP status code 400 (bad request) instead of 200 (ok). I plan to generalize this for the other API routes in the future. Updated home.js to properly handle formatted error message when the response's status code is not 200 (ok). Bumped v1 version string (due to home.js). --- config.sample.js | 5 ++++ controllers/uploadController.js | 48 ++++++++++++++++++++------------- controllers/utilsController.js | 4 +++ public/js/home.js | 2 +- views/_globals.njk | 2 +- 5 files changed, 40 insertions(+), 21 deletions(-) diff --git a/config.sample.js b/config.sample.js index 1a55d62..eee6993 100644 --- a/config.sample.js +++ b/config.sample.js @@ -72,6 +72,11 @@ module.exports = { '.profile' ], + /* + If set to true, files with no extensions will always be rejected. + */ + filterNoExtension: false, + /* Show hash of the current git commit in homepage. */ diff --git a/controllers/uploadController.js b/controllers/uploadController.js index 364b2b1..56583c1 100644 --- a/controllers/uploadController.js +++ b/controllers/uploadController.js @@ -64,7 +64,7 @@ const upload = multer({ const extname = utils.extname(file.originalname) if (uploadsController.isExtensionFiltered(extname)) { // eslint-disable-next-line standard/no-callback-literal - return cb(`${extname.substr(1).toUpperCase()} files are not permitted due to security reasons.`) + return cb(`${extname ? `${extname.substr(1).toUpperCase()} files` : 'Files with no extension'} are not permitted.`) } // Re-map Dropzone keys so people can manually use the API without prepending 'dz' @@ -81,7 +81,7 @@ const upload = multer({ return cb('Chunk error occurred. Total file size is larger than the maximum file size.') } else if (!chunkedUploads) { // eslint-disable-next-line standard/no-callback-literal - return cb('Chunked uploads is disabled at the moment.') + return cb('Chunked uploads are disabled at the moment.') } } @@ -90,8 +90,9 @@ const upload = multer({ }).array('files[]') uploadsController.isExtensionFiltered = extname => { + if (!extname && config.filterNoExtension) { return true } // If there are extensions that have to be filtered - if (config.extensionsFilter && config.extensionsFilter.length) { + if (extname && config.extensionsFilter && config.extensionsFilter.length) { const match = config.extensionsFilter.some(extension => extname === extension.toLowerCase()) if ((config.filterBlacklist && match) || (!config.filterBlacklist && !match)) { return true @@ -113,13 +114,21 @@ uploadsController.getFileNameLength = req => { uploadsController.getUniqueRandomName = (length, extension) => { return new Promise((resolve, reject) => { const access = i => { - const name = randomstring.generate(length) + extension - fs.access(path.join(uploadsDir, name), error => { - if (error) { return resolve(name) } - console.log(`A file named ${name} already exists (${++i}/${maxTries}).`) - if (i < maxTries) { return access(i) } - // eslint-disable-next-line prefer-promise-reject-errors - return reject('Sorry, we could not allocate a unique random name. Try again?') + const identifier = randomstring.generate(length) + // Read all files names from uploads directory, then filter matching names (as in the identifier) + fs.readdir(uploadsDir, (error, names) => { + if (error) { return reject(error) } + if (names.length) { + for (const name of names.filter(name => name.startsWith(identifier))) { + if (name.split('.')[0] === identifier) { + console.log(`Identifier ${identifier} is already used (${++i}/${maxTries}).`) + if (i < maxTries) { return access(i) } + // eslint-disable-next-line prefer-promise-reject-errors + return reject('Sorry, we could not allocate a unique random name. Try again?') + } + } + } + return resolve(identifier + extension) }) } access(0) @@ -157,7 +166,7 @@ uploadsController.actuallyUpload = async (req, res, user, albumid) => { const erred = error => { const isError = error instanceof Error if (isError) { console.error(error) } - res.json({ + res.status(400).json({ success: false, description: isError ? error.toString() : error }) @@ -203,7 +212,7 @@ uploadsController.actuallyUploadByUrl = async (req, res, user, albumid) => { const erred = error => { const isError = error instanceof Error if (isError) { console.error(error) } - res.json({ + res.status(400).json({ success: false, description: isError ? error.toString() : error }) @@ -319,26 +328,27 @@ uploadsController.actuallyFinishChunks = async (req, res, user, albumid) => { const erred = error => { const isError = error instanceof Error if (isError) { console.error(error) } - res.json({ + res.status(400).json({ success: false, description: isError ? error.toString() : error }) } const files = req.body.files - if (!files || !(files instanceof Array)) { return erred('Missing "files" property (Array).') } - if (!files.length) { return erred('"files" property can not be empty.') } + if (!files || !(files instanceof Array) || !files.length) { return erred('Invalid "files" property (Array).') } let iteration = 0 const infoMap = [] for (const file of files) { - if (!file.uuid || typeof file.uuid !== 'string') { return erred('Missing or empty "uuid" property (string).') } - if (typeof file.count !== 'number') { return erred('Missing "count" property (number).') } - if (file.count < 1) { return erred('"count" property can not be less than 1.') } + if (!file.uuid || typeof file.uuid !== 'string') { return erred('Invalid "uuid" property (string).') } + if (typeof file.count !== 'number' || file.count < 1) { return erred('Invalid "count" property (number).') } const uuidDir = path.join(chunksDir, file.uuid) fs.readdir(uuidDir, async (error, chunkNames) => { - if (error) { return erred(error) } + if (error) { + if (error.code === 'ENOENT') { return erred('UUID is not being used.') } + return erred(error) + } if (file.count < chunkNames.length) { return erred('Chunks count mismatch.') } const extension = typeof file.original === 'string' ? utils.extname(file.original) : '' diff --git a/controllers/utilsController.js b/controllers/utilsController.js index 40aace1..40b4d51 100644 --- a/controllers/utilsController.js +++ b/controllers/utilsController.js @@ -25,6 +25,10 @@ utilsController.mayGenerateThumb = extname => { utilsController.preserves = ['.tar.gz', '.tar.z', '.tar.bz2', '.tar.lzma', '.tar.lzo', '.tar.xz'] utilsController.extname = filename => { + // Always return blank string if the filename does not seem to have a valid extension + // Files such as .DS_Store (anything that starts with a dot, without any extension after) will still be accepted + if (!/\../.test(filename)) { return '' } + let lower = filename.toLowerCase() // due to this, the returned extname will always be lower case let multi = '' let extname = '' diff --git a/public/js/home.js b/public/js/home.js index 8b45f0a..0e6f9ba 100644 --- a/public/js/home.js +++ b/public/js/home.js @@ -282,7 +282,7 @@ page.prepareDropzone = function () { page.dropzone.on('error', function (file, error) { file.previewElement.querySelector('.progress').style.display = 'none' file.previewElement.querySelector('.name').innerHTML = file.name - file.previewElement.querySelector('.error').innerHTML = error + file.previewElement.querySelector('.error').innerHTML = error.description || error }) if (typeof page.prepareShareX === 'function') { page.prepareShareX() } diff --git a/views/_globals.njk b/views/_globals.njk index 3fb0d25..6395e8f 100644 --- a/views/_globals.njk +++ b/views/_globals.njk @@ -15,7 +15,7 @@ v2: Images and config files (manifest.json, browserconfig.xml, etc). v3: CSS and JS files (libs such as bulma, lazyload, etc). #} -{% set v1 = "LZ9JN4pnIf" %} +{% set v1 = "Me9KhOnP5M" %} {% set v2 = "Ii3JYKIhb0" %} {% set v3 = "8xbKOM7u3w" %}