From 4f37ede88607eaa88c02f98e4f1453b0ae97722f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?KMY=EF=BC=88=E9=9B=AA=E3=81=82=E3=81=99=E3=81=8B=EF=BC=89?= Date: Sun, 22 Oct 2023 09:40:20 +0900 Subject: [PATCH] =?UTF-8?q?Fix:=20#162=20=E7=B7=A8=E9=9B=86=EF=BC=8F?= =?UTF-8?q?=E4=BB=96=E3=81=AE=E3=82=B5=E3=83=BC=E3=83=90=E3=83=BC=E3=81=8B?= =?UTF-8?q?=E3=82=89=E3=81=AE=E7=B7=A8=E9=9B=86=E3=81=A7=E3=80=81=E3=83=95?= =?UTF-8?q?=E3=82=A9=E3=83=AD=E3=83=BC=E3=81=97=E3=81=A6=E3=81=84=E3=81=AA?= =?UTF-8?q?=E3=81=84=E7=9B=B8=E6=89=8B=E3=81=8B=E3=82=89=E3=81=AE=E3=83=A1?= =?UTF-8?q?=E3=83=B3=E3=82=B7=E3=83=A7=E3=83=B3=E3=81=AB=E9=96=A2=E3=81=99?= =?UTF-8?q?=E3=82=8BNG=E3=83=AF=E3=83=BC=E3=83=89=E3=81=AB=E5=AF=BE?= =?UTF-8?q?=E5=BF=9C=20(#163)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix: #162 フォローしていないアカウントへのメンションのNGワードが編集で有効にならない問題 * Fix: 他のサーバーから来た編集についても適用 --- .../process_status_update_service.rb | 15 ++ app/services/post_status_service.rb | 2 +- app/services/update_status_service.rb | 15 +- .../process_status_update_service_spec.rb | 173 +++++++++++++++++- spec/services/post_status_service_spec.rb | 20 ++ spec/services/update_status_service_spec.rb | 99 ++++++++++ 6 files changed, 315 insertions(+), 9 deletions(-) diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index cbef13fbed..765a59c890 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -5,6 +5,8 @@ class ActivityPub::ProcessStatusUpdateService < BaseService include Redisable include Lockable + class AbortError < ::StandardError; end + def call(status, activity_json, object_json, request_id: nil) raise ArgumentError, 'Status has unsaved changes' if status.changed? @@ -30,6 +32,9 @@ class ActivityPub::ProcessStatusUpdateService < BaseService handle_implicit_update! end + @status + rescue AbortError + @status.reload @status end @@ -46,6 +51,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService update_poll! update_immediate_attributes! update_metadata! + validate_status_mentions! create_edits! end @@ -160,6 +166,15 @@ class ActivityPub::ProcessStatusUpdateService < BaseService !Admin::NgWord.reject?("#{@status_parser.spoiler_text}\n#{@status_parser.text}") && !Admin::NgWord.hashtag_reject?(@raw_tags.size) end + def validate_status_mentions! + raise AbortError if mention_to_stranger? && Admin::NgWord.stranger_mention_reject?("#{@status.spoiler_text}\n#{@status.text}") + end + + def mention_to_stranger? + @status.mentions.map(&:account).to_a.any? { |mentioned_account| mentioned_account.id != @status.account.id && !mentioned_account.following?(@status.account) } || + (@status.thread.present? && @status.thread.account.id != @status.account.id && !@status.thread.account.following?(@status.account)) + end + def update_immediate_attributes! @status.text = @status_parser.text || '' @status.spoiler_text = @status_parser.spoiler_text || '' diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index 5410323dbb..0e3bd8805d 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -214,7 +214,7 @@ class PostStatusService < BaseService end def mention_to_stranger? - @status.mentions.map(&:account).to_a.any? { |mentioned_account| mentioned_account.id != @account && !mentioned_account.following?(@account) } || + @status.mentions.map(&:account).to_a.any? { |mentioned_account| mentioned_account.id != @account.id && !mentioned_account.following?(@account) } || (@in_reply_to && @in_reply_to.account.id != @account.id && !@in_reply_to.account.following?(@account)) end diff --git a/app/services/update_status_service.rb b/app/services/update_status_service.rb index 0a5de6b907..a9733b0658 100644 --- a/app/services/update_status_service.rb +++ b/app/services/update_status_service.rb @@ -35,10 +35,13 @@ class UpdateStatusService < BaseService update_poll! if @options.key?(:poll) update_immediate_attributes! create_edit! unless @options[:no_history] + + reset_preview_card! + process_mentions_service.call(@status) + validate_status_mentions! end queue_poll_notifications! - reset_preview_card! update_metadata! update_references! broadcast_updates! @@ -81,6 +84,15 @@ class UpdateStatusService < BaseService raise Mastodon::ValidationError, I18n.t('statuses.too_many_hashtags') if Admin::NgWord.hashtag_reject_with_extractor?(@options[:text]) end + def validate_status_mentions! + raise Mastodon::ValidationError, I18n.t('statuses.contains_ng_words') if mention_to_stranger? && Setting.stranger_mention_from_local_ng && Admin::NgWord.stranger_mention_reject?("#{@options[:spoiler_text]}\n#{@options[:text]}") + end + + def mention_to_stranger? + @status.mentions.map(&:account).to_a.any? { |mentioned_account| mentioned_account.id != @status.account.id && !mentioned_account.following?(@status.account) } || + (@status.thread.present? && @status.thread.account.id != @status.account.id && !@status.thread.account.following?(@status.account)) + end + def validate_media! return [] if @options[:media_ids].blank? || !@options[:media_ids].is_a?(Enumerable) @@ -167,7 +179,6 @@ class UpdateStatusService < BaseService def update_metadata! ProcessHashtagsService.new.call(@status) - process_mentions_service.call(@status) @status.update(limited_scope: :circle) if process_mentions_service.mentions? end diff --git a/spec/services/activitypub/process_status_update_service_spec.rb b/spec/services/activitypub/process_status_update_service_spec.rb index 9d91f31cc5..0612c94d8c 100644 --- a/spec/services/activitypub/process_status_update_service_spec.rb +++ b/spec/services/activitypub/process_status_update_service_spec.rb @@ -9,19 +9,24 @@ end RSpec.describe ActivityPub::ProcessStatusUpdateService, type: :service do subject { described_class.new } - let!(:status) { Fabricate(:status, text: 'Hello world', account: Fabricate(:account, domain: 'example.com')) } + let(:thread) { nil } + let!(:status) { Fabricate(:status, text: 'Hello world', account: Fabricate(:account, domain: 'example.com'), thread: thread) } + let(:json_tags) do + [ + { type: 'Hashtag', name: 'hoge' }, + { type: 'Mention', href: ActivityPub::TagManager.instance.uri_for(alice) }, + ] + end + let(:content) { 'Hello universe' } let(:payload) do { '@context': 'https://www.w3.org/ns/activitystreams', id: 'foo', type: 'Note', summary: 'Show more', - content: 'Hello universe', + content: content, updated: '2021-09-08T22:39:25Z', - tag: [ - { type: 'Hashtag', name: 'hoge' }, - { type: 'Mention', href: ActivityPub::TagManager.instance.uri_for(alice) }, - ], + tag: json_tags, } end let(:json) { Oj.load(Oj.dump(payload)) } @@ -462,5 +467,161 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService, type: :service do subject.call(status, json, json) expect(status.reload.edited_at.to_s).to eq '2021-09-08 22:39:25 UTC' end + + describe 'ng word is set' do + let(:json_tags) { [] } + + context 'when hit ng words' do + let(:content) { 'ng word test' } + + it 'update status' do + Form::AdminSettings.new(ng_words: 'test').save + + subject.call(status, json, json) + expect(status.reload.text).to_not eq content + end + end + + context 'when not hit ng words' do + let(:content) { 'ng word aiueo' } + + it 'update status' do + Form::AdminSettings.new(ng_words: 'test').save + + subject.call(status, json, json) + expect(status.reload.text).to eq content + end + end + + context 'when hit ng words for mention' do + let(:json_tags) do + [ + { type: 'Mention', href: ActivityPub::TagManager.instance.uri_for(alice) }, + ] + end + let(:content) { 'ng word test' } + + it 'update status' do + Form::AdminSettings.new(ng_words_for_stranger_mention: 'test', stranger_mention_from_local_ng: '1').save + + subject.call(status, json, json) + expect(status.reload.text).to_not eq content + expect(status.mentioned_accounts.pluck(:id)).to_not include alice.id + end + end + + context 'when hit ng words for mention but local posts are not checked' do + let(:json_tags) do + [ + { type: 'Mention', href: ActivityPub::TagManager.instance.uri_for(alice) }, + ] + end + let(:content) { 'ng word test' } + + it 'update status' do + Form::AdminSettings.new(ng_words_for_stranger_mention: 'test', stranger_mention_from_local_ng: '0').save + + subject.call(status, json, json) + expect(status.reload.text).to_not eq content + expect(status.mentioned_accounts.pluck(:id)).to_not include alice.id + end + end + + context 'when hit ng words for mention to follower' do + let(:json_tags) do + [ + { type: 'Mention', href: ActivityPub::TagManager.instance.uri_for(alice) }, + ] + end + let(:content) { 'ng word test' } + + before do + alice.follow!(status.account) + end + + it 'update status' do + Form::AdminSettings.new(ng_words_for_stranger_mention: 'test').save + + subject.call(status, json, json) + expect(status.reload.text).to eq content + expect(status.mentioned_accounts.pluck(:id)).to include alice.id + end + end + + context 'when hit ng words for reply' do + let(:json_tags) do + [ + { type: 'Mention', href: ActivityPub::TagManager.instance.uri_for(alice) }, + ] + end + let(:content) { 'ng word test' } + let(:thread) { Fabricate(:status, account: alice) } + + it 'update status' do + Form::AdminSettings.new(ng_words_for_stranger_mention: 'test').save + + subject.call(status, json, json) + expect(status.reload.text).to_not eq content + expect(status.mentioned_accounts.pluck(:id)).to_not include alice.id + end + end + + context 'when hit ng words for reply to follower' do + let(:json_tags) do + [ + { type: 'Mention', href: ActivityPub::TagManager.instance.uri_for(alice) }, + ] + end + let(:content) { 'ng word test' } + let(:thread) { Fabricate(:status, account: alice) } + + before do + alice.follow!(status.account) + end + + it 'update status' do + Form::AdminSettings.new(ng_words_for_stranger_mention: 'test').save + + subject.call(status, json, json) + expect(status.reload.text).to eq content + expect(status.mentioned_accounts.pluck(:id)).to include alice.id + end + end + + context 'when using hashtag under limit' do + let(:json_tags) do + [ + { type: 'Hashtag', name: 'a' }, + { type: 'Hashtag', name: 'b' }, + ] + end + let(:content) { 'ohagi is good' } + + it 'update status' do + Form::AdminSettings.new(post_hash_tags_max: 2).save + + subject.call(status, json, json) + expect(status.reload.text).to eq content + end + end + + context 'when using hashtag over limit' do + let(:json_tags) do + [ + { type: 'Hashtag', name: 'a' }, + { type: 'Hashtag', name: 'b' }, + { type: 'Hashtag', name: 'c' }, + ] + end + let(:content) { 'ohagi is good' } + + it 'update status' do + Form::AdminSettings.new(post_hash_tags_max: 2).save + + subject.call(status, json, json) + expect(status.reload.text).to_not eq content + end + end + end end end diff --git a/spec/services/post_status_service_spec.rb b/spec/services/post_status_service_spec.rb index adf7e33d8e..a1a242a517 100644 --- a/spec/services/post_status_service_spec.rb +++ b/spec/services/post_status_service_spec.rb @@ -510,6 +510,26 @@ RSpec.describe PostStatusService, type: :service do expect(status).to be_persisted expect(status.text).to eq text end + + it 'using hashtag under limit' do + account = Fabricate(:account) + text = '#a #b' + Form::AdminSettings.new(post_hash_tags_max: 2).save + + status = subject.call(account, text: text) + + expect(status).to be_persisted + expect(status.tags.count).to eq 2 + expect(status.text).to eq text + end + + it 'using hashtag over limit' do + account = Fabricate(:account) + text = '#a #b #c' + Form::AdminSettings.new(post_hash_tags_max: 2).save + + expect { subject.call(account, text: text) }.to raise_error Mastodon::ValidationError + end end def create_status_with_options(**options) diff --git a/spec/services/update_status_service_spec.rb b/spec/services/update_status_service_spec.rb index 288466bdeb..d2f3d42a0f 100644 --- a/spec/services/update_status_service_spec.rb +++ b/spec/services/update_status_service_spec.rb @@ -218,4 +218,103 @@ RSpec.describe UpdateStatusService, type: :service do subject.call(status, status.account_id, text: 'Bar') expect(ActivityPub::DistributionWorker).to have_received(:perform_async) end + + describe 'ng word is set' do + let(:account) { Fabricate(:account) } + let(:status) { PostStatusService.new.call(account, text: 'ohagi') } + + it 'hit ng words' do + text = 'ng word test' + Form::AdminSettings.new(ng_words: 'test').save + + expect { subject.call(status, status.account_id, text: text) }.to raise_error(Mastodon::ValidationError) + end + + it 'not hit ng words' do + text = 'ng word aiueo' + Form::AdminSettings.new(ng_words: 'test').save + + status2 = subject.call(status, status.account_id, text: text) + + expect(status2).to be_persisted + expect(status2.text).to eq text + end + + it 'hit ng words for mention' do + Fabricate(:account, username: 'ohagi', domain: nil) + text = 'ng word test @ohagi' + Form::AdminSettings.new(ng_words_for_stranger_mention: 'test', stranger_mention_from_local_ng: '1').save + + expect { subject.call(status, status.account_id, text: text) }.to raise_error(Mastodon::ValidationError) + expect(status.reload.text).to_not eq text + expect(status.mentioned_accounts.pluck(:username)).to_not include 'ohagi' + end + + it 'hit ng words for mention but local posts are not checked' do + Fabricate(:account, username: 'ohagi', domain: nil) + text = 'ng word test @ohagi' + Form::AdminSettings.new(ng_words_for_stranger_mention: 'test', stranger_mention_from_local_ng: '0').save + + status2 = subject.call(status, status.account_id, text: text) + + expect(status2).to be_persisted + expect(status2.text).to eq text + end + + it 'hit ng words for mention to follower' do + mentioned = Fabricate(:account, username: 'ohagi', domain: nil) + mentioned.follow!(account) + text = 'ng word test @ohagi' + Form::AdminSettings.new(ng_words_for_stranger_mention: 'test', stranger_mention_from_local_ng: '1').save + + status2 = subject.call(status, status.account_id, text: text) + + expect(status2).to be_persisted + expect(status2.text).to eq text + end + + it 'hit ng words for reply' do + text = 'ng word test' + Form::AdminSettings.new(ng_words_for_stranger_mention: 'test', stranger_mention_from_local_ng: '1').save + + status = PostStatusService.new.call(account, text: 'hello', thread: Fabricate(:status)) + + expect { subject.call(status, status.account_id, text: text) }.to raise_error(Mastodon::ValidationError) + expect(status.reload.text).to_not eq text + end + + it 'hit ng words for reply to follower' do + mentioned = Fabricate(:account, username: 'ohagi', domain: nil) + mentioned.follow!(account) + text = 'ng word test' + Form::AdminSettings.new(ng_words_for_stranger_mention: 'test', stranger_mention_from_local_ng: '1').save + + status = PostStatusService.new.call(account, text: 'hello', thread: Fabricate(:status, account: mentioned)) + + status = subject.call(status, status.account_id, text: text) + + expect(status).to be_persisted + expect(status.text).to eq text + end + + it 'using hashtag under limit' do + text = '#a #b' + Form::AdminSettings.new(post_hash_tags_max: 2).save + + subject.call(status, status.account_id, text: text) + + expect(status.reload.tags.count).to eq 2 + expect(status.text).to eq text + end + + it 'using hashtag over limit' do + text = '#a #b #c' + Form::AdminSettings.new(post_hash_tags_max: 2).save + + expect { subject.call(status, status.account_id, text: text) }.to raise_error Mastodon::ValidationError + + expect(status.reload.tags.count).to eq 0 + expect(status.text).to_not eq text + end + end end