Server: Upgraded Formidable parsing library

packages/server/package.json

@@ -33,7 +33,7 @@
     "bulma-prefers-dark": "0.1.0-beta.1",
     "compare-versions": "3.6.0",
     "dayjs": "1.11.9",
-    "formidable": "2.1.2",
+    "formidable": "3.5.1",
     "fs-extra": "11.1.1",
     "html-entities": "1.4.0",
     "jquery": "3.7.1",
@@ -60,7 +60,7 @@
   "devDependencies": {
     "@joplin/tools": "~2.13",
     "@rmp135/sql-ts": "1.18.0",
-    "@types/formidable": "2.0.6",
+    "@types/formidable": "3.4.3",
     "@types/fs-extra": "11.0.1",
     "@types/jest": "29.5.4",
     "@types/jest-expect-message": "1.0.4",

packages/server/src/utils/requestUtils.ts

@@ -1,10 +1,12 @@
 import { cookieGet } from './cookies';
 import { ErrorForbidden } from './errors';
 import { AppContext } from './types';
-import * as formidable from 'formidable';
+import * as formidableFn from 'formidable';
 import { Fields, Files } from 'formidable';
 import { IncomingMessage } from 'http';
 
+const formidable = require('formidable').default as typeof formidableFn;
+
 export type BodyFields = Record<string, any>;
 
 interface FormParseResult {
@@ -24,6 +26,28 @@ interface FormParseRequest extends IncomingMessage {
 	body: any;
 }
 
+// Previously Formidable would return the files and fields as key/value pairs.
+// With v3, the value however is always an array. This is unclear why they did
+// this but for example a field `email=test@example.com` would come out as
+// `email: ['test@example.com']`. Since all our code expect simple key/value
+// pairs, we use this function to convert back to the old style.
+//
+// For the extra challenge, they made this change only if the content-type is
+// "application/x-www-form-urlencoded". Other content types such as JSON are not
+// modified.
+const convertFieldsToKeyValue = (fields: Record<string, any[]>) => {
+	const convertedFields: Record<string, any> = {};
+	for (const [k, v] of Object.entries(fields)) {
+		if (Array.isArray(v)) {
+			convertedFields[k] = v.length ? v[0] : undefined;
+		} else {
+			convertedFields[k] = v;
+		}
+
+	}
+	return convertedFields;
+};
+
 // Input should be Koa ctx.req, which corresponds to the native Node request
 export async function formParse(request: IncomingMessage): Promise<FormParseResult> {
 	const req: FormParseRequest = request as any;
@@ -40,6 +64,8 @@ export async function formParse(request: IncomingMessage): Promise<FormParseResu
 
 	if (req.__parsed) return req.__parsed;
 
+	const isFormContentType = req.headers['content-type'] === 'application/x-www-form-urlencoded';
+
 	// Note that for Formidable to work, the content-type must be set in the
 	// headers
 	// eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied
@@ -56,7 +82,10 @@ export async function formParse(request: IncomingMessage): Promise<FormParseResu
 			// nothing, the code will just end there, or maybe wait
 			// indefinitely. So we cache the result on success and return it if
 			// some code somewhere tries again to parse the form.
-			req.__parsed = { fields, files };
+			req.__parsed = {
+				fields: isFormContentType ? convertFieldsToKeyValue(fields) : fields,
+				files: convertFieldsToKeyValue(files),
+			};
 			resolve(req.__parsed);
 		});
 	});

renovate.json5

@@ -113,10 +113,10 @@
 		"react-native",
 
 		// We currently don't have automated tests that verify that the
-		// sidemenus work correctly in the mobile app.
+		// sidemenus work correctly in the mobile app. Because an update to
-		// Because an update to react-native-reanimated has previously broken
+		// react-native-reanimated has previously broken the sidemenu
-		// the sidemenu (https://github.com/laurent22/joplin/issues/8456), we
+		// (https://github.com/laurent22/joplin/issues/8456), we disable auto
-		// disable auto updates for it:
+		// updates for it:
 		"react-native-reanimated",
 
 		// Need special processing when upgrading
@@ -129,17 +129,15 @@
 		"open",
 		"pretty-bytes",
 		"strip-ansi",
-		"formidable",
-		"@types/formidable",
 		"node-emoji",
 
 		// @koa/cors has undocumented breaking changes, and the package is not
 		// well supported so we're stuck with latest v3 for now
 		"@koa/cors",
 
-		// Can't upgrade beyond 2.x because it doesn't work with Electron. If trying to
+		// Can't upgrade beyond 2.x because it doesn't work with Electron. If
-		// upgrade again, check that adding a link from the CodeMirror editor works.
+		// trying to upgrade again, check that adding a link from the CodeMirror
-		// See /packages/app-desktop/gui/dialogs.ts
+		// editor works. See /packages/app-desktop/gui/dialogs.ts
 		"smalltalk",
 
 		// Don't think we can trust this lib because they don't release a

yarn.lock

@@ -5085,7 +5085,7 @@ __metadata:
     "@joplin/utils": ~2.13
     "@koa/cors": 3.4.3
     "@rmp135/sql-ts": 1.18.0
-    "@types/formidable": 2.0.6
+    "@types/formidable": 3.4.3
     "@types/fs-extra": 11.0.1
     "@types/jest": 29.5.4
     "@types/jest-expect-message": 1.0.4
@@ -5102,7 +5102,7 @@ __metadata:
     bulma-prefers-dark: 0.1.0-beta.1
     compare-versions: 3.6.0
     dayjs: 1.11.9
-    formidable: 2.1.2
+    formidable: 3.5.1
     fs-extra: 11.1.1
     gulp: 4.0.2
     html-entities: 1.4.0
@@ -7735,12 +7735,12 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@types/formidable@npm:2.0.6":
+"@types/formidable@npm:3.4.3":
-  version: 2.0.6
+  version: 3.4.3
-  resolution: "@types/formidable@npm:2.0.6"
+  resolution: "@types/formidable@npm:3.4.3"
   dependencies:
     "@types/node": "*"
-  checksum: d6be0ac12bf8dd2e4f8a022271ee6e501c7f6d7dd58d71c68497ca7da84bee1538d1a2a64a90b56dad557ddb291d48c5731206269e9ab53ed91264e68a4d1476
+  checksum: 6246a51e8b9c7f7f1bec85ccc8c6019f2fe8d46cecc0bf03a8bae68b6c0406bca97a67d71159a18b1f38db5a544c0dfd5c3272acf47877ca163e7e4cc1b414f4
   languageName: node
   linkType: hard
 
@@ -17620,15 +17620,14 @@ __metadata:
   languageName: node
   linkType: hard
 
-"formidable@npm:2.1.2":
+"formidable@npm:3.5.1":
-  version: 2.1.2
+  version: 3.5.1
-  resolution: "formidable@npm:2.1.2"
+  resolution: "formidable@npm:3.5.1"
   dependencies:
     dezalgo: ^1.0.4
     hexoid: ^1.0.0
     once: ^1.4.0
-    qs: ^6.11.0
+  checksum: 46b21496f9f985161cf7636163147b6728f9997c7e1d59433680d92619758bf6862330e6d105b5816bafcd1ab32f27ef183455991f93ef836ea731c68db62af9
-  checksum: 81c8e5d89f5eb873e992893468f0de22c01678ca3d315db62be0560f9de1c77d4faefc9b1f4575098eb2263b3c81ba1024833a9fc3206297ddbac88a4f69b7a8
   languageName: node
   linkType: hard