refactor(backend): skip fetching notes when the data is same-origin (#11200)

* refactor(backend): skip fetching notes when the data is same-origin

* Update CHANGELOG.md

* sentFrom
This commit is contained in:
Kagami Sascha Rosylight 2023-07-09 01:59:44 +02:00 committed by GitHub
parent 74a05ec739
commit 5059d4d7e1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 131 additions and 23 deletions

View file

@ -39,6 +39,7 @@
- nsfwjs のモデルロードを排他することで、重複ロードによってメモリ使用量が増加しないように - nsfwjs のモデルロードを排他することで、重複ロードによってメモリ使用量が増加しないように
- 連合の配送ジョブのパフォーマンスを向上ロック機構の見直し、Redisキャッシュの活用 - 連合の配送ジョブのパフォーマンスを向上ロック機構の見直し、Redisキャッシュの活用
- 全体的なDBクエリのパフォーマンスを向上 - 全体的なDBクエリのパフォーマンスを向上
- featuredートのsignedGet回数を減らしました
## 13.13.2 ## 13.13.2

View file

@ -177,7 +177,7 @@ export class ApNoteService {
// リプライ // リプライ
const reply: Note | null = note.inReplyTo const reply: Note | null = note.inReplyTo
? await this.resolveNote(note.inReplyTo, resolver) ? await this.resolveNote(note.inReplyTo, { resolver })
.then(x => { .then(x => {
if (x == null) { if (x == null) {
this.logger.warn('Specified inReplyTo, but not found'); this.logger.warn('Specified inReplyTo, but not found');
@ -293,9 +293,8 @@ export class ApNoteService {
* Misskeyに登録しそれを返します * Misskeyに登録しそれを返します
*/ */
@bindThis @bindThis
public async resolveNote(value: string | IObject, resolver?: Resolver): Promise<Note | null> { public async resolveNote(value: string | IObject, options: { sentFrom?: URL, resolver?: Resolver } = {}): Promise<Note | null> {
const uri = typeof value === 'string' ? value : value.id; const uri = getApId(value);
if (uri == null) throw new Error('missing uri');
// ブロックしていたら中断 // ブロックしていたら中断
const meta = await this.metaService.fetch(); const meta = await this.metaService.fetch();
@ -318,7 +317,8 @@ export class ApNoteService {
// リモートサーバーからフェッチしてきて登録 // リモートサーバーからフェッチしてきて登録
// ここでuriの代わりに添付されてきたNote Objectが指定されていると、サーバーフェッチを経ずにートが生成されるが // ここでuriの代わりに添付されてきたNote Objectが指定されていると、サーバーフェッチを経ずにートが生成されるが
// 添付されてきたNote Objectは偽装されている可能性があるため、常にuriを指定してサーバーフェッチを行う。 // 添付されてきたNote Objectは偽装されている可能性があるため、常にuriを指定してサーバーフェッチを行う。
return await this.createNote(uri, resolver, true); const createFrom = options.sentFrom?.origin === new URL(uri).origin ? value : uri;
return await this.createNote(createFrom, options.resolver, true);
} finally { } finally {
unlock(); unlock();
} }

View file

@ -260,7 +260,7 @@ export class ApPersonService implements OnModuleInit {
// Create user // Create user
let user: RemoteUser | null = null; let user: RemoteUser | null = null;
try { try {
// Start transaction // Start transaction
await this.db.transaction(async transactionalEntityManager => { await this.db.transaction(async transactionalEntityManager => {
user = await transactionalEntityManager.save(new User({ user = await transactionalEntityManager.save(new User({
id: this.idService.genId(), id: this.idService.genId(),
@ -306,9 +306,9 @@ export class ApPersonService implements OnModuleInit {
} }
}); });
} catch (e) { } catch (e) {
// duplicate key error // duplicate key error
if (isDuplicateKeyValueError(e)) { if (isDuplicateKeyValueError(e)) {
// /users/@a => /users/:id のように入力がaliasなときにエラーになることがあるのを対応 // /users/@a => /users/:id のように入力がaliasなときにエラーになることがあるのを対応
const u = await this.usersRepository.findOneBy({ uri: person.id }); const u = await this.usersRepository.findOneBy({ uri: person.id });
if (u == null) throw new Error('already registered'); if (u == null) throw new Error('already registered');
@ -604,7 +604,10 @@ export class ApPersonService implements OnModuleInit {
const featuredNotes = await Promise.all(items const featuredNotes = await Promise.all(items
.filter(item => getApType(item) === 'Note') // TODO: Noteでなくてもいいかも .filter(item => getApType(item) === 'Note') // TODO: Noteでなくてもいいかも
.slice(0, 5) .slice(0, 5)
.map(item => limit(() => this.apNoteService.resolveNote(item, _resolver)))); .map(item => limit(() => this.apNoteService.resolveNote(item, {
resolver: _resolver,
sentFrom: new URL(user.uri),
}))));
await this.db.transaction(async transactionalEntityManager => { await this.db.transaction(async transactionalEntityManager => {
await transactionalEntityManager.delete(UserNotePining, { userId: user.id }); await transactionalEntityManager.delete(UserNotePining, { userId: user.id });

View file

@ -18,7 +18,8 @@ type MockResponse = {
}; };
export class MockResolver extends Resolver { export class MockResolver extends Resolver {
private _rs = new Map<string, MockResponse>(); #responseMap = new Map<string, MockResponse>();
#remoteGetTrials: string[] = [];
constructor(loggerService: LoggerService) { constructor(loggerService: LoggerService) {
super( super(
@ -38,18 +39,28 @@ export class MockResolver extends Resolver {
); );
} }
public async _register(uri: string, content: string | Record<string, any>, type = 'application/activity+json') { public register(uri: string, content: string | Record<string, any>, type = 'application/activity+json'): void {
this._rs.set(uri, { this.#responseMap.set(uri, {
type, type,
content: typeof content === 'string' ? content : JSON.stringify(content), content: typeof content === 'string' ? content : JSON.stringify(content),
}); });
} }
public clear(): void {
this.#responseMap.clear();
this.#remoteGetTrials.length = 0;
}
public remoteGetTrials(): string[] {
return this.#remoteGetTrials;
}
@bindThis @bindThis
public async resolve(value: string | IObject): Promise<IObject> { public async resolve(value: string | IObject): Promise<IObject> {
if (typeof value !== 'string') return value; if (typeof value !== 'string') return value;
const r = this._rs.get(value); this.#remoteGetTrials.push(value);
const r = this.#responseMap.get(value);
if (!r) { if (!r) {
throw new Error('Not registed for mock'); throw new Error('Not registed for mock');

View file

@ -11,16 +11,19 @@ import { GlobalModule } from '@/GlobalModule.js';
import { CoreModule } from '@/core/CoreModule.js'; import { CoreModule } from '@/core/CoreModule.js';
import { FederatedInstanceService } from '@/core/FederatedInstanceService.js'; import { FederatedInstanceService } from '@/core/FederatedInstanceService.js';
import { LoggerService } from '@/core/LoggerService.js'; import { LoggerService } from '@/core/LoggerService.js';
import type { IActor } from '@/core/activitypub/type.js'; import type { IActor, ICollection, IPost } from '@/core/activitypub/type.js';
import { Note } from '@/models/index.js'; import { Note } from '@/models/index.js';
import { secureRndstr } from '@/misc/secure-rndstr.js'; import { secureRndstr } from '@/misc/secure-rndstr.js';
import { MockResolver } from '../misc/mock-resolver.js'; import { MockResolver } from '../misc/mock-resolver.js';
const host = 'https://host1.test'; const host = 'https://host1.test';
function createRandomActor(): IActor & { id: string } { type NonTransientIActor = IActor & { id: string };
type NonTransientIPost = IPost & { id: string };
function createRandomActor({ actorHost = host } = {}): NonTransientIActor {
const preferredUsername = secureRndstr(8); const preferredUsername = secureRndstr(8);
const actorId = `${host}/users/${preferredUsername.toLowerCase()}`; const actorId = `${actorHost}/users/${preferredUsername.toLowerCase()}`;
return { return {
'@context': 'https://www.w3.org/ns/activitystreams', '@context': 'https://www.w3.org/ns/activitystreams',
@ -32,13 +35,41 @@ function createRandomActor(): IActor & { id: string } {
}; };
} }
function createRandomNote(actor: NonTransientIActor): NonTransientIPost {
const id = secureRndstr(8);
const noteId = `${new URL(actor.id).origin}/notes/${id}`;
return {
id: noteId,
type: 'Note',
attributedTo: actor.id,
content: 'test test foo',
};
}
function createRandomNotes(actor: NonTransientIActor, length: number): NonTransientIPost[] {
return new Array(length).fill(null).map(() => createRandomNote(actor));
}
function createRandomFeaturedCollection(actor: NonTransientIActor, length: number): ICollection {
const items = createRandomNotes(actor, length);
return {
'@context': 'https://www.w3.org/ns/activitystreams',
type: 'Collection',
id: actor.outbox as string,
totalItems: items.length,
items,
};
}
describe('ActivityPub', () => { describe('ActivityPub', () => {
let noteService: ApNoteService; let noteService: ApNoteService;
let personService: ApPersonService; let personService: ApPersonService;
let rendererService: ApRendererService; let rendererService: ApRendererService;
let resolver: MockResolver; let resolver: MockResolver;
beforeEach(async () => { beforeAll(async () => {
const app = await Test.createTestingModule({ const app = await Test.createTestingModule({
imports: [GlobalModule, CoreModule], imports: [GlobalModule, CoreModule],
}).compile(); }).compile();
@ -53,7 +84,11 @@ describe('ActivityPub', () => {
// Prevent ApPersonService from fetching instance, as it causes Jest import-after-test error // Prevent ApPersonService from fetching instance, as it causes Jest import-after-test error
const federatedInstanceService = app.get<FederatedInstanceService>(FederatedInstanceService); const federatedInstanceService = app.get<FederatedInstanceService>(FederatedInstanceService);
jest.spyOn(federatedInstanceService, 'fetch').mockImplementation(() => new Promise(() => {})); jest.spyOn(federatedInstanceService, 'fetch').mockImplementation(() => new Promise(() => { }));
});
beforeEach(() => {
resolver.clear();
}); });
describe('Parse minimum object', () => { describe('Parse minimum object', () => {
@ -69,7 +104,7 @@ describe('ActivityPub', () => {
}; };
test('Minimum Actor', async () => { test('Minimum Actor', async () => {
resolver._register(actor.id, actor); resolver.register(actor.id, actor);
const user = await personService.createPerson(actor.id, resolver); const user = await personService.createPerson(actor.id, resolver);
@ -79,8 +114,8 @@ describe('ActivityPub', () => {
}); });
test('Minimum Note', async () => { test('Minimum Note', async () => {
resolver._register(actor.id, actor); resolver.register(actor.id, actor);
resolver._register(post.id, post); resolver.register(post.id, post);
const note = await noteService.createNote(post.id, resolver, true); const note = await noteService.createNote(post.id, resolver, true);
@ -97,7 +132,7 @@ describe('ActivityPub', () => {
name: secureRndstr(129), name: secureRndstr(129),
}; };
resolver._register(actor.id, actor); resolver.register(actor.id, actor);
const user = await personService.createPerson(actor.id, resolver); const user = await personService.createPerson(actor.id, resolver);
@ -110,7 +145,7 @@ describe('ActivityPub', () => {
name: '', name: '',
}; };
resolver._register(actor.id, actor); resolver.register(actor.id, actor);
const user = await personService.createPerson(actor.id, resolver); const user = await personService.createPerson(actor.id, resolver);
@ -126,4 +161,62 @@ describe('ActivityPub', () => {
} as Note); } as Note);
}); });
}); });
describe('Featured', () => {
test('Fetch featured notes from IActor', async () => {
const actor = createRandomActor();
actor.featured = `${actor.id}/collections/featured`;
const featured = createRandomFeaturedCollection(actor, 5);
resolver.register(actor.id, actor);
resolver.register(actor.featured, featured);
await personService.createPerson(actor.id, resolver);
// All notes in `featured` are same-origin, no need to fetch notes again
assert.deepStrictEqual(resolver.remoteGetTrials(), [actor.id, actor.featured]);
// Created notes without resolving anything
for (const item of featured.items as IPost[]) {
const note = await noteService.fetchNote(item);
assert.ok(note);
assert.strictEqual(note.text, 'test test foo');
assert.strictEqual(note.uri, item.id);
}
});
test('Fetch featured notes from IActor pointing to another remote server', async () => {
const actor1 = createRandomActor();
actor1.featured = `${actor1.id}/collections/featured`;
const actor2 = createRandomActor({ actorHost: 'https://host2.test' });
const actor2Note = createRandomNote(actor2);
const featured = createRandomFeaturedCollection(actor1, 0);
(featured.items as IPost[]).push({
...actor2Note,
content: 'test test bar', // fraud!
});
resolver.register(actor1.id, actor1);
resolver.register(actor1.featured, featured);
resolver.register(actor2.id, actor2);
resolver.register(actor2Note.id, actor2Note);
await personService.createPerson(actor1.id, resolver);
// actor2Note is from a different server and needs to be fetched again
assert.deepStrictEqual(
resolver.remoteGetTrials(),
[actor1.id, actor1.featured, actor2Note.id, actor2.id],
);
const note = await noteService.fetchNote(actor2Note.id);
assert.ok(note);
// Reflects the original content instead of the fraud
assert.strictEqual(note.text, 'test test foo');
assert.strictEqual(note.uri, actor2Note.id);
});
});
}); });