images: update form parsing

This is addressing a critical error, see GHSA-8cp3-66vr-3r4c

That was not trivial as I bumped into this bug node-formidable/formidable#959
which led me to move form parsing to the middleware stage

package.json

@@ -73,7 +73,7 @@
  "express": "^4.16.3",
  "express-oauth-server": "^2.0.0",
  "fold-to-ascii": "^5.0.0",
- "formidable": "^1.0.17",
+ "formidable": "^3.5.1",
  "gm": "^1.23.1",
  "inventaire-i18n": "github:inventaire/inventaire-i18n",
  "isbn-groups": "^2.0.1",

server/controllers/images/lib/containers.ts

@@ -2,6 +2,7 @@ import localClient from '#controllers/images/lib/local_client'
 import swiftClient from '#controllers/images/lib/swift_client'
 import { getHashFilename, removeExif, shrinkAndFormat } from '#lib/images'
 import { emit } from '#lib/radio'
+import { assert_ } from '#lib/utils/assert_types'
 import { log, info } from '#lib/utils/logs'
 import config from '#server/config'
 
@@ -20,7 +21,9 @@ if (mode === 'swift') {
 const { putImage, deleteImage } = client
 
 const transformAndPutImage = (container, fn) => async fileData => {
- const { id = 0, path } = fileData
+ const { id = 0 } = fileData
+ const path = fileData.path || fileData.filepath
+ assert_.string(path)
  await fn(path)
  const filename = await getHashFilename(path)
  const url = await putImage(container, path, filename)

server/controllers/images/lib/parse_form.ts

@@ -1,15 +1,28 @@
 import { tmpdir } from 'node:os'
-import formidable from 'formidable'
+import formidable, { type Fields, type Files } from 'formidable'
 import { mkdirp } from '#lib/fs'
 
-const { IncomingForm } = formidable
 const uploadDir = `${tmpdir()}/formidable`
 await mkdirp(uploadDir)
 
-export default req => new Promise((resolve, reject) => {
- const form = new IncomingForm({ uploadDir })
- return form.parse(req, (err, fields, files) => {
- if (err) reject(err)
- else resolve({ fields, files })
- })
-})
+export interface ParsedForm {
+ fields: Fields
+ files: Files
+}
+
+// Parse forms in an early middleware to not let the time to any other middleware
+// to start consuming the form request stream, to avoid getting hanging requests
+// See https://github.com/node-formidable/formidable/issues/959
+export async function parseFormMiddleware (req, res, next) {
+ if (!req.headers['content-type'].startsWith('multipart/form-data')) return next()
+
+ try {
+ const form = formidable({ uploadDir })
+ const [ fields, files ] = await form.parse(req)
+ const reqForm: ParsedForm = { fields, files }
+ req.form = reqForm
+ next()
+ } catch (err) {
+ next(err)
+ }
+}

server/controllers/images/upload.ts

@@ -1,8 +1,9 @@
 import { isNonEmptyPlainObject } from '#lib/boolean_validations'
 import { newError } from '#lib/error/error'
+import { assert_ } from '#lib/utils/assert_types'
 import { Log } from '#lib/utils/logs'
+import type { FormReq } from '#server/types/server'
 import { containers, uploadContainersNames } from './lib/containers.js'
-import parseForm from './lib/parse_form.js'
 
 const sanitization = {
  nonJsonBody: true,
@@ -12,12 +13,12 @@ const sanitization = {
  },
 }
 
-async function controller (params, req) {
+async function controller (params, req: FormReq) {
  const { container } = params
 
  const { putImage } = containers[container]
 
- const files = await parseForm(req).then(getFilesFromFormData)
+ const files = getFilesFromFormData(req.form)
  if (container === 'users') files.forEach(validateFile)
 
  return Promise.all(files.map(putImage))
@@ -32,12 +33,15 @@ function getFilesFromFormData (formData) {
  throw newError('no file provided', 400, formData)
  }
 
- for (const key in files) {
- const file = files[key]
+ return Object.entries(files).map(([ key, fileArray ]) => {
+ assert_.array(fileArray)
+ const file = fileArray[0]
+ assert_.string(file.filepath)
  file.id = key
- }
-
- return Object.values(files)
+ // This somehow does not have any effect: the "path" attribute is undefined when we reach transformAndPutImage
+ file.path = file.pathname
+ return file
+ })
 }
 
 function validateFile (file) {

server/lib/images.ts

@@ -1,6 +1,7 @@
 import { readFile } from 'node:fs/promises'
 import gm from 'gm'
 import { sha1 } from '#lib/crypto'
+import { assert_ } from '#lib/utils/assert_types'
 import config from '#server/config'
 
 const { maxSize } = config.mediaStorage.images
@@ -20,16 +21,19 @@ export function shrinkAndFormatStream (data, width, height) {
 }
 
 export function getHashFilename (path) {
+ assert_.string(path)
  return readFile(path)
  .then(sha1)
 }
 
 export function shrinkAndFormat (path, width = maxSize, height = maxSize) {
+ assert_.string(path)
  return new Promise((resolve, reject) => shrinkAndFormatStream(path, width, height)
  .write(path, returnPath(path, resolve, reject)))
 }
 
-export function removeExif (path) {
+export function removeExif (path: string) {
+ assert_.string(path)
  return new Promise((resolve, reject) => {
  gm(path)
  .noProfile()

server/middlewares/middlewares.ts

@@ -1,3 +1,4 @@
+import { parseFormMiddleware } from '#controllers/images/lib/parse_form'
 import auth from './auth.js'
 import { acceptUrlencoded, jsonBodyParser, deduplicateRequests } from './content.js'
 import requestsLogger from './requests_logger.js'
@@ -11,6 +12,8 @@ export default [
 
  security.setCorsPolicy,
 
+ [ '/api/images', parseFormMiddleware ],
+
  // server/controllers/auth/fake_submit.ts relies on the possibility
  // to submit a url encoded form data, so it needs to have the body-parser ready for it
  acceptUrlencoded('/api/submit'),

server/types/server.ts

@@ -1,3 +1,4 @@
+import type { ParsedForm } from '#controllers/images/lib/parse_form'
 import type { User, UserId } from '#types/user'
 import type Express from 'express'
 
@@ -12,3 +13,7 @@ export type Res = Express.Response
 export type Next = () => void
 
 export type Sanitized<Params> = Params & { reqUserId?: UserId }
+
+export interface FormReq extends AuthentifiedReq {
+ form: ParsedForm
+}