Skip to content

Commit

Permalink
fix: improve image upload to filesystem may caused app crash
Browse files Browse the repository at this point in the history
Signed-off-by: Raccoon <raccoon@hackmd.io>
  • Loading branch information
a60814billy committed May 30, 2020
1 parent 4fdb7f8 commit 8b67d69
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 6 deletions.
46 changes: 44 additions & 2 deletions lib/imageRouter/filesystem.js
@@ -1,10 +1,39 @@
'use strict'

const crypto = require('crypto')
const fs = require('fs')
const URL = require('url').URL
const path = require('path')

const config = require('../config')
const logger = require('../logger')

/**
* generate a random filename for uploaded image
*/
function randomFilename () {
const buf = crypto.randomBytes(16)
return `upload_${buf.toString('hex')}`
}

/**
* pick a filename not exist in filesystem
* maximum attempt 5 times
*/
function pickFilename (defaultFilename) {
let retryCounter = 5
let filename = defaultFilename
const extname = path.extname(defaultFilename)
while (retryCounter-- > 0) {
if (fs.existsSync(path.join(config.uploadsPath, filename))) {
filename = `${randomFilename()}${extname}`
continue
}
return filename
}
throw new Error('file exists.')
}

exports.uploadImage = function (imagePath, callback) {
if (!imagePath || typeof imagePath !== 'string') {
callback(new Error('Image path is missing or wrong'), null)
Expand All @@ -16,11 +45,24 @@ exports.uploadImage = function (imagePath, callback) {
return
}

let filename = path.basename(imagePath)
try {
filename = pickFilename(path.basename(imagePath))
} catch (e) {
return callback(e, null)
}

try {
fs.copyFileSync(imagePath, path.join(config.uploadsPath, filename))
} catch (e) {
return callback(e, null)
}

let url
try {
url = (new URL(path.basename(imagePath), config.serverURL + '/uploads/')).href
url = (new URL(filename, config.serverURL + '/uploads/')).href
} catch (e) {
url = config.serverURL + '/uploads/' + path.basename(imagePath)
url = config.serverURL + '/uploads/' + filename
}

callback(null, url)
Expand Down
7 changes: 3 additions & 4 deletions lib/imageRouter/index.js
@@ -1,5 +1,6 @@
'use strict'

const fs = require('fs')
const Router = require('express').Router
const formidable = require('formidable')

Expand All @@ -15,10 +16,6 @@ imageRouter.post('/uploadimage', function (req, res) {

form.keepExtensions = true

if (config.imageUploadType === 'filesystem') {
form.uploadDir = config.uploadsPath
}

form.parse(req, function (err, fields, files) {
if (err || !files.image || !files.image.path) {
response.errorForbidden(req, res)
Expand All @@ -29,6 +26,8 @@ imageRouter.post('/uploadimage', function (req, res) {

const uploadProvider = require('./' + config.imageUploadType)
uploadProvider.uploadImage(files.image.path, function (err, url) {
// remove temporary upload file, and ignore any error
fs.unlink(files.image.path, () => {})
if (err !== null) {
logger.error(err)
return res.status(500).end('upload image error')
Expand Down

0 comments on commit 8b67d69

Please sign in to comment.