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

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

* Update CHANGELOG.md

---------

Co-authored-by: syuilo <4439005+syuilo@users.noreply.github.com>
This commit is contained in:
zyoshoka 2024-04-14 10:23:48 +09:00 committed by GitHub
parent 7cf0c18f83
commit 8c5d9a6295
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 296 additions and 43 deletions

View File

@ -48,6 +48,7 @@
- Fix: エンドポイント`notes/translate`のエラーを改善 - Fix: エンドポイント`notes/translate`のエラーを改善
- Fix: CleanRemoteFilesProcessorService report progress from 100% (#13632) - Fix: CleanRemoteFilesProcessorService report progress from 100% (#13632)
- Fix: 一部の音声ファイルが映像ファイルとして扱われる問題を修正 - Fix: 一部の音声ファイルが映像ファイルとして扱われる問題を修正
- Fix: リプライのみの引用リートと、CWのみの引用リートが純粋なリートとして誤って扱われてしまう問題を修正
- Fix: 登録にメール認証が必須になっている場合、登録されているメールアドレスを削除できないように - Fix: 登録にメール認証が必須になっている場合、登録されているメールアドレスを削除できないように
(Cherry-picked from https://github.com/MisskeyIO/misskey/pull/606) (Cherry-picked from https://github.com/MisskeyIO/misskey/pull/606)

View File

@ -13,7 +13,7 @@ import type { NotesRepository } from '@/models/_.js';
import { NoteEntityService } from '@/core/entities/NoteEntityService.js'; import { NoteEntityService } from '@/core/entities/NoteEntityService.js';
import { FanoutTimelineName, FanoutTimelineService } from '@/core/FanoutTimelineService.js'; import { FanoutTimelineName, FanoutTimelineService } from '@/core/FanoutTimelineService.js';
import { isUserRelated } from '@/misc/is-user-related.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 { CacheService } from '@/core/CacheService.js';
import { isReply } from '@/misc/is-reply.js'; import { isReply } from '@/misc/is-reply.js';
import { isInstanceMuted } from '@/misc/is-instance-muted.js'; import { isInstanceMuted } from '@/misc/is-instance-muted.js';
@ -95,7 +95,7 @@ export class FanoutTimelineEndpointService {
if (ps.excludePureRenotes) { if (ps.excludePureRenotes) {
const parentFilter = filter; const parentFilter = filter;
filter = (note) => !isPureRenote(note) && parentFilter(note); filter = (note) => (!isRenote(note) || isQuote(note)) && parentFilter(note);
} }
if (ps.me) { if (ps.me) {
@ -116,7 +116,7 @@ export class FanoutTimelineEndpointService {
filter = (note) => { filter = (note) => {
if (isUserRelated(note, userIdsWhoBlockingMe, ps.ignoreAuthorFromBlock)) return false; if (isUserRelated(note, userIdsWhoBlockingMe, ps.ignoreAuthorFromBlock)) return false;
if (isUserRelated(note, userIdsWhoMeMuting, ps.ignoreAuthorFromMute)) 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; if (isInstanceMuted(note, userMutedInstances)) return false;
return parentFilter(note); return parentFilter(note);

View File

@ -306,7 +306,7 @@ export class NoteCreateService implements OnApplicationShutdown {
} }
// Check blocking // Check blocking
if (data.renote && !this.isQuote(data)) { if (this.isRenote(data) && !this.isQuote(data)) {
if (data.renote.userHost === null) { if (data.renote.userHost === null) {
if (data.renote.userId !== user.id) { if (data.renote.userId !== user.id) {
const blocked = await this.userBlockingService.checkBlocked(data.renote.userId, user.id); const blocked = await this.userBlockingService.checkBlocked(data.renote.userId, user.id);
@ -641,7 +641,7 @@ export class NoteCreateService implements OnApplicationShutdown {
} }
// If it is renote // If it is renote
if (data.renote) { if (this.isRenote(data)) {
const type = this.isQuote(data) ? 'quote' : 'renote'; const type = this.isQuote(data) ? 'quote' : 'renote';
// Notify // Notify
@ -725,9 +725,20 @@ export class NoteCreateService implements OnApplicationShutdown {
} }
@bindThis @bindThis
private isQuote(note: Option): note is Option & { renote: MiNote } { private isRenote(note: Option): note is Option & { renote: MiNote } {
// sync with misc/is-quote.ts return note.renote != null;
return !!note.renote && (!!note.text || !!note.cw || (!!note.files && !!note.files.length) || !!note.poll); }
@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 @bindThis
@ -795,7 +806,7 @@ export class NoteCreateService implements OnApplicationShutdown {
private async renderNoteOrRenoteActivity(data: Option, note: MiNote) { private async renderNoteOrRenoteActivity(data: Option, note: MiNote) {
if (data.localOnly) return null; 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.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); : this.apRendererService.renderCreate(await this.apRendererService.renderNote(note, false), note);

View File

@ -24,7 +24,7 @@ import { bindThis } from '@/decorators.js';
import { MetaService } from '@/core/MetaService.js'; import { MetaService } from '@/core/MetaService.js';
import { SearchService } from '@/core/SearchService.js'; import { SearchService } from '@/core/SearchService.js';
import { ModerationLogService } from '@/core/ModerationLogService.js'; import { ModerationLogService } from '@/core/ModerationLogService.js';
import { isPureRenote } from '@/misc/is-pure-renote.js'; import { isQuote, isRenote } from '@/misc/is-renote.js';
@Injectable() @Injectable()
export class NoteDeleteService { export class NoteDeleteService {
@ -79,7 +79,7 @@ export class NoteDeleteService {
let renote: MiNote | null = null; let renote: MiNote | null = null;
// if deleted note is renote // if deleted note is renote
if (isPureRenote(note)) { if (isRenote(note) && !isQuote(note)) {
renote = await this.notesRepository.findOneBy({ renote = await this.notesRepository.findOneBy({
id: note.renoteId, id: note.renoteId,
}); });

View File

@ -1,15 +0,0 @@
/*
* SPDX-FileCopyrightText: syuilo and misskey-project
* SPDX-License-Identifier: AGPL-3.0-only
*/
import type { MiNote } from '@/models/Note.js';
export function isPureRenote(note: MiNote): note is MiNote & { renoteId: NonNullable<MiNote['renoteId']> } {
if (!note.renoteId) return false;
if (note.text) return false; // it's quoted with text
if (note.fileIds.length !== 0) return false; // it's quoted with files
if (note.hasPoll) return false; // it's quoted with poll
return true;
}

View File

@ -1,12 +0,0 @@
/*
* SPDX-FileCopyrightText: syuilo and misskey-project
* SPDX-License-Identifier: AGPL-3.0-only
*/
import type { MiNote } from '@/models/Note.js';
// eslint-disable-next-line import/no-default-export
export default function(note: MiNote): boolean {
// sync with NoteCreateService.isQuote
return note.renoteId != null && (note.text != null || note.hasPoll || (note.fileIds != null && note.fileIds.length > 0));
}

View File

@ -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;
}

View File

@ -28,7 +28,7 @@ import { UtilityService } from '@/core/UtilityService.js';
import { UserEntityService } from '@/core/entities/UserEntityService.js'; import { UserEntityService } from '@/core/entities/UserEntityService.js';
import { bindThis } from '@/decorators.js'; import { bindThis } from '@/decorators.js';
import { IActivity } from '@/core/activitypub/type.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 { FastifyInstance, FastifyRequest, FastifyReply, FastifyPluginOptions, FastifyBodyParser } from 'fastify';
import type { FindOptionsWhere } from 'typeorm'; import type { FindOptionsWhere } from 'typeorm';
@ -91,7 +91,7 @@ export class ActivityPubServerService {
*/ */
@bindThis @bindThis
private async packActivity(note: MiNote): Promise<any> { private async packActivity(note: MiNote): Promise<any> {
if (isPureRenote(note)) { if (isRenote(note) && !isQuote(note)) {
const renote = await this.notesRepository.findOneByOrFail({ id: note.renoteId }); const renote = await this.notesRepository.findOneByOrFail({ id: note.renoteId });
return this.apRendererService.renderAnnounce(renote.uri ? renote.uri : `${this.config.url}/notes/${renote.id}`, note); return this.apRendererService.renderAnnounce(renote.uri ? renote.uri : `${this.config.url}/notes/${renote.id}`, note);
} }

View File

@ -16,7 +16,7 @@ import { Endpoint } from '@/server/api/endpoint-base.js';
import { NoteEntityService } from '@/core/entities/NoteEntityService.js'; import { NoteEntityService } from '@/core/entities/NoteEntityService.js';
import { NoteCreateService } from '@/core/NoteCreateService.js'; import { NoteCreateService } from '@/core/NoteCreateService.js';
import { DI } from '@/di-symbols.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 { MetaService } from '@/core/MetaService.js';
import { UtilityService } from '@/core/UtilityService.js'; import { UtilityService } from '@/core/UtilityService.js';
import { IdentifiableError } from '@/misc/identifiable-error.js'; import { IdentifiableError } from '@/misc/identifiable-error.js';
@ -275,7 +275,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
if (renote == null) { if (renote == null) {
throw new ApiError(meta.errors.noSuchRenoteTarget); throw new ApiError(meta.errors.noSuchRenoteTarget);
} else if (isPureRenote(renote)) { } else if (isRenote(renote) && !isQuote(renote)) {
throw new ApiError(meta.errors.cannotReRenote); throw new ApiError(meta.errors.cannotReRenote);
} }
@ -321,7 +321,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
if (reply == null) { if (reply == null) {
throw new ApiError(meta.errors.noSuchReplyTarget); throw new ApiError(meta.errors.noSuchReplyTarget);
} else if (isPureRenote(reply)) { } else if (isRenote(reply) && !isQuote(reply)) {
throw new ApiError(meta.errors.cannotReplyToPureRenote); throw new ApiError(meta.errors.cannotReplyToPureRenote);
} else if (!await this.noteEntityService.isVisibleForMe(reply, me.id)) { } else if (!await this.noteEntityService.isVisibleForMe(reply, me.id)) {
throw new ApiError(meta.errors.cannotReplyToInvisibleNote); throw new ApiError(meta.errors.cannotReplyToInvisibleNote);

View File

@ -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);
});
});
});

View File

@ -0,0 +1,88 @@
/*
* SPDX-FileCopyrightText: syuilo and misskey-project
* SPDX-License-Identifier: AGPL-3.0-only
*/
import { isQuote, isRenote } from '@/misc/is-renote.js';
import { MiNote } from '@/models/Note.js';
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,
};
describe('misc:is-renote', () => {
test('note without renoteId should not be Renote', () => {
expect(isRenote(base)).toBe(false);
});
test('note with renoteId should be Renote and not be Quote', () => {
const note: MiNote = { ...base, renoteId: 'some-renote-id' };
expect(isRenote(note)).toBe(true);
expect(isQuote(note as any)).toBe(false);
});
test('note with renoteId and text should be Quote', () => {
const note: MiNote = { ...base, renoteId: 'some-renote-id', text: 'some-text' };
expect(isRenote(note)).toBe(true);
expect(isQuote(note as any)).toBe(true);
});
test('note with renoteId and cw should be Quote', () => {
const note: MiNote = { ...base, renoteId: 'some-renote-id', cw: 'some-cw' };
expect(isRenote(note)).toBe(true);
expect(isQuote(note as any)).toBe(true);
});
test('note with renoteId and replyId should be Quote', () => {
const note: MiNote = { ...base, renoteId: 'some-renote-id', replyId: 'some-reply-id' };
expect(isRenote(note)).toBe(true);
expect(isQuote(note as any)).toBe(true);
});
test('note with renoteId and poll should be Quote', () => {
const note: MiNote = { ...base, renoteId: 'some-renote-id', hasPoll: true };
expect(isRenote(note)).toBe(true);
expect(isQuote(note as any)).toBe(true);
});
test('note with renoteId and non-empty fileIds should be Quote', () => {
const note: MiNote = { ...base, renoteId: 'some-renote-id', fileIds: ['some-file-id'] };
expect(isRenote(note)).toBe(true);
expect(isQuote(note as any)).toBe(true);
});
});