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

@@ -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",

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);

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);

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']);

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);
-};

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]]']));
 	}

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));

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,

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);

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,
 	];
 

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);

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);