enhance(server): downloadUrlでContent-Dispositionからファイル名を取得 (#10150)

* enhance(server): downloadUrlでContent-Dispositionからファイル名を取得
Resolve #10036
Resolve #4750

* untitled

* オブジェクトストレージのContent-Dispositionのファイル名の拡張子をContent-Typeに添ったものにする

* ✌️

* tiff

* fix filename

* add test

* /files/でもContent-Disposition

* comment

* fix test

packages/backend/src/core/DownloadService.ts

@@ -6,6 +6,7 @@ import IPCIDR from 'ip-cidr';
 import PrivateIp from 'private-ip';
 import chalk from 'chalk';
 import got, * as Got from 'got';
+import { parse } from 'content-disposition';
 import { DI } from '@/di-symbols.js';
 import type { Config } from '@/config.js';
 import { HttpRequestService } from '@/core/HttpRequestService.js';
@@ -32,13 +33,18 @@ export class DownloadService {
 	}
 
 	@bindThis
-	public async downloadUrl(url: string, path: string): Promise<void> {
+	public async downloadUrl(url: string, path: string): Promise<{
+		filename: string;
+	}> {
 		this.logger.info(`Downloading ${chalk.cyan(url)} to ${chalk.cyanBright(path)} ...`);
 
 		const timeout = 30 * 1000;
 		const operationTimeout = 60 * 1000;
 		const maxSize = this.config.maxFileSize ?? 262144000;
 
+		const urlObj = new URL(url);
+		let filename = urlObj.pathname.split('/').pop() ?? 'untitled';
+
 		const req = got.stream(url, {
 			headers: {
 				'User-Agent': this.config.userAgent,
@@ -77,6 +83,14 @@ export class DownloadService {
 					req.destroy();
 				}
 			}
+
+			const contentDisposition = res.headers['content-disposition'];
+			if (contentDisposition != null) {
+				const parsed = parse(contentDisposition);
+				if (parsed.parameters.filename) {
+					filename = parsed.parameters.filename;
+				}
+			}
 		}).on('downloadProgress', (progress: Got.Progress) => {
 			if (progress.transferred > maxSize) {
 				this.logger.warn(`maxSize exceeded (${progress.transferred} > ${maxSize}) on downloadProgress`);
@@ -95,6 +109,10 @@ export class DownloadService {
 		}
 
 		this.logger.succ(`Download finished: ${chalk.cyan(url)}`);
+
+		return {
+			filename,
+		};
 	}
 
 	@bindThis

packages/backend/src/core/DriveService.ts

@@ -34,6 +34,7 @@ import { FileInfoService } from '@/core/FileInfoService.js';
 import { bindThis } from '@/decorators.js';
 import { RoleService } from '@/core/RoleService.js';
 import type S3 from 'aws-sdk/clients/s3.js';
+import { correctFilename } from '@/misc/correct-filename.js';
 
 type AddFileArgs = {
 	/** User who wish to add file */
@@ -168,7 +169,7 @@ export class DriveService {
 			//#region Uploads
 			this.registerLogger.info(`uploading original: ${key}`);
 			const uploads = [
-				this.upload(key, fs.createReadStream(path), type, name),
+				this.upload(key, fs.createReadStream(path), type, ext, name),
 			];
 
 			if (alts.webpublic) {
@@ -176,7 +177,7 @@ export class DriveService {
 				webpublicUrl = `${ baseUrl }/${ webpublicKey }`;
 
 				this.registerLogger.info(`uploading webpublic: ${webpublicKey}`);
-				uploads.push(this.upload(webpublicKey, alts.webpublic.data, alts.webpublic.type, name));
+				uploads.push(this.upload(webpublicKey, alts.webpublic.data, alts.webpublic.type, alts.webpublic.ext, name));
 			}
 
 			if (alts.thumbnail) {
@@ -184,7 +185,7 @@ export class DriveService {
 				thumbnailUrl = `${ baseUrl }/${ thumbnailKey }`;
 
 				this.registerLogger.info(`uploading thumbnail: ${thumbnailKey}`);
-				uploads.push(this.upload(thumbnailKey, alts.thumbnail.data, alts.thumbnail.type));
+				uploads.push(this.upload(thumbnailKey, alts.thumbnail.data, alts.thumbnail.type, alts.thumbnail.ext));
 			}
 
 			await Promise.all(uploads);
@@ -360,7 +361,7 @@ export class DriveService {
 	 * Upload to ObjectStorage
 	 */
 	@bindThis
-	private async upload(key: string, stream: fs.ReadStream | Buffer, type: string, filename?: string) {
+	private async upload(key: string, stream: fs.ReadStream | Buffer, type: string, ext?: string | null, filename?: string) {
 		if (type === 'image/apng') type = 'image/png';
 		if (!FILE_TYPE_BROWSERSAFE.includes(type)) type = 'application/octet-stream';
 
@@ -374,7 +375,12 @@ export class DriveService {
 			CacheControl: 'max-age=31536000, immutable',
 		} as S3.PutObjectRequest;
 
-		if (filename) params.ContentDisposition = contentDisposition('inline', filename);
+		if (filename) params.ContentDisposition = contentDisposition(
+			'inline',
+			// 拡張子からContent-Typeを設定してそうな挙動を示すオブジェクトストレージ (upcloud?) も存在するので、
+			// 許可されているファイル形式でしか拡張子をつけない
+			ext ? correctFilename(filename, ext) : filename,
+		);
 		if (meta.objectStorageSetPublicRead) params.ACL = 'public-read';
 
 		const s3 = this.s3Service.getS3(meta);
@@ -466,7 +472,12 @@ export class DriveService {
 		//}
 
 		// detect name
-		const detectedName = name ?? (info.type.ext ? `untitled.${info.type.ext}` : 'untitled');
+		const detectedName = correctFilename(
+			// DriveFile.nameは256文字, validateFileNameは200文字制限であるため、
+			// extを付加してデータベースの文字数制限に当たることはまずない
+			(name && this.driveFileEntityService.validateFileName(name)) ? name : 'untitled',
+			info.type.ext
+		);
 
 		if (user && !force) {
 		// Check if there is a file with the same hash
@@ -736,10 +747,12 @@ export class DriveService {
 		requestIp = null,
 		requestHeaders = null,
 	}: UploadFromUrlArgs): Promise<DriveFile> {
-		let name = new URL(url).pathname.split('/').pop() ?? null;
+		// Create temp file
-		if (name == null || !this.driveFileEntityService.validateFileName(name)) {
+		const [path, cleanup] = await createTemp();
-			name = null;
+	
-		}
+		try {
+			// write content at URL to temp file
+			const { filename: name } = await this.downloadService.downloadUrl(url, path);
 
 			// If the comment is same as the name, skip comment
 			// (image.name is passed in when receiving attachment)
@@ -747,13 +760,6 @@ export class DriveService {
 				comment = null;
 			}
 
-		// Create temp file
-		const [path, cleanup] = await createTemp();
-	
-		try {
-			// write content at URL to temp file
-			await this.downloadService.downloadUrl(url, path);
-	
 			const driveFile = await this.addFile({ user, path, name, comment, folderId, force, isLink, url, uri, sensitive, requestIp, requestHeaders });
 			this.downloaderLogger.succ(`Got: ${driveFile.id}`);
 			return driveFile!;

packages/backend/src/misc/correct-filename.ts

@@ -0,0 +1,15 @@
+// 与えられた拡張子とファイル名が一致しているかどうかを確認し、
+// 一致していない場合は拡張子を付与して返す
+export function correctFilename(filename: string, ext: string | null) {
+    const dotExt = ext ? `.${ext}` : '.unknown';
+    if (filename.endsWith(dotExt)) {
+        return filename;
+    }
+    if (ext === 'jpg' && filename.endsWith('.jpeg')) {
+        return filename;
+    }
+    if (ext === 'tif' && filename.endsWith('.tiff')) {
+        return filename;
+    }
+    return `${filename}${dotExt}`;
+}

packages/backend/src/server/FileServerService.ts

@@ -22,6 +22,7 @@ import { bindThis } from '@/decorators.js';
 import type { FastifyInstance, FastifyRequest, FastifyReply, FastifyPluginOptions } from 'fastify';
 import { isMimeImage } from '@/misc/is-mime-image.js';
 import sharp from 'sharp';
+import { correctFilename } from '@/misc/correct-filename.js';
 
 const _filename = fileURLToPath(import.meta.url);
 const _dirname = dirname(_filename);
@@ -51,15 +52,6 @@ export class FileServerService {
 		//this.createServer = this.createServer.bind(this);
 	}
 
-	@bindThis
-	public commonReadableHandlerGenerator(reply: FastifyReply) {
-		return (err: Error): void => {
-			this.logger.error(err);
-			reply.code(500);
-			reply.header('Cache-Control', 'max-age=300');
-		};
-	}
-
 	@bindThis
 	public createServer(fastify: FastifyInstance, options: FastifyPluginOptions, done: (err?: Error) => void) {
 		fastify.addHook('onRequest', (request, reply, done) => {
@@ -190,13 +182,19 @@ export class FileServerService {
 				}
 
 				reply.header('Content-Type', FILE_TYPE_BROWSERSAFE.includes(image.type) ? image.type : 'application/octet-stream');
+				reply.header('Content-Disposition',
+					contentDisposition(
+						'inline',
+						correctFilename(file.filename, image.ext)
+					)
+				);
 				return image.data;
 			}
 
 			if (file.fileRole !== 'original') {
-				const filename = rename(file.file.name, {
+				const filename = rename(file.filename, {
 					suffix: file.fileRole === 'thumbnail' ? '-thumb' : '-web',
-					extname: file.ext ? `.${file.ext}` : undefined,
+					extname: file.ext ? `.${file.ext}` : '.unknown',
 				}).toString();
 
 				reply.header('Content-Type', FILE_TYPE_BROWSERSAFE.includes(file.mime) ? file.mime : 'application/octet-stream');
@@ -204,12 +202,10 @@ export class FileServerService {
 				reply.header('Content-Disposition', contentDisposition('inline', filename));
 				return fs.createReadStream(file.path);
 			} else {
-				const stream = fs.createReadStream(file.path);
-				stream.on('error', this.commonReadableHandlerGenerator(reply));
 				reply.header('Content-Type', FILE_TYPE_BROWSERSAFE.includes(file.file.type) ? file.file.type : 'application/octet-stream');
 				reply.header('Cache-Control', 'max-age=31536000, immutable');
-				reply.header('Content-Disposition', contentDisposition('inline', file.file.name));
+				reply.header('Content-Disposition', contentDisposition('inline', file.filename));
-				return stream;
+				return fs.createReadStream(file.path);
 			}
 		} catch (e) {
 			if ('cleanup' in file) file.cleanup();
@@ -360,6 +356,12 @@ export class FileServerService {
 
 			reply.header('Content-Type', image.type);
 			reply.header('Cache-Control', 'max-age=31536000, immutable');
+			reply.header('Content-Disposition',
+				contentDisposition(
+					'inline',
+					correctFilename(file.filename, image.ext)
+				)
+			);
 			return image.data;
 		} catch (e) {
 			if ('cleanup' in file) file.cleanup();
@@ -369,8 +371,8 @@ export class FileServerService {
 
 	@bindThis
 	private async getStreamAndTypeFromUrl(url: string): Promise<
-		{ state: 'remote'; fileRole?: 'thumbnail' | 'webpublic' | 'original'; file?: DriveFile; mime: string; ext: string | null; path: string; cleanup: () => void; }
+		{ state: 'remote'; fileRole?: 'thumbnail' | 'webpublic' | 'original'; file?: DriveFile; mime: string; ext: string | null; path: string; cleanup: () => void; filename: string; }
-		| { state: 'stored_internal'; fileRole: 'thumbnail' | 'webpublic' | 'original'; file: DriveFile; mime: string; ext: string | null; path: string; }
+		| { state: 'stored_internal'; fileRole: 'thumbnail' | 'webpublic' | 'original'; file: DriveFile; filename: string; mime: string; ext: string | null; path: string; }
 		| '404'
 		| '204'
 	> {
@@ -386,11 +388,11 @@ export class FileServerService {
 
 	@bindThis
 	private async downloadAndDetectTypeFromUrl(url: string): Promise<
-		{ state: 'remote' ; mime: string; ext: string | null; path: string; cleanup: () => void; }
+		{ state: 'remote' ; mime: string; ext: string | null; path: string; cleanup: () => void; filename: string; }
 	> {
 		const [path, cleanup] = await createTemp();
 		try {
-			await this.downloadService.downloadUrl(url, path);
+			const { filename } = await this.downloadService.downloadUrl(url, path);
 
 			const { mime, ext } = await this.fileInfoService.detectType(path);
 	
@@ -398,6 +400,7 @@ export class FileServerService {
 				state: 'remote',
 				mime, ext,
 				path, cleanup,
+				filename,
 			};
 		} catch (e) {
 			cleanup();
@@ -407,8 +410,8 @@ export class FileServerService {
 
 	@bindThis
 	private async getFileFromKey(key: string): Promise<
-		{ state: 'remote'; fileRole: 'thumbnail' | 'webpublic' | 'original'; file: DriveFile; url: string; mime: string; ext: string | null; path: string; cleanup: () => void; }
+		{ state: 'remote'; fileRole: 'thumbnail' | 'webpublic' | 'original'; file: DriveFile; filename: string; url: string; mime: string; ext: string | null; path: string; cleanup: () => void; }
-		| { state: 'stored_internal'; fileRole: 'thumbnail' | 'webpublic' | 'original'; file: DriveFile; mime: string; ext: string | null; path: string; }
+		| { state: 'stored_internal'; fileRole: 'thumbnail' | 'webpublic' | 'original'; file: DriveFile; filename: string; mime: string; ext: string | null; path: string; }
 		| '404'
 		| '204'
 	> {
@@ -432,6 +435,7 @@ export class FileServerService {
 				url: file.uri,
 				fileRole: isThumbnail ? 'thumbnail' : isWebpublic ? 'webpublic' : 'original',
 				file,
+				filename: file.name,
 			};
 		}
 
@@ -443,6 +447,7 @@ export class FileServerService {
 				state: 'stored_internal',
 				fileRole: isThumbnail ? 'thumbnail' : 'webpublic',
 				file,
+				filename: file.name,
 				mime, ext,
 				path,
 			};
@@ -452,6 +457,7 @@ export class FileServerService {
 			state: 'stored_internal',
 			fileRole: 'original',
 			file,
+			filename: file.name,
 			mime: file.type,
 			ext: null,
 			path,

packages/backend/test/e2e/endpoints.ts

@@ -410,11 +410,19 @@ describe('Endpoints', () => {
 		});
 
 		test('ファイルに名前を付けられる', async () => {
+			const res = await uploadFile(alice, { name: 'Belmond.jpg' });
+
+			assert.strictEqual(res.status, 200);
+			assert.strictEqual(typeof res.body === 'object' && !Array.isArray(res.body), true);
+			assert.strictEqual(res.body.name, 'Belmond.jpg');
+		});
+
+		test('ファイルに名前を付けられるが、拡張子は正しいものになる', async () => {
 			const res = await uploadFile(alice, { name: 'Belmond.png' });
 
 			assert.strictEqual(res.status, 200);
 			assert.strictEqual(typeof res.body === 'object' && !Array.isArray(res.body), true);
-			assert.strictEqual(res.body.name, 'Belmond.png');
+			assert.strictEqual(res.body.name, 'Belmond.png.jpg');
 		});
 
 		test('ファイル無しで怒られる', async () => {

packages/backend/test/unit/misc/others.ts

@@ -0,0 +1,39 @@
+import { describe, test, expect } from '@jest/globals';
+import { contentDisposition } from '@/misc/content-disposition.js';
+import { correctFilename } from '@/misc/correct-filename.js';
+
+describe('misc:content-disposition', () => {
+    test('inline', () => {
+        expect(contentDisposition('inline', 'foo bar')).toBe('inline; filename=\"foo_bar\"; filename*=UTF-8\'\'foo%20bar');
+    });
+    test('attachment', () => {
+        expect(contentDisposition('attachment', 'foo bar')).toBe('attachment; filename=\"foo_bar\"; filename*=UTF-8\'\'foo%20bar');
+    });
+    test('non ascii', () => {
+        expect(contentDisposition('attachment', 'ファイル名')).toBe('attachment; filename=\"_____\"; filename*=UTF-8\'\'%E3%83%95%E3%82%A1%E3%82%A4%E3%83%AB%E5%90%8D');
+    });
+});
+
+describe('misc:correct-filename', () => {
+    test('simple', () => {
+        expect(correctFilename('filename', 'jpg')).toBe('filename.jpg');
+    });
+    test('with same ext', () => {
+        expect(correctFilename('filename.jpg', 'jpg')).toBe('filename.jpg');
+    });
+    test('with different ext', () => {
+        expect(correctFilename('filename.webp', 'jpg')).toBe('filename.webp.jpg');
+    });
+    test('non ascii with space', () => {
+        expect(correctFilename('ファイル 名前', 'jpg')).toBe('ファイル 名前.jpg');
+    });
+    test('jpeg', () => {
+        expect(correctFilename('filename.jpeg', 'jpg')).toBe('filename.jpeg');
+    });
+    test('tiff', () => {
+        expect(correctFilename('filename.tiff', 'tif')).toBe('filename.tiff');
+    });
+    test('null ext', () => {
+        expect(correctFilename('filename', null)).toBe('filename.unknown');
+    });
+});