AP Link等は添付ファイル扱いしないようになど (#13754)

* Linkは添付ファイルではない

* CHANGELOG

5 files changed, 43 insertions, 31 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f22cec856f..68015596bd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -73,6 +73,7 @@
 - Fix: 一部のタイムラインのストリーミングでインスタンスミュートが効かない問題を修正
 - Fix: グローバルタイムラインで返信が表示されないことがある問題を修正
 - Fix: リノートをミュートしたユーザの投稿のリノートがミュートされる問題を修正
+- Fix: AP Link等は添付ファイル扱いしないようになど (#13754)
 
 ## 2024.3.1
 
diff --git a/packages/backend/src/core/activitypub/models/ApImageService.ts b/packages/backend/src/core/activitypub/models/ApImageService.ts
index 89b6ef23d0..3691967270 100644
--- a/packages/backend/src/core/activitypub/models/ApImageService.ts
+++ b/packages/backend/src/core/activitypub/models/ApImageService.ts
@@ -17,7 +17,7 @@ import { bindThis } from '@/decorators.js';
 import { checkHttps } from '@/misc/check-https.js';
 import { ApResolverService } from '../ApResolverService.js';
 import { ApLoggerService } from '../ApLoggerService.js';
-import type { IObject } from '../type.js';
+import { isDocument, type IObject } from '../type.js';
 
 @Injectable()
 export class ApImageService {
@@ -39,7 +39,7 @@ export class ApImageService {
 	 * Imageを作成します。
 	 */
 	@bindThis
-	public async createImage(actor: MiRemoteUser, value: string | IObject): Promise<MiDriveFile> {
+	public async createImage(actor: MiRemoteUser, value: string | IObject): Promise<MiDriveFile | null> {
 		// 投稿者が凍結されていたらスキップ
 		if (actor.isSuspended) {
 			throw new Error('actor has been suspended');
@@ -47,16 +47,18 @@ export class ApImageService {
 
 		const image = await this.apResolverService.createResolver().resolve(value);
 
+		if (!isDocument(image)) return null;
+
 		if (image.url == null) {
-			throw new Error('invalid image: url not provided');
+			return null;
 		}
 
 		if (typeof image.url !== 'string') {
-			throw new Error('invalid image: unexpected type of url: ' + JSON.stringify(image.url, null, 2));
+			return null;
 		}
 
 		if (!checkHttps(image.url)) {
-			throw new Error('invalid image: unexpected schema of url: ' + image.url);
+			return null;
 		}
 
 		this.logger.info(`Creating the Image: ${image.url}`);
@@ -86,12 +88,11 @@ export class ApImageService {
 	/**
 	 * Imageを解決します。
 	 *
-	 * Misskeyに対象のImageが登録されていればそれを返し、そうでなければ
-	 * リモートサーバーからフェッチしてMisskeyに登録しそれを返します。
+	 * ImageをリモートサーバーからフェッチしてMisskeyに登録しそれを返します。
 	 */
 	@bindThis
-	public async resolveImage(actor: MiRemoteUser, value: string | IObject): Promise<MiDriveFile> {
-		// TODO
+	public async resolveImage(actor: MiRemoteUser, value: string | IObject): Promise<MiDriveFile | null> {
+		// TODO: Misskeyに対象のImageが登録されていればそれを返す
 
 		// リモートサーバーからフェッチしてきて登録
 		return await this.createImage(actor, value);
diff --git a/packages/backend/src/core/activitypub/models/ApNoteService.ts b/packages/backend/src/core/activitypub/models/ApNoteService.ts
index 4d64b08e15..05f7879983 100644
--- a/packages/backend/src/core/activitypub/models/ApNoteService.ts
+++ b/packages/backend/src/core/activitypub/models/ApNoteService.ts
@@ -4,7 +4,6 @@
  */
 
 import { forwardRef, Inject, Injectable } from '@nestjs/common';
-import promiseLimit from 'promise-limit';
 import { In } from 'typeorm';
 import { DI } from '@/di-symbols.js';
 import type { PollsRepository, EmojisRepository } from '@/models/_.js';
@@ -209,15 +208,13 @@ export class ApNoteService {
 		}
 
 		// 添付ファイル
-		// TODO: attachmentは必ずしもImageではない
-		// TODO: attachmentは必ずしも配列ではない
-		const limit = promiseLimit<MiDriveFile>(2);
-		const files = (await Promise.all(toArray(note.attachment).map(attach => (
-			limit(() => this.apImageService.resolveImage(actor, {
-				...attach,
-				sensitive: note.sensitive, // Noteがsensitiveなら添付もsensitiveにする
-			}))
-		))));
+		const files: MiDriveFile[] = [];
+
+		for (const attach of toArray(note.attachment)) {
+			attach.sensitive ||= note.sensitive;	// Noteがsensitiveなら添付もsensitiveにする
+			const file = await this.apImageService.resolveImage(actor, attach);
+			if (file) files.push(file);
+		}
 
 		// リプライ
 		const reply: MiNote | null = note.inReplyTo
diff --git a/packages/backend/src/core/activitypub/type.ts b/packages/backend/src/core/activitypub/type.ts
index b43dddad61..09322888d5 100644
--- a/packages/backend/src/core/activitypub/type.ts
+++ b/packages/backend/src/core/activitypub/type.ts
@@ -25,6 +25,7 @@ export interface IObject {
 	endTime?: Date;
 	icon?: any;
 	image?: any;
+	mediaType?: string;
 	url?: ApObject | string;
 	href?: string;
 	tag?: IObject | IObject[];
@@ -240,14 +241,14 @@ export interface IKey extends IObject {
 }
 
 export interface IApDocument extends IObject {
-	type: 'Document';
-	name: string | null;
-	mediaType: string;
+	type: 'Audio' | 'Document' | 'Image' | 'Page' | 'Video';
 }
 
-export interface IApImage extends IObject {
+export const isDocument = (object: IObject): object is IApDocument =>
+	['Audio', 'Document', 'Image', 'Page', 'Video'].includes(getApType(object));
+
+export interface IApImage extends IApDocument {
 	type: 'Image';
-	name: string | null;
 }
 
 export interface ICreate extends IActivity {
diff --git a/packages/backend/test/unit/activitypub.ts b/packages/backend/test/unit/activitypub.ts
index b4b06b06bd..aa3f3a4ff1 100644
--- a/packages/backend/test/unit/activitypub.ts
+++ b/packages/backend/test/unit/activitypub.ts
@@ -17,7 +17,7 @@ import { GlobalModule } from '@/GlobalModule.js';
 import { CoreModule } from '@/core/CoreModule.js';
 import { FederatedInstanceService } from '@/core/FederatedInstanceService.js';
 import { LoggerService } from '@/core/LoggerService.js';
-import type { IActor, IApDocument, ICollection, IPost } from '@/core/activitypub/type.js';
+import type { IActor, IApDocument, ICollection, IObject, IPost } from '@/core/activitypub/type.js';
 import { MiMeta, MiNote } from '@/models/_.js';
 import { secureRndstr } from '@/misc/secure-rndstr.js';
 import { DownloadService } from '@/core/DownloadService.js';
@@ -295,7 +295,7 @@ describe('ActivityPub', () => {
 				await createRandomRemoteUser(resolver, personService),
 				imageObject,
 			);
-			assert.ok(!driveFile.isLink);
+			assert.ok(driveFile && !driveFile.isLink);
 
 			const sensitiveImageObject: IApDocument = {
 				type: 'Document',
@@ -308,7 +308,7 @@ describe('ActivityPub', () => {
 				await createRandomRemoteUser(resolver, personService),
 				sensitiveImageObject,
 			);
-			assert.ok(!sensitiveDriveFile.isLink);
+			assert.ok(sensitiveDriveFile && !sensitiveDriveFile.isLink);
 		});
 
 		test('cacheRemoteFiles=false disables caching', async () => {
@@ -324,7 +324,7 @@ describe('ActivityPub', () => {
 				await createRandomRemoteUser(resolver, personService),
 				imageObject,
 			);
-			assert.ok(driveFile.isLink);
+			assert.ok(driveFile && driveFile.isLink);
 
 			const sensitiveImageObject: IApDocument = {
 				type: 'Document',
@@ -337,7 +337,7 @@ describe('ActivityPub', () => {
 				await createRandomRemoteUser(resolver, personService),
 				sensitiveImageObject,
 			);
-			assert.ok(sensitiveDriveFile.isLink);
+			assert.ok(sensitiveDriveFile && sensitiveDriveFile.isLink);
 		});
 
 		test('cacheRemoteSensitiveFiles=false only affects sensitive files', async () => {
@@ -353,7 +353,7 @@ describe('ActivityPub', () => {
 				await createRandomRemoteUser(resolver, personService),
 				imageObject,
 			);
-			assert.ok(!driveFile.isLink);
+			assert.ok(driveFile && !driveFile.isLink);
 
 			const sensitiveImageObject: IApDocument = {
 				type: 'Document',
@@ -366,7 +366,19 @@ describe('ActivityPub', () => {
 				await createRandomRemoteUser(resolver, personService),
 				sensitiveImageObject,
 			);
-			assert.ok(sensitiveDriveFile.isLink);
+			assert.ok(sensitiveDriveFile && sensitiveDriveFile.isLink);
+		});
+
+		test('Link is not an attachment files', async () => {
+			const linkObject: IObject = {
+				type: 'Link',
+				href: 'https://example.com/',
+			};
+			const driveFile = await imageService.createImage(
+				await createRandomRemoteUser(resolver, personService),
+				linkObject,
+			);
+			assert.strictEqual(driveFile, null);
 		});
 	});
 });