From 8b67d6916d0041eb13040a0d11903174d8ff586a Mon Sep 17 00:00:00 2001
From: Raccoon <raccoon@hackmd.io>
Date: Sun, 31 May 2020 00:41:01 +0800
Subject: [PATCH] fix: improve image upload to filesystem may caused app crash

Signed-off-by: Raccoon <raccoon@hackmd.io>
---
 lib/imageRouter/filesystem.js | 46 +++++++++++++++++++++++++++++++++--
 lib/imageRouter/index.js      |  7 +++---
 2 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/lib/imageRouter/filesystem.js b/lib/imageRouter/filesystem.js
index 53ecc9d4..49a811ef 100644
--- a/lib/imageRouter/filesystem.js
+++ b/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)
@@ -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)
diff --git a/lib/imageRouter/index.js b/lib/imageRouter/index.js
index 9b0c4028..5af005bc 100644
--- a/lib/imageRouter/index.js
+++ b/lib/imageRouter/index.js
@@ -1,5 +1,6 @@
 'use strict'
 
+const fs = require('fs')
 const Router = require('express').Router
 const formidable = require('formidable')
 
@@ -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)
@@ -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')