Skip to content

Commit

Permalink
Updates
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
BobbyWibowo committed Nov 28, 2018
1 parent 6e160fa commit 6543a87
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 21 deletions.
5 changes: 5 additions & 0 deletions config.sample.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
48 changes: 29 additions & 19 deletions controllers/uploadController.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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.')
}
}

Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
})
Expand Down Expand Up @@ -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
})
Expand Down Expand Up @@ -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) : ''
Expand Down
4 changes: 4 additions & 0 deletions controllers/utilsController.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ''
Expand Down
2 changes: 1 addition & 1 deletion public/js/home.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() }
Expand Down
2 changes: 1 addition & 1 deletion views/_globals.njk
Original file line number Diff line number Diff line change
Expand Up @@ -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" %}

Expand Down

1 comment on commit 6543a87

@BobbyWibowo
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTICE!
The reworked unique name generator will try to read file list of the uploads directory, then filter the names, to check for matching identifier.
This will likely be a few times slower than its old implementation of just checking whether a file with the exact same name exist or not.
It should generally be fine however.

Please sign in to comment.