Skip to content

Commit

Permalink
fix(backend): incorrect logic for determining whether Quote or not (m…
Browse files Browse the repository at this point in the history
…isskey-dev#13700)

* fix(backend): incorrect logic for determining whether Quote or not

* Update CHANGELOG.md

---------

Co-authored-by: syuilo <[email protected]>
  • Loading branch information
zyoshoka and syuilo authored Apr 14, 2024
1 parent 7cf0c18 commit 8c5d9a6
Show file tree
Hide file tree
Showing 11 changed files with 296 additions and 43 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
- Fix: エンドポイント`notes/translate`のエラーを改善
- Fix: CleanRemoteFilesProcessorService report progress from 100% (#13632)
- Fix: 一部の音声ファイルが映像ファイルとして扱われる問題を修正
- Fix: リプライのみの引用リノートと、CWのみの引用リノートが純粋なリノートとして誤って扱われてしまう問題を修正
- Fix: 登録にメール認証が必須になっている場合、登録されているメールアドレスを削除できないように
(Cherry-picked from https://github.com/MisskeyIO/misskey/pull/606)

Expand Down
6 changes: 3 additions & 3 deletions packages/backend/src/core/FanoutTimelineEndpointService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type { NotesRepository } from '@/models/_.js';
import { NoteEntityService } from '@/core/entities/NoteEntityService.js';
import { FanoutTimelineName, FanoutTimelineService } from '@/core/FanoutTimelineService.js';
import { isUserRelated } from '@/misc/is-user-related.js';
import { isPureRenote } from '@/misc/is-pure-renote.js';
import { isQuote, isRenote } from '@/misc/is-renote.js';
import { CacheService } from '@/core/CacheService.js';
import { isReply } from '@/misc/is-reply.js';
import { isInstanceMuted } from '@/misc/is-instance-muted.js';
Expand Down Expand Up @@ -95,7 +95,7 @@ export class FanoutTimelineEndpointService {

if (ps.excludePureRenotes) {
const parentFilter = filter;
filter = (note) => !isPureRenote(note) && parentFilter(note);
filter = (note) => (!isRenote(note) || isQuote(note)) && parentFilter(note);
}

if (ps.me) {
Expand All @@ -116,7 +116,7 @@ export class FanoutTimelineEndpointService {
filter = (note) => {
if (isUserRelated(note, userIdsWhoBlockingMe, ps.ignoreAuthorFromBlock)) return false;
if (isUserRelated(note, userIdsWhoMeMuting, ps.ignoreAuthorFromMute)) return false;
if (isPureRenote(note) && isUserRelated(note, userIdsWhoMeMutingRenotes, ps.ignoreAuthorFromMute)) return false;
if (isRenote(note) && !isQuote(note) && isUserRelated(note, userIdsWhoMeMutingRenotes, ps.ignoreAuthorFromMute)) return false;
if (isInstanceMuted(note, userMutedInstances)) return false;

return parentFilter(note);
Expand Down
23 changes: 17 additions & 6 deletions packages/backend/src/core/NoteCreateService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ export class NoteCreateService implements OnApplicationShutdown {
}

// Check blocking
if (data.renote && !this.isQuote(data)) {
if (this.isRenote(data) && !this.isQuote(data)) {
if (data.renote.userHost === null) {
if (data.renote.userId !== user.id) {
const blocked = await this.userBlockingService.checkBlocked(data.renote.userId, user.id);
Expand Down Expand Up @@ -641,7 +641,7 @@ export class NoteCreateService implements OnApplicationShutdown {
}

// If it is renote
if (data.renote) {
if (this.isRenote(data)) {
const type = this.isQuote(data) ? 'quote' : 'renote';

// Notify
Expand Down Expand Up @@ -725,9 +725,20 @@ export class NoteCreateService implements OnApplicationShutdown {
}

@bindThis
private isQuote(note: Option): note is Option & { renote: MiNote } {
// sync with misc/is-quote.ts
return !!note.renote && (!!note.text || !!note.cw || (!!note.files && !!note.files.length) || !!note.poll);
private isRenote(note: Option): note is Option & { renote: MiNote } {
return note.renote != null;
}

@bindThis
private isQuote(note: Option & { renote: MiNote }): note is Option & { renote: MiNote } & (
{ text: string } | { cw: string } | { reply: MiNote } | { poll: IPoll } | { files: MiDriveFile[] }
) {
// NOTE: SYNC WITH misc/is-quote.ts
return note.text != null ||
note.reply != null ||
note.cw != null ||
note.poll != null ||
(note.files != null && note.files.length > 0);
}

@bindThis
Expand Down Expand Up @@ -795,7 +806,7 @@ export class NoteCreateService implements OnApplicationShutdown {
private async renderNoteOrRenoteActivity(data: Option, note: MiNote) {
if (data.localOnly) return null;

const content = data.renote && !this.isQuote(data)
const content = this.isRenote(data) && !this.isQuote(data)
? this.apRendererService.renderAnnounce(data.renote.uri ? data.renote.uri : `${this.config.url}/notes/${data.renote.id}`, note)
: this.apRendererService.renderCreate(await this.apRendererService.renderNote(note, false), note);

Expand Down
4 changes: 2 additions & 2 deletions packages/backend/src/core/NoteDeleteService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { bindThis } from '@/decorators.js';
import { MetaService } from '@/core/MetaService.js';
import { SearchService } from '@/core/SearchService.js';
import { ModerationLogService } from '@/core/ModerationLogService.js';
import { isPureRenote } from '@/misc/is-pure-renote.js';
import { isQuote, isRenote } from '@/misc/is-renote.js';

@Injectable()
export class NoteDeleteService {
Expand Down Expand Up @@ -79,7 +79,7 @@ export class NoteDeleteService {
let renote: MiNote | null = null;

// if deleted note is renote
if (isPureRenote(note)) {
if (isRenote(note) && !isQuote(note)) {
renote = await this.notesRepository.findOneBy({
id: note.renoteId,
});
Expand Down
15 changes: 0 additions & 15 deletions packages/backend/src/misc/is-pure-renote.ts

This file was deleted.

12 changes: 0 additions & 12 deletions packages/backend/src/misc/is-quote.ts

This file was deleted.

36 changes: 36 additions & 0 deletions packages/backend/src/misc/is-renote.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* SPDX-FileCopyrightText: syuilo and misskey-project
* SPDX-License-Identifier: AGPL-3.0-only
*/

import type { MiNote } from '@/models/Note.js';

type Renote =
MiNote & {
renoteId: NonNullable<MiNote['renoteId']>
};

type Quote =
Renote & ({
text: NonNullable<MiNote['text']>
} | {
cw: NonNullable<MiNote['cw']>
} | {
replyId: NonNullable<MiNote['replyId']>
reply: NonNullable<MiNote['reply']>
} | {
hasPoll: true
});

export function isRenote(note: MiNote): note is Renote {
return note.renoteId != null;
}

export function isQuote(note: Renote): note is Quote {
// NOTE: SYNC WITH NoteCreateService.isQuote
return note.text != null ||
note.cw != null ||
note.replyId != null ||
note.hasPoll ||
note.fileIds.length > 0;
}
4 changes: 2 additions & 2 deletions packages/backend/src/server/ActivityPubServerService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import { UtilityService } from '@/core/UtilityService.js';
import { UserEntityService } from '@/core/entities/UserEntityService.js';
import { bindThis } from '@/decorators.js';
import { IActivity } from '@/core/activitypub/type.js';
import { isPureRenote } from '@/misc/is-pure-renote.js';
import { isQuote, isRenote } from '@/misc/is-renote.js';
import type { FastifyInstance, FastifyRequest, FastifyReply, FastifyPluginOptions, FastifyBodyParser } from 'fastify';
import type { FindOptionsWhere } from 'typeorm';

Expand Down Expand Up @@ -91,7 +91,7 @@ export class ActivityPubServerService {
*/
@bindThis
private async packActivity(note: MiNote): Promise<any> {
if (isPureRenote(note)) {
if (isRenote(note) && !isQuote(note)) {
const renote = await this.notesRepository.findOneByOrFail({ id: note.renoteId });
return this.apRendererService.renderAnnounce(renote.uri ? renote.uri : `${this.config.url}/notes/${renote.id}`, note);
}
Expand Down
6 changes: 3 additions & 3 deletions packages/backend/src/server/api/endpoints/notes/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { Endpoint } from '@/server/api/endpoint-base.js';
import { NoteEntityService } from '@/core/entities/NoteEntityService.js';
import { NoteCreateService } from '@/core/NoteCreateService.js';
import { DI } from '@/di-symbols.js';
import { isPureRenote } from '@/misc/is-pure-renote.js';
import { isQuote, isRenote } from '@/misc/is-renote.js';
import { MetaService } from '@/core/MetaService.js';
import { UtilityService } from '@/core/UtilityService.js';
import { IdentifiableError } from '@/misc/identifiable-error.js';
Expand Down Expand Up @@ -275,7 +275,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-

if (renote == null) {
throw new ApiError(meta.errors.noSuchRenoteTarget);
} else if (isPureRenote(renote)) {
} else if (isRenote(renote) && !isQuote(renote)) {
throw new ApiError(meta.errors.cannotReRenote);
}

Expand Down Expand Up @@ -321,7 +321,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-

if (reply == null) {
throw new ApiError(meta.errors.noSuchReplyTarget);
} else if (isPureRenote(reply)) {
} else if (isRenote(reply) && !isQuote(reply)) {
throw new ApiError(meta.errors.cannotReplyToPureRenote);
} else if (!await this.noteEntityService.isVisibleForMe(reply, me.id)) {
throw new ApiError(meta.errors.cannotReplyToInvisibleNote);
Expand Down
144 changes: 144 additions & 0 deletions packages/backend/test/unit/NoteCreateService.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
/*
* SPDX-FileCopyrightText: syuilo and misskey-project
* SPDX-License-Identifier: AGPL-3.0-only
*/

import { Test } from '@nestjs/testing';

import { CoreModule } from '@/core/CoreModule.js';
import { NoteCreateService } from '@/core/NoteCreateService.js';
import { GlobalModule } from '@/GlobalModule.js';
import { MiNote } from '@/models/Note.js';
import { IPoll } from '@/models/Poll.js';
import { MiDriveFile } from '@/models/DriveFile.js';

describe('NoteCreateService', () => {
let noteCreateService: NoteCreateService;

beforeAll(async () => {
const app = await Test.createTestingModule({
imports: [GlobalModule, CoreModule],
}).compile();
noteCreateService = app.get<NoteCreateService>(NoteCreateService);
});

describe('is-renote', () => {
const base: MiNote = {
id: 'some-note-id',
replyId: null,
reply: null,
renoteId: null,
renote: null,
threadId: null,
text: null,
name: null,
cw: null,
userId: 'some-user-id',
user: null,
localOnly: false,
reactionAcceptance: null,
renoteCount: 0,
repliesCount: 0,
clippedCount: 0,
reactions: {},
visibility: 'public',
uri: null,
url: null,
fileIds: [],
attachedFileTypes: [],
visibleUserIds: [],
mentions: [],
mentionedRemoteUsers: '',
reactionAndUserPairCache: [],
emojis: [],
tags: [],
hasPoll: false,
channelId: null,
channel: null,
userHost: null,
replyUserId: null,
replyUserHost: null,
renoteUserId: null,
renoteUserHost: null,
};

const poll: IPoll = {
choices: ['kinoko', 'takenoko'],
multiple: false,
expiresAt: null,
};

const file: MiDriveFile = {
id: 'some-file-id',
userId: null,
user: null,
userHost: null,
md5: '',
name: '',
type: '',
size: 0,
comment: null,
blurhash: null,
properties: {},
storedInternal: false,
url: '',
thumbnailUrl: null,
webpublicUrl: null,
webpublicType: null,
accessKey: null,
thumbnailAccessKey: null,
webpublicAccessKey: null,
uri: null,
src: null,
folderId: null,
folder: null,
isSensitive: false,
maybeSensitive: false,
maybePorn: false,
isLink: false,
requestHeaders: null,
requestIp: null,
};

test('note without renote should not be Renote', () => {
const note = { renote: null };
expect(noteCreateService['isRenote'](note)).toBe(false);
});

test('note with renote should be Renote and not be Quote', () => {
const note = { renote: base };
expect(noteCreateService['isRenote'](note)).toBe(true);
expect(noteCreateService['isQuote'](note)).toBe(false);
});

test('note with renote and text should be Quote', () => {
const note = { renote: base, text: 'some-text' };
expect(noteCreateService['isRenote'](note)).toBe(true);
expect(noteCreateService['isQuote'](note)).toBe(true);
});

test('note with renote and cw should be Quote', () => {
const note = { renote: base, cw: 'some-cw' };
expect(noteCreateService['isRenote'](note)).toBe(true);
expect(noteCreateService['isQuote'](note)).toBe(true);
});

test('note with renote and reply should be Quote', () => {
const note = { renote: base, reply: { ...base, id: 'another-note-id' } };
expect(noteCreateService['isRenote'](note)).toBe(true);
expect(noteCreateService['isQuote'](note)).toBe(true);
});

test('note with renote and poll should be Quote', () => {
const note = { renote: base, poll };
expect(noteCreateService['isRenote'](note)).toBe(true);
expect(noteCreateService['isQuote'](note)).toBe(true);
});

test('note with renote and non-empty files should be Quote', () => {
const note = { renote: base, files: [file] };
expect(noteCreateService['isRenote'](note)).toBe(true);
expect(noteCreateService['isQuote'](note)).toBe(true);
});
});
});
Loading

0 comments on commit 8c5d9a6

Please sign in to comment.