From 385f4f12be4dbd41832bebbe043288ee524a1bee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bar=C4=B1=C5=9F=20U=C5=9Fakl=C4=B1?= Date: Tue, 20 May 2025 10:45:56 -0400 Subject: [PATCH] replace connect-multiparty with Multer (#13439) * post upload route * more multer changes keep name and type fields in file objects so we dont break all plugins using these * remove log * fix: thumbs delete * test: add array check --- install/package.json | 3 +-- src/controllers/accounts/edit.js | 2 +- src/controllers/admin/uploads.js | 16 +++++++------ src/controllers/uploads.js | 11 ++++----- src/middleware/index.js | 41 ++++++++++++++++---------------- src/middleware/uploads.js | 2 +- src/routes/admin.js | 12 +++++++--- src/routes/api.js | 8 ++++--- src/routes/authentication.js | 12 +++++++--- src/routes/helpers.js | 6 +++-- src/routes/write/topics.js | 11 +++++---- test/helpers/index.js | 2 +- 12 files changed, 73 insertions(+), 53 deletions(-) diff --git a/install/package.json b/install/package.json index b3dca2f562..e56c9e056c 100644 --- a/install/package.json +++ b/install/package.json @@ -58,7 +58,6 @@ "compression": "1.8.0", "connect-flash": "0.1.1", "connect-mongo": "5.1.0", - "connect-multiparty": "2.2.0", "connect-pg-simple": "10.0.0", "connect-redis": "8.1.0", "cookie-parser": "1.4.7", @@ -95,7 +94,7 @@ "mongodb": "6.16.0", "morgan": "1.10.0", "mousetrap": "1.6.5", - "multiparty": "4.2.3", + "multer": "2.0.0", "nconf": "0.13.0", "nodebb-plugin-2factor": "7.5.10", "nodebb-plugin-composer-default": "10.2.50", diff --git a/src/controllers/accounts/edit.js b/src/controllers/accounts/edit.js index 61a13399a0..1192687a5f 100644 --- a/src/controllers/accounts/edit.js +++ b/src/controllers/accounts/edit.js @@ -143,7 +143,7 @@ async function renderRoute(name, req, res) { } editController.uploadPicture = async function (req, res, next) { - const userPhoto = req.files.files[0]; + const userPhoto = req.files[0]; try { const updateUid = await user.getUidByUserslug(req.params.userslug); const isAllowed = await privileges.users.canEdit(req.uid, updateUid); diff --git a/src/controllers/admin/uploads.js b/src/controllers/admin/uploads.js index 56d64674cf..433edd07ce 100644 --- a/src/controllers/admin/uploads.js +++ b/src/controllers/admin/uploads.js @@ -13,7 +13,9 @@ const image = require('../../image'); const plugins = require('../../plugins'); const pagination = require('../../pagination'); -const allowedImageTypes = ['image/png', 'image/jpeg', 'image/pjpeg', 'image/jpg', 'image/gif', 'image/svg+xml']; +const allowedImageTypes = [ + 'image/png', 'image/jpeg', 'image/pjpeg', 'image/jpg', 'image/gif', 'image/svg+xml', +]; const uploadsController = module.exports; @@ -147,7 +149,7 @@ async function getFileData(currentDir, file) { } uploadsController.uploadCategoryPicture = async function (req, res, next) { - const uploadedFile = req.files.files[0]; + const uploadedFile = req.files[0]; let params = null; try { @@ -202,7 +204,7 @@ async function sanitizeSvg(filePath) { } uploadsController.uploadFavicon = async function (req, res, next) { - const uploadedFile = req.files.files[0]; + const uploadedFile = req.files[0]; const allowedTypes = ['image/x-icon', 'image/vnd.microsoft.icon']; await validateUpload(uploadedFile, allowedTypes); @@ -217,7 +219,7 @@ uploadsController.uploadFavicon = async function (req, res, next) { }; uploadsController.uploadTouchIcon = async function (req, res, next) { - const uploadedFile = req.files.files[0]; + const uploadedFile = req.files[0]; const allowedTypes = ['image/png']; const sizes = [36, 48, 72, 96, 144, 192, 512]; @@ -244,7 +246,7 @@ uploadsController.uploadTouchIcon = async function (req, res, next) { uploadsController.uploadMaskableIcon = async function (req, res, next) { - const uploadedFile = req.files.files[0]; + const uploadedFile = req.files[0]; const allowedTypes = ['image/png']; await validateUpload(uploadedFile, allowedTypes); @@ -263,7 +265,7 @@ uploadsController.uploadLogo = async function (req, res, next) { }; uploadsController.uploadFile = async function (req, res, next) { - const uploadedFile = req.files.files[0]; + const uploadedFile = req.files[0]; let params; try { params = JSON.parse(req.body.params); @@ -294,7 +296,7 @@ uploadsController.uploadOgImage = async function (req, res, next) { }; async function upload(name, req, res, next) { - const uploadedFile = req.files.files[0]; + const uploadedFile = req.files[0]; await validateUpload(uploadedFile, allowedImageTypes); const filename = name + path.extname(uploadedFile.name); diff --git a/src/controllers/uploads.js b/src/controllers/uploads.js index d2e392b5d3..d2cfb48107 100644 --- a/src/controllers/uploads.js +++ b/src/controllers/uploads.js @@ -18,7 +18,7 @@ const uploadsController = module.exports; uploadsController.upload = async function (req, res, filesIterator) { let files; try { - files = req.files.files; + files = req.files; } catch (e) { return helpers.formatApiResponse(400, res); } @@ -27,9 +27,6 @@ uploadsController.upload = async function (req, res, filesIterator) { if (!Array.isArray(files)) { return helpers.formatApiResponse(500, res, new Error('[[error:invalid-file]]')); } - if (Array.isArray(files[0])) { - files = files[0]; - } try { const images = []; @@ -126,7 +123,7 @@ async function resizeImage(fileObj) { uploadsController.uploadThumb = async function (req, res) { if (!meta.config.allowTopicsThumbnail) { - deleteTempFiles(req.files.files); + deleteTempFiles(req.files); return helpers.formatApiResponse(503, res, new Error('[[error:topic-thumbnails-are-disabled]]')); } @@ -201,7 +198,9 @@ async function saveFileToLocal(uid, folder, uploadedFile) { } function deleteTempFiles(files) { - files.forEach(fileObj => file.delete(fileObj.path)); + if (Array.isArray(files)) { + files.forEach(fileObj => file.delete(fileObj.path)); + } } require('../promisify')(uploadsController, ['upload', 'uploadPost', 'uploadThumb']); diff --git a/src/middleware/index.js b/src/middleware/index.js index 5a0e69842f..417d8309bc 100644 --- a/src/middleware/index.js +++ b/src/middleware/index.js @@ -5,7 +5,6 @@ const validator = require('validator'); const nconf = require('nconf'); const toobusy = require('toobusy-js'); const util = require('util'); -const multipart = require('connect-multiparty'); const { csrfSynchronisedProtection } = require('./csrf'); const plugins = require('../plugins'); @@ -27,7 +26,7 @@ const delayCache = cacheCreate({ ttl: 1000 * 60, max: 200, }); -const multipartMiddleware = multipart(); + const middleware = module.exports; @@ -101,17 +100,30 @@ middleware.pluginHooks = helpers.try(async (req, res, next) => { }); middleware.validateFiles = function validateFiles(req, res, next) { - if (!req.files.files) { + if (!req.files) { return next(new Error(['[[error:invalid-files]]'])); } - - if (Array.isArray(req.files.files) && req.files.files.length) { - return next(); + function makeFilesCompatible(files) { + if (Array.isArray(files)) { + // multer uses originalname and mimetype, but we use name and type + files.forEach((file) => { + if (file.originalname) { + file.name = file.originalname; + } + if (file.mimetype) { + file.type = file.mimetype; + } + }); + } + next(); + } + if (Array.isArray(req.files) && req.files.length) { + return makeFilesCompatible(req.files); } - if (typeof req.files.files === 'object') { - req.files.files = [req.files.files]; - return next(); + if (typeof req.files === 'object') { + req.files = [req.files]; + return makeFilesCompatible(req.files); } return next(new Error(['[[error:invalid-files]]'])); @@ -291,14 +303,3 @@ middleware.checkRequired = function (fields, req, res, next) { controllers.helpers.formatApiResponse(400, res, new Error(`[[error:required-parameters-missing, ${missing.join(' ')}]]`)); }; - -middleware.handleMultipart = (req, res, next) => { - // Applies multipart handler on applicable content-type - const { 'content-type': contentType } = req.headers; - - if (contentType && !contentType.startsWith('multipart/form-data')) { - return next(); - } - - multipartMiddleware(req, res, next); -}; diff --git a/src/middleware/uploads.js b/src/middleware/uploads.js index d1ce5b09b2..9b434b3286 100644 --- a/src/middleware/uploads.js +++ b/src/middleware/uploads.js @@ -23,7 +23,7 @@ exports.ratelimit = helpers.try(async (req, res, next) => { ttl: meta.config.uploadRateLimitCooldown * 1000, }); } - const count = (cache.get(`${req.ip}:uploaded_file_count`) || 0) + req.files.files.length; + const count = (cache.get(`${req.ip}:uploaded_file_count`) || 0) + req.files.length; if (count > meta.config.uploadRateLimitThreshold) { return next(new Error(['[[error:upload-ratelimit-reached]]'])); } diff --git a/src/routes/admin.js b/src/routes/admin.js index 89a3050ee6..b7e751695c 100644 --- a/src/routes/admin.js +++ b/src/routes/admin.js @@ -82,10 +82,16 @@ function apiRoutes(router, name, middleware, controllers) { router.get(`/api/${name}/analytics`, middleware.ensureLoggedIn, helpers.tryRoute(controllers.admin.dashboard.getAnalytics)); router.get(`/api/${name}/advanced/cache/dump`, middleware.ensureLoggedIn, helpers.tryRoute(controllers.admin.cache.dump)); - const multipart = require('connect-multiparty'); - const multipartMiddleware = multipart(); + const multer = require('multer'); + const storage = multer.diskStorage({}); + const upload = multer({ storage }); - const middlewares = [multipartMiddleware, middleware.validateFiles, middleware.applyCSRF, middleware.ensureLoggedIn]; + const middlewares = [ + upload.array('files[]', 20), + middleware.validateFiles, + middleware.applyCSRF, + middleware.ensureLoggedIn, + ]; router.post(`/api/${name}/category/uploadpicture`, middlewares, helpers.tryRoute(controllers.admin.uploads.uploadCategoryPicture)); router.post(`/api/${name}/uploadfavicon`, middlewares, helpers.tryRoute(controllers.admin.uploads.uploadFavicon)); diff --git a/src/routes/api.js b/src/routes/api.js index 0fe575a326..e374e242a4 100644 --- a/src/routes/api.js +++ b/src/routes/api.js @@ -23,11 +23,13 @@ module.exports = function (app, middleware, controllers) { router.get('/topic/teaser/:topic_id', [...middlewares], helpers.tryRoute(controllers.topics.teaser)); router.get('/topic/pagination/:topic_id', [...middlewares], helpers.tryRoute(controllers.topics.pagination)); - const multipart = require('connect-multiparty'); - const multipartMiddleware = multipart(); + const multer = require('multer'); + const storage = multer.diskStorage({}); + const upload = multer({ storage }); + const postMiddlewares = [ middleware.maintenanceMode, - multipartMiddleware, + upload.array('files[]', 20), middleware.validateFiles, middleware.uploads.ratelimit, middleware.applyCSRF, diff --git a/src/routes/authentication.js b/src/routes/authentication.js index 9d89df90e1..720675b29d 100644 --- a/src/routes/authentication.js +++ b/src/routes/authentication.js @@ -154,9 +154,15 @@ Auth.reloadRoutes = async function (params) { }); }); - const multipart = require('connect-multiparty'); - const multipartMiddleware = multipart(); - const middlewares = [multipartMiddleware, Auth.middleware.applyCSRF, Auth.middleware.applyBlacklist]; + + const multer = require('multer'); + const storage = multer.diskStorage({}); + const upload = multer({ storage }); + const middlewares = [ + upload.any(), + Auth.middleware.applyCSRF, + Auth.middleware.applyBlacklist, + ]; router.post('/register', middlewares, controllers.authentication.register); router.post('/register/complete', middlewares, controllers.authentication.registerComplete); diff --git a/src/routes/helpers.js b/src/routes/helpers.js index 34a455076e..2109a0bd9c 100644 --- a/src/routes/helpers.js +++ b/src/routes/helpers.js @@ -54,7 +54,9 @@ helpers.setupApiRoute = function (...args) { const [router, verb, name] = args; let middlewares = args.length > 4 ? args[args.length - 2] : []; const controller = args[args.length - 1]; - + const multer = require('multer'); + const storage = multer.diskStorage({}); + const upload = multer({ storage }); middlewares = [ middleware.autoLocale, middleware.applyBlacklist, @@ -63,7 +65,7 @@ helpers.setupApiRoute = function (...args) { middleware.registrationComplete, middleware.pluginHooks, middleware.logApiUsage, - middleware.handleMultipart, + upload.any(), ...middlewares, ]; diff --git a/src/routes/write/topics.js b/src/routes/write/topics.js index df10f66633..eb56cdaf42 100644 --- a/src/routes/write/topics.js +++ b/src/routes/write/topics.js @@ -10,9 +10,6 @@ const { setupApiRoute } = routeHelpers; module.exports = function () { const middlewares = [middleware.ensureLoggedIn]; - const multipart = require('connect-multiparty'); - const multipartMiddleware = multipart(); - setupApiRoute(router, 'post', '/', [middleware.checkRequired.bind(null, ['cid', 'title', 'content'])], controllers.write.topics.create); setupApiRoute(router, 'get', '/:tid', [], controllers.write.topics.get); setupApiRoute(router, 'post', '/:tid', [middleware.checkRequired.bind(null, ['content']), middleware.assert.topic], controllers.write.topics.reply); @@ -37,7 +34,13 @@ module.exports = function () { setupApiRoute(router, 'delete', '/:tid/tags', [...middlewares, middleware.assert.topic], controllers.write.topics.deleteTags); setupApiRoute(router, 'get', '/:tid/thumbs', [], controllers.write.topics.getThumbs); - setupApiRoute(router, 'post', '/:tid/thumbs', [multipartMiddleware, middleware.validateFiles, middleware.uploads.ratelimit, ...middlewares], controllers.write.topics.addThumb); + + setupApiRoute(router, 'post', '/:tid/thumbs', [ + middleware.validateFiles, + middleware.uploads.ratelimit, + ...middlewares, + ], controllers.write.topics.addThumb); + setupApiRoute(router, 'put', '/:tid/thumbs', [...middlewares, middleware.checkRequired.bind(null, ['tid'])], controllers.write.topics.migrateThumbs); setupApiRoute(router, 'delete', '/:tid/thumbs', [...middlewares, middleware.checkRequired.bind(null, ['path'])], controllers.write.topics.deleteThumb); setupApiRoute(router, 'put', '/:tid/thumbs/order', [...middlewares, middleware.checkRequired.bind(null, ['path', 'order'])], controllers.write.topics.reorderThumbs); diff --git a/test/helpers/index.js b/test/helpers/index.js index e71a05edaa..89de878ce8 100644 --- a/test/helpers/index.js +++ b/test/helpers/index.js @@ -95,7 +95,7 @@ helpers.uploadFile = async function (uploadEndPoint, filePath, data, jar, csrf_t const file = await fs.promises.readFile(filePath); const blob = new Blob([file], { type: mime.getType(filePath) }); - form.append('files', blob, path.basename(filePath)); + form.append('files[]', blob, path.basename(filePath)); if (data && data.params) { form.append('params', data.params);