From cd4fbc851b0fc766c93552971cb916e4ccd1ef55 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Tue, 6 May 2025 12:55:51 -0400 Subject: improve compatibility with multipart/form-data mastodon API requests --- .../backend/src/server/ServerUtilityService.ts | 141 +++++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 packages/backend/src/server/ServerUtilityService.ts (limited to 'packages/backend/src/server/ServerUtilityService.ts') diff --git a/packages/backend/src/server/ServerUtilityService.ts b/packages/backend/src/server/ServerUtilityService.ts new file mode 100644 index 0000000000..f2900fad4f --- /dev/null +++ b/packages/backend/src/server/ServerUtilityService.ts @@ -0,0 +1,141 @@ +/* + * SPDX-FileCopyrightText: hazelnoot and other Sharkey contributors + * SPDX-License-Identifier: AGPL-3.0-only + */ + +import querystring from 'querystring'; +import multipart from '@fastify/multipart'; +import { Inject, Injectable } from '@nestjs/common'; +import { FastifyInstance } from 'fastify'; +import { DI } from '@/di-symbols.js'; +import type { Config } from '@/config.js'; + +@Injectable() +export class ServerUtilityService { + constructor( + @Inject(DI.config) + private readonly config: Config, + ) {} + + public addMultipartFormDataContentType(fastify: FastifyInstance): void { + fastify.register(multipart, { + limits: { + fileSize: this.config.maxFileSize, + files: 1, + }, + }); + + // Default behavior saves files to memory - we don't want that! + // Store to temporary file instead, and copy the body fields while we're at it. + fastify.addHook<{ Body?: Record }>('onRequest', async request => { + if (request.isMultipart()) { + const body = request.body ??= {}; + + // Save upload to temp directory. + // These are attached to request.savedRequestFiles + await request.saveRequestFiles(); + + // Copy fields to body + const formData = await request.formData(); + formData.forEach((v, k) => { + // This can be string or File, and we handle files above. + if (typeof(v) === 'string') { + // This is just progressive conversion from undefined -> string -> string[] + if (body[k]) { + if (Array.isArray(body[k])) { + body[k].push(v); + } else { + body[k] = [body[k], v]; + } + } else { + body[k] = v; + } + } + }); + } + }); + } + + public addFormUrlEncodedContentType(fastify: FastifyInstance) { + fastify.addContentTypeParser('application/x-www-form-urlencoded', (_, payload, done) => { + let body = ''; + payload.on('data', (data) => { + body += data; + }); + payload.on('end', () => { + try { + const parsed = querystring.parse(body); + done(null, parsed); + } catch (e) { + done(e as Error); + } + }); + payload.on('error', done); + }); + } + + public addCORS(fastify: FastifyInstance) { + fastify.addHook('onRequest', (_, reply, done) => { + // Allow web-based clients to connect from other origins. + reply.header('Access-Control-Allow-Origin', '*'); + + // Mastodon uses all types of request methods. + reply.header('Access-Control-Allow-Methods', '*'); + + // Allow web-based clients to access Link header - required for mastodon pagination. + // https://stackoverflow.com/a/54928828 + // https://docs.joinmastodon.org/api/guidelines/#pagination + // https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Access-Control-Expose-Headers + reply.header('Access-Control-Expose-Headers', 'Link'); + + // Cache to avoid extra pre-flight requests + // https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Access-Control-Max-Age + reply.header('Access-Control-Max-Age', 60 * 60 * 24); // 1 day in seconds + + done(); + }); + } + + public addFlattenedQueryType(fastify: FastifyInstance) { + // Remove trailing "[]" from query params + fastify.addHook<{ Querystring?: Record }>('preValidation', (request, _reply, done) => { + if (!request.query || typeof(request.query) !== 'object') { + return done(); + } + + for (const key of Object.keys(request.query)) { + if (!key.endsWith('[]')) { + continue; + } + if (request.query[key] == null) { + continue; + } + + const newKey = key.substring(0, key.length - 2); + const newValue = request.query[key]; + const oldValue = request.query[newKey]; + + // Move the value to the correct key + if (oldValue != null) { + if (Array.isArray(oldValue)) { + // Works for both array and single values + request.query[newKey] = oldValue.concat(newValue); + } else if (Array.isArray(newValue)) { + // Preserve order + request.query[newKey] = [oldValue, ...newValue]; + } else { + // Preserve order + request.query[newKey] = [oldValue, newValue]; + } + } else { + request.query[newKey] = newValue; + } + + // Remove the invalid key + delete request.query[key]; + } + + return done(); + }); + } +} -- cgit v1.2.3-freya From 89cab66898f4edb238eee2321564337034dfa2bc Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Tue, 6 May 2025 18:26:33 -0400 Subject: fix multipart/form-data decoding --- packages/backend/src/misc/create-temp.ts | 13 ++++++++ .../backend/src/server/ServerUtilityService.ts | 39 ++++++++++++++++------ 2 files changed, 42 insertions(+), 10 deletions(-) (limited to 'packages/backend/src/server/ServerUtilityService.ts') diff --git a/packages/backend/src/misc/create-temp.ts b/packages/backend/src/misc/create-temp.ts index 6cc896046f..fda63c7a9d 100644 --- a/packages/backend/src/misc/create-temp.ts +++ b/packages/backend/src/misc/create-temp.ts @@ -3,6 +3,8 @@ * SPDX-License-Identifier: AGPL-3.0-only */ +import { pipeline } from 'node:stream/promises'; +import fs from 'node:fs'; import * as tmp from 'tmp'; export function createTemp(): Promise<[string, () => void]> { @@ -27,3 +29,14 @@ export function createTempDir(): Promise<[string, () => void]> { ); }); } + +export async function saveToTempFile(stream: NodeJS.ReadableStream): Promise { + const [filepath, cleanup] = await createTemp(); + try { + await pipeline(stream, fs.createWriteStream(filepath)); + return filepath; + } catch (e) { + cleanup(); + throw e; + } +} diff --git a/packages/backend/src/server/ServerUtilityService.ts b/packages/backend/src/server/ServerUtilityService.ts index f2900fad4f..d90b37ca50 100644 --- a/packages/backend/src/server/ServerUtilityService.ts +++ b/packages/backend/src/server/ServerUtilityService.ts @@ -9,6 +9,7 @@ import { Inject, Injectable } from '@nestjs/common'; import { FastifyInstance } from 'fastify'; import { DI } from '@/di-symbols.js'; import type { Config } from '@/config.js'; +import { saveToTempFile } from '@/misc/create-temp.js'; @Injectable() export class ServerUtilityService { @@ -29,17 +30,16 @@ export class ServerUtilityService { // Store to temporary file instead, and copy the body fields while we're at it. fastify.addHook<{ Body?: Record }>('onRequest', async request => { if (request.isMultipart()) { - const body = request.body ??= {}; + // We can't use saveRequestFiles() because it erases all the data fields. + // Instead, recreate it manually. + // https://github.com/fastify/fastify-multipart/issues/549 - // Save upload to temp directory. - // These are attached to request.savedRequestFiles - await request.saveRequestFiles(); + for await (const part of request.parts()) { + if (part.type === 'field') { + const k = part.fieldname; + const v = String(part.value); + const body = request.body ??= {}; - // Copy fields to body - const formData = await request.formData(); - formData.forEach((v, k) => { - // This can be string or File, and we handle files above. - if (typeof(v) === 'string') { // This is just progressive conversion from undefined -> string -> string[] if (body[k]) { if (Array.isArray(body[k])) { @@ -50,8 +50,27 @@ export class ServerUtilityService { } else { body[k] = v; } + } else { // Otherwise it's a file + try { + const [filepath] = await saveToTempFile(part.file); + + const tmpUploads = (request.tmpUploads ??= []); + tmpUploads.push(filepath); + + const requestSavedFiles = (request.savedRequestFiles ??= []); + requestSavedFiles.push({ + ...part, + filepath, + }); + } catch (e) { + // Cleanup to avoid file leak in case of errors + await request.cleanRequestFiles(); + request.tmpUploads = null; + request.savedRequestFiles = null; + throw e; + } } - }); + } } }); } -- cgit v1.2.3-freya From cbe88122b9c1446a61d69082df8f0ba729837a08 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Tue, 6 May 2025 18:46:08 -0400 Subject: fix hook targets --- packages/backend/src/server/ServerUtilityService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'packages/backend/src/server/ServerUtilityService.ts') diff --git a/packages/backend/src/server/ServerUtilityService.ts b/packages/backend/src/server/ServerUtilityService.ts index d90b37ca50..115717534f 100644 --- a/packages/backend/src/server/ServerUtilityService.ts +++ b/packages/backend/src/server/ServerUtilityService.ts @@ -28,7 +28,7 @@ export class ServerUtilityService { // Default behavior saves files to memory - we don't want that! // Store to temporary file instead, and copy the body fields while we're at it. - fastify.addHook<{ Body?: Record }>('onRequest', async request => { + fastify.addHook<{ Body?: Record }>('preValidation', async request => { if (request.isMultipart()) { // We can't use saveRequestFiles() because it erases all the data fields. // Instead, recreate it manually. @@ -94,7 +94,7 @@ export class ServerUtilityService { } public addCORS(fastify: FastifyInstance) { - fastify.addHook('onRequest', (_, reply, done) => { + fastify.addHook('preHandler', (_, reply, done) => { // Allow web-based clients to connect from other origins. reply.header('Access-Control-Allow-Origin', '*'); -- cgit v1.2.3-freya From 6757c227a9509f9b92a9e7a0e9f606a89f3e9727 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Tue, 6 May 2025 18:48:22 -0400 Subject: check type of field values --- packages/backend/src/server/ServerUtilityService.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'packages/backend/src/server/ServerUtilityService.ts') diff --git a/packages/backend/src/server/ServerUtilityService.ts b/packages/backend/src/server/ServerUtilityService.ts index 115717534f..8cecc5df58 100644 --- a/packages/backend/src/server/ServerUtilityService.ts +++ b/packages/backend/src/server/ServerUtilityService.ts @@ -37,9 +37,13 @@ export class ServerUtilityService { for await (const part of request.parts()) { if (part.type === 'field') { const k = part.fieldname; - const v = String(part.value); + const v = part.value; const body = request.body ??= {}; + // Value can be string, buffer, or undefined. + // We only support the first one. + if (typeof(v) !== 'string') continue; + // This is just progressive conversion from undefined -> string -> string[] if (body[k]) { if (Array.isArray(body[k])) { -- cgit v1.2.3-freya From f7ca7a2cc04bd686ede95ac39524bc6f3fefe5ea Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Tue, 6 May 2025 20:24:14 -0400 Subject: minor refactor to ServerUtilityService.addMultipartFormDataContentType --- packages/backend/src/server/ServerUtilityService.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'packages/backend/src/server/ServerUtilityService.ts') diff --git a/packages/backend/src/server/ServerUtilityService.ts b/packages/backend/src/server/ServerUtilityService.ts index 8cecc5df58..00eb97f679 100644 --- a/packages/backend/src/server/ServerUtilityService.ts +++ b/packages/backend/src/server/ServerUtilityService.ts @@ -45,14 +45,12 @@ export class ServerUtilityService { if (typeof(v) !== 'string') continue; // This is just progressive conversion from undefined -> string -> string[] - if (body[k]) { - if (Array.isArray(body[k])) { - body[k].push(v); - } else { - body[k] = [body[k], v]; - } - } else { + if (!body[k]) { body[k] = v; + } else if (Array.isArray(body[k])) { + body[k].push(v); + } else { + body[k] = [body[k], v]; } } else { // Otherwise it's a file try { -- cgit v1.2.3-freya From 22bba7fe6d787b1341340e331f3b0a706b00a383 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Tue, 6 May 2025 21:52:19 -0400 Subject: fix media upload error caused by extraneous array brackets --- packages/backend/src/server/ServerUtilityService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/backend/src/server/ServerUtilityService.ts') diff --git a/packages/backend/src/server/ServerUtilityService.ts b/packages/backend/src/server/ServerUtilityService.ts index 00eb97f679..c2a3132489 100644 --- a/packages/backend/src/server/ServerUtilityService.ts +++ b/packages/backend/src/server/ServerUtilityService.ts @@ -54,7 +54,7 @@ export class ServerUtilityService { } } else { // Otherwise it's a file try { - const [filepath] = await saveToTempFile(part.file); + const filepath = await saveToTempFile(part.file); const tmpUploads = (request.tmpUploads ??= []); tmpUploads.push(filepath); -- cgit v1.2.3-freya From e75e4f11a20e989038f2a5ab8c3cd2a1c3b44e40 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Thu, 8 May 2025 16:42:16 -0400 Subject: match saveToTempFile return type with other create-temp function --- packages/backend/src/misc/create-temp.ts | 4 ++-- packages/backend/src/server/ServerUtilityService.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'packages/backend/src/server/ServerUtilityService.ts') diff --git a/packages/backend/src/misc/create-temp.ts b/packages/backend/src/misc/create-temp.ts index fda63c7a9d..f2138abf66 100644 --- a/packages/backend/src/misc/create-temp.ts +++ b/packages/backend/src/misc/create-temp.ts @@ -30,11 +30,11 @@ export function createTempDir(): Promise<[string, () => void]> { }); } -export async function saveToTempFile(stream: NodeJS.ReadableStream): Promise { +export async function saveToTempFile(stream: NodeJS.ReadableStream): Promise<[string, () => void]> { const [filepath, cleanup] = await createTemp(); try { await pipeline(stream, fs.createWriteStream(filepath)); - return filepath; + return [filepath, cleanup]; } catch (e) { cleanup(); throw e; diff --git a/packages/backend/src/server/ServerUtilityService.ts b/packages/backend/src/server/ServerUtilityService.ts index c2a3132489..00eb97f679 100644 --- a/packages/backend/src/server/ServerUtilityService.ts +++ b/packages/backend/src/server/ServerUtilityService.ts @@ -54,7 +54,7 @@ export class ServerUtilityService { } } else { // Otherwise it's a file try { - const filepath = await saveToTempFile(part.file); + const [filepath] = await saveToTempFile(part.file); const tmpUploads = (request.tmpUploads ??= []); tmpUploads.push(filepath); -- cgit v1.2.3-freya