Skip to content

Commit

Permalink
AP Link等は添付ファイル扱いしないようになど (misskey-dev#13754)
Browse files Browse the repository at this point in the history
* Linkは添付ファイルではない

* CHANGELOG
  • Loading branch information
mei23 authored Apr 28, 2024
1 parent 8e8ee2a commit c7d7da8
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 31 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
- Fix: 一部のタイムラインのストリーミングでインスタンスミュートが効かない問題を修正
- Fix: グローバルタイムラインで返信が表示されないことがある問題を修正
- Fix: リノートをミュートしたユーザの投稿のリノートがミュートされる問題を修正
- Fix: AP Link等は添付ファイル扱いしないようになど (#13754)

## 2024.3.1

Expand Down
19 changes: 10 additions & 9 deletions packages/backend/src/core/activitypub/models/ApImageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -39,24 +39,26 @@ 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');
}

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}`);
Expand Down Expand Up @@ -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);
Expand Down
17 changes: 7 additions & 10 deletions packages/backend/src/core/activitypub/models/ApNoteService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand Down
11 changes: 6 additions & 5 deletions packages/backend/src/core/activitypub/type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export interface IObject {
endTime?: Date;
icon?: any;
image?: any;
mediaType?: string;
url?: ApObject | string;
href?: string;
tag?: IObject | IObject[];
Expand Down Expand Up @@ -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 {
Expand Down
26 changes: 19 additions & 7 deletions packages/backend/test/unit/activitypub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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',
Expand All @@ -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 () => {
Expand All @@ -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',
Expand All @@ -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 () => {
Expand All @@ -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',
Expand All @@ -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);
});
});
});

0 comments on commit c7d7da8

Please sign in to comment.