Skip to content

Commit 8c5d9a6

Browse files
zyoshokasyuilo
andauthored
fix(backend): incorrect logic for determining whether Quote or not (misskey-dev#13700)
* fix(backend): incorrect logic for determining whether Quote or not * Update CHANGELOG.md --------- Co-authored-by: syuilo <4439005+syuilo@users.noreply.github.com>
1 parent 7cf0c18 commit 8c5d9a6

11 files changed

+296
-43
lines changed

CHANGELOG.md

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

packages/backend/src/core/FanoutTimelineEndpointService.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import type { NotesRepository } from '@/models/_.js';
1313
import { NoteEntityService } from '@/core/entities/NoteEntityService.js';
1414
import { FanoutTimelineName, FanoutTimelineService } from '@/core/FanoutTimelineService.js';
1515
import { isUserRelated } from '@/misc/is-user-related.js';
16-
import { isPureRenote } from '@/misc/is-pure-renote.js';
16+
import { isQuote, isRenote } from '@/misc/is-renote.js';
1717
import { CacheService } from '@/core/CacheService.js';
1818
import { isReply } from '@/misc/is-reply.js';
1919
import { isInstanceMuted } from '@/misc/is-instance-muted.js';
@@ -95,7 +95,7 @@ export class FanoutTimelineEndpointService {
9595

9696
if (ps.excludePureRenotes) {
9797
const parentFilter = filter;
98-
filter = (note) => !isPureRenote(note) && parentFilter(note);
98+
filter = (note) => (!isRenote(note) || isQuote(note)) && parentFilter(note);
9999
}
100100

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

122122
return parentFilter(note);

packages/backend/src/core/NoteCreateService.ts

+17-6
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ export class NoteCreateService implements OnApplicationShutdown {
306306
}
307307

308308
// Check blocking
309-
if (data.renote && !this.isQuote(data)) {
309+
if (this.isRenote(data) && !this.isQuote(data)) {
310310
if (data.renote.userHost === null) {
311311
if (data.renote.userId !== user.id) {
312312
const blocked = await this.userBlockingService.checkBlocked(data.renote.userId, user.id);
@@ -641,7 +641,7 @@ export class NoteCreateService implements OnApplicationShutdown {
641641
}
642642

643643
// If it is renote
644-
if (data.renote) {
644+
if (this.isRenote(data)) {
645645
const type = this.isQuote(data) ? 'quote' : 'renote';
646646

647647
// Notify
@@ -725,9 +725,20 @@ export class NoteCreateService implements OnApplicationShutdown {
725725
}
726726

727727
@bindThis
728-
private isQuote(note: Option): note is Option & { renote: MiNote } {
729-
// sync with misc/is-quote.ts
730-
return !!note.renote && (!!note.text || !!note.cw || (!!note.files && !!note.files.length) || !!note.poll);
728+
private isRenote(note: Option): note is Option & { renote: MiNote } {
729+
return note.renote != null;
730+
}
731+
732+
@bindThis
733+
private isQuote(note: Option & { renote: MiNote }): note is Option & { renote: MiNote } & (
734+
{ text: string } | { cw: string } | { reply: MiNote } | { poll: IPoll } | { files: MiDriveFile[] }
735+
) {
736+
// NOTE: SYNC WITH misc/is-quote.ts
737+
return note.text != null ||
738+
note.reply != null ||
739+
note.cw != null ||
740+
note.poll != null ||
741+
(note.files != null && note.files.length > 0);
731742
}
732743

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

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

packages/backend/src/core/NoteDeleteService.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import { bindThis } from '@/decorators.js';
2424
import { MetaService } from '@/core/MetaService.js';
2525
import { SearchService } from '@/core/SearchService.js';
2626
import { ModerationLogService } from '@/core/ModerationLogService.js';
27-
import { isPureRenote } from '@/misc/is-pure-renote.js';
27+
import { isQuote, isRenote } from '@/misc/is-renote.js';
2828

2929
@Injectable()
3030
export class NoteDeleteService {
@@ -79,7 +79,7 @@ export class NoteDeleteService {
7979
let renote: MiNote | null = null;
8080

8181
// if deleted note is renote
82-
if (isPureRenote(note)) {
82+
if (isRenote(note) && !isQuote(note)) {
8383
renote = await this.notesRepository.findOneBy({
8484
id: note.renoteId,
8585
});

packages/backend/src/misc/is-pure-renote.ts

-15
This file was deleted.

packages/backend/src/misc/is-quote.ts

-12
This file was deleted.
+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
* SPDX-FileCopyrightText: syuilo and misskey-project
3+
* SPDX-License-Identifier: AGPL-3.0-only
4+
*/
5+
6+
import type { MiNote } from '@/models/Note.js';
7+
8+
type Renote =
9+
MiNote & {
10+
renoteId: NonNullable<MiNote['renoteId']>
11+
};
12+
13+
type Quote =
14+
Renote & ({
15+
text: NonNullable<MiNote['text']>
16+
} | {
17+
cw: NonNullable<MiNote['cw']>
18+
} | {
19+
replyId: NonNullable<MiNote['replyId']>
20+
reply: NonNullable<MiNote['reply']>
21+
} | {
22+
hasPoll: true
23+
});
24+
25+
export function isRenote(note: MiNote): note is Renote {
26+
return note.renoteId != null;
27+
}
28+
29+
export function isQuote(note: Renote): note is Quote {
30+
// NOTE: SYNC WITH NoteCreateService.isQuote
31+
return note.text != null ||
32+
note.cw != null ||
33+
note.replyId != null ||
34+
note.hasPoll ||
35+
note.fileIds.length > 0;
36+
}

packages/backend/src/server/ActivityPubServerService.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import { UtilityService } from '@/core/UtilityService.js';
2828
import { UserEntityService } from '@/core/entities/UserEntityService.js';
2929
import { bindThis } from '@/decorators.js';
3030
import { IActivity } from '@/core/activitypub/type.js';
31-
import { isPureRenote } from '@/misc/is-pure-renote.js';
31+
import { isQuote, isRenote } from '@/misc/is-renote.js';
3232
import type { FastifyInstance, FastifyRequest, FastifyReply, FastifyPluginOptions, FastifyBodyParser } from 'fastify';
3333
import type { FindOptionsWhere } from 'typeorm';
3434

@@ -91,7 +91,7 @@ export class ActivityPubServerService {
9191
*/
9292
@bindThis
9393
private async packActivity(note: MiNote): Promise<any> {
94-
if (isPureRenote(note)) {
94+
if (isRenote(note) && !isQuote(note)) {
9595
const renote = await this.notesRepository.findOneByOrFail({ id: note.renoteId });
9696
return this.apRendererService.renderAnnounce(renote.uri ? renote.uri : `${this.config.url}/notes/${renote.id}`, note);
9797
}

packages/backend/src/server/api/endpoints/notes/create.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { Endpoint } from '@/server/api/endpoint-base.js';
1616
import { NoteEntityService } from '@/core/entities/NoteEntityService.js';
1717
import { NoteCreateService } from '@/core/NoteCreateService.js';
1818
import { DI } from '@/di-symbols.js';
19-
import { isPureRenote } from '@/misc/is-pure-renote.js';
19+
import { isQuote, isRenote } from '@/misc/is-renote.js';
2020
import { MetaService } from '@/core/MetaService.js';
2121
import { UtilityService } from '@/core/UtilityService.js';
2222
import { IdentifiableError } from '@/misc/identifiable-error.js';
@@ -275,7 +275,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
275275

276276
if (renote == null) {
277277
throw new ApiError(meta.errors.noSuchRenoteTarget);
278-
} else if (isPureRenote(renote)) {
278+
} else if (isRenote(renote) && !isQuote(renote)) {
279279
throw new ApiError(meta.errors.cannotReRenote);
280280
}
281281

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

322322
if (reply == null) {
323323
throw new ApiError(meta.errors.noSuchReplyTarget);
324-
} else if (isPureRenote(reply)) {
324+
} else if (isRenote(reply) && !isQuote(reply)) {
325325
throw new ApiError(meta.errors.cannotReplyToPureRenote);
326326
} else if (!await this.noteEntityService.isVisibleForMe(reply, me.id)) {
327327
throw new ApiError(meta.errors.cannotReplyToInvisibleNote);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
/*
2+
* SPDX-FileCopyrightText: syuilo and misskey-project
3+
* SPDX-License-Identifier: AGPL-3.0-only
4+
*/
5+
6+
import { Test } from '@nestjs/testing';
7+
8+
import { CoreModule } from '@/core/CoreModule.js';
9+
import { NoteCreateService } from '@/core/NoteCreateService.js';
10+
import { GlobalModule } from '@/GlobalModule.js';
11+
import { MiNote } from '@/models/Note.js';
12+
import { IPoll } from '@/models/Poll.js';
13+
import { MiDriveFile } from '@/models/DriveFile.js';
14+
15+
describe('NoteCreateService', () => {
16+
let noteCreateService: NoteCreateService;
17+
18+
beforeAll(async () => {
19+
const app = await Test.createTestingModule({
20+
imports: [GlobalModule, CoreModule],
21+
}).compile();
22+
noteCreateService = app.get<NoteCreateService>(NoteCreateService);
23+
});
24+
25+
describe('is-renote', () => {
26+
const base: MiNote = {
27+
id: 'some-note-id',
28+
replyId: null,
29+
reply: null,
30+
renoteId: null,
31+
renote: null,
32+
threadId: null,
33+
text: null,
34+
name: null,
35+
cw: null,
36+
userId: 'some-user-id',
37+
user: null,
38+
localOnly: false,
39+
reactionAcceptance: null,
40+
renoteCount: 0,
41+
repliesCount: 0,
42+
clippedCount: 0,
43+
reactions: {},
44+
visibility: 'public',
45+
uri: null,
46+
url: null,
47+
fileIds: [],
48+
attachedFileTypes: [],
49+
visibleUserIds: [],
50+
mentions: [],
51+
mentionedRemoteUsers: '',
52+
reactionAndUserPairCache: [],
53+
emojis: [],
54+
tags: [],
55+
hasPoll: false,
56+
channelId: null,
57+
channel: null,
58+
userHost: null,
59+
replyUserId: null,
60+
replyUserHost: null,
61+
renoteUserId: null,
62+
renoteUserHost: null,
63+
};
64+
65+
const poll: IPoll = {
66+
choices: ['kinoko', 'takenoko'],
67+
multiple: false,
68+
expiresAt: null,
69+
};
70+
71+
const file: MiDriveFile = {
72+
id: 'some-file-id',
73+
userId: null,
74+
user: null,
75+
userHost: null,
76+
md5: '',
77+
name: '',
78+
type: '',
79+
size: 0,
80+
comment: null,
81+
blurhash: null,
82+
properties: {},
83+
storedInternal: false,
84+
url: '',
85+
thumbnailUrl: null,
86+
webpublicUrl: null,
87+
webpublicType: null,
88+
accessKey: null,
89+
thumbnailAccessKey: null,
90+
webpublicAccessKey: null,
91+
uri: null,
92+
src: null,
93+
folderId: null,
94+
folder: null,
95+
isSensitive: false,
96+
maybeSensitive: false,
97+
maybePorn: false,
98+
isLink: false,
99+
requestHeaders: null,
100+
requestIp: null,
101+
};
102+
103+
test('note without renote should not be Renote', () => {
104+
const note = { renote: null };
105+
expect(noteCreateService['isRenote'](note)).toBe(false);
106+
});
107+
108+
test('note with renote should be Renote and not be Quote', () => {
109+
const note = { renote: base };
110+
expect(noteCreateService['isRenote'](note)).toBe(true);
111+
expect(noteCreateService['isQuote'](note)).toBe(false);
112+
});
113+
114+
test('note with renote and text should be Quote', () => {
115+
const note = { renote: base, text: 'some-text' };
116+
expect(noteCreateService['isRenote'](note)).toBe(true);
117+
expect(noteCreateService['isQuote'](note)).toBe(true);
118+
});
119+
120+
test('note with renote and cw should be Quote', () => {
121+
const note = { renote: base, cw: 'some-cw' };
122+
expect(noteCreateService['isRenote'](note)).toBe(true);
123+
expect(noteCreateService['isQuote'](note)).toBe(true);
124+
});
125+
126+
test('note with renote and reply should be Quote', () => {
127+
const note = { renote: base, reply: { ...base, id: 'another-note-id' } };
128+
expect(noteCreateService['isRenote'](note)).toBe(true);
129+
expect(noteCreateService['isQuote'](note)).toBe(true);
130+
});
131+
132+
test('note with renote and poll should be Quote', () => {
133+
const note = { renote: base, poll };
134+
expect(noteCreateService['isRenote'](note)).toBe(true);
135+
expect(noteCreateService['isQuote'](note)).toBe(true);
136+
});
137+
138+
test('note with renote and non-empty files should be Quote', () => {
139+
const note = { renote: base, files: [file] };
140+
expect(noteCreateService['isRenote'](note)).toBe(true);
141+
expect(noteCreateService['isQuote'](note)).toBe(true);
142+
});
143+
});
144+
});

0 commit comments

Comments
 (0)