From b8fdffe8246f271b4e49c8599229bcd576aeb6cf Mon Sep 17 00:00:00 2001 From: Jeong Arm Date: Wed, 2 Oct 2024 17:08:02 +0900 Subject: [PATCH] Ignore error if mentioned account was not processable (#29215) Co-authored-by: Claire --- app/lib/activitypub/activity/create.rb | 10 ++++++ app/workers/mention_resolve_worker.rb | 37 +++++++++++++++++++ spec/lib/activitypub/activity/create_spec.rb | 37 +++++++++++++++++++ spec/workers/mention_resolve_worker_spec.rb | 38 ++++++++++++++++++++ 4 files changed, 122 insertions(+) create mode 100644 app/workers/mention_resolve_worker.rb create mode 100644 spec/workers/mention_resolve_worker_spec.rb diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 09a8caf1f..928803cd6 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -42,6 +42,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity def process_status @tags = [] @mentions = [] + @unresolved_mentions = [] @silenced_account_ids = [] @params = {} @@ -55,6 +56,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity end resolve_thread(@status) + resolve_unresolved_mentions(@status) fetch_replies(@status) distribute forward_for_reply @@ -197,6 +199,8 @@ class ActivityPub::Activity::Create < ActivityPub::Activity return if account.nil? @mentions << Mention.new(account: account, silent: false) + rescue Mastodon::UnexpectedResponseError, HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError + @unresolved_mentions << tag['href'] end def process_emoji(tag) @@ -301,6 +305,12 @@ class ActivityPub::Activity::Create < ActivityPub::Activity ThreadResolveWorker.perform_async(status.id, in_reply_to_uri, { 'request_id' => @options[:request_id] }) end + def resolve_unresolved_mentions(status) + @unresolved_mentions.uniq.each do |uri| + MentionResolveWorker.perform_in(rand(30...600).seconds, status.id, uri, { 'request_id' => @options[:request_id] }) + end + end + def fetch_replies(status) collection = @object['replies'] return if collection.blank? diff --git a/app/workers/mention_resolve_worker.rb b/app/workers/mention_resolve_worker.rb new file mode 100644 index 000000000..72dcd9633 --- /dev/null +++ b/app/workers/mention_resolve_worker.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +class MentionResolveWorker + include Sidekiq::Worker + include ExponentialBackoff + include JsonLdHelper + + sidekiq_options queue: 'pull', retry: 7 + + def perform(status_id, uri, options = {}) + status = Status.find_by(id: status_id) + return if status.nil? + + account = account_from_uri(uri) + account = ActivityPub::FetchRemoteAccountService.new.call(uri, request_id: options[:request_id]) if account.nil? + + return if account.nil? + + status.mentions.create!(account: account, silent: false) + rescue ActiveRecord::RecordNotFound + # Do nothing + rescue Mastodon::UnexpectedResponseError => e + response = e.response + + if response_error_unsalvageable?(response) + # Give up + else + raise e + end + end + + private + + def account_from_uri(uri) + ActivityPub::TagManager.instance.uri_to_resource(uri, Account) + end +end diff --git a/spec/lib/activitypub/activity/create_spec.rb b/spec/lib/activitypub/activity/create_spec.rb index 0986ae171..bdc8fd9d5 100644 --- a/spec/lib/activitypub/activity/create_spec.rb +++ b/spec/lib/activitypub/activity/create_spec.rb @@ -63,6 +63,24 @@ RSpec.describe ActivityPub::Activity::Create do } end + let(:invalid_mention_json) do + { + id: [ActivityPub::TagManager.instance.uri_for(sender), 'post2'].join('/'), + type: 'Note', + to: [ + 'https://www.w3.org/ns/activitystreams#Public', + ActivityPub::TagManager.instance.uri_for(follower), + ], + content: '@bob lorem ipsum', + published: 1.hour.ago.utc.iso8601, + updated: 1.hour.ago.utc.iso8601, + tag: { + type: 'Mention', + href: 'http://notexisting.dontexistingtld/actor', + }, + } + end + def activity_for_object(json) { '@context': 'https://www.w3.org/ns/activitystreams', @@ -117,6 +135,25 @@ RSpec.describe ActivityPub::Activity::Create do # Creates two notifications expect(Notification.count).to eq 2 end + + it 'ignores unprocessable mention', :aggregate_failures do + stub_request(:get, invalid_mention_json[:tag][:href]).to_raise(HTTP::ConnectionError) + # When receiving the post that contains an invalid mention… + described_class.new(activity_for_object(invalid_mention_json), sender, delivery: true).perform + + # NOTE: Refering explicitly to the workers is a bit awkward + DistributionWorker.drain + FeedInsertWorker.drain + + # …it creates a status + status = Status.find_by(uri: invalid_mention_json[:id]) + + # Check the process did not crash + expect(status.nil?).to be false + + # It has queued a mention resolve job + expect(MentionResolveWorker).to have_enqueued_sidekiq_job(status.id, invalid_mention_json[:tag][:href], anything) + end end describe '#perform' do diff --git a/spec/workers/mention_resolve_worker_spec.rb b/spec/workers/mention_resolve_worker_spec.rb new file mode 100644 index 000000000..5e23876b4 --- /dev/null +++ b/spec/workers/mention_resolve_worker_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe MentionResolveWorker do + let(:status_id) { -42 } + let(:uri) { 'https://example.com/users/unknown' } + + describe '#perform' do + subject { described_class.new.perform(status_id, uri, {}) } + + context 'with a non-existent status' do + it 'returns nil' do + expect(subject).to be_nil + end + end + + context 'with a valid user' do + let(:status) { Fabricate(:status) } + let(:status_id) { status.id } + + let(:service_double) { instance_double(ActivityPub::FetchRemoteAccountService) } + + before do + allow(ActivityPub::FetchRemoteAccountService).to receive(:new).and_return(service_double) + + allow(service_double).to receive(:call).with(uri, anything) { Fabricate(:account, domain: 'example.com', uri: uri) } + end + + it 'resolves the account and adds a new mention', :aggregate_failures do + expect { subject } + .to change { status.reload.mentions }.from([]).to(a_collection_including(having_attributes(account: having_attributes(uri: uri), silent: false))) + + expect(service_double).to have_received(:call).once + end + end + end +end