From 322d74fc2aa9db2d05e5bad944661c6edf1660e1 Mon Sep 17 00:00:00 2001 From: ThibG Date: Fri, 17 Jul 2020 07:07:54 +0200 Subject: Fix boosted toots from blocked account not being retroactively removed from TL (#14339) * Fix boosted toots from blocked account not being retroactively removed from TL Fixes #14301 * Add test for clear_from_timeline --- app/lib/feed_manager.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'app/lib') diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index 53ff31f5e..96fa6cfc0 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -124,9 +124,15 @@ class FeedManager end def clear_from_timeline(account, target_account) + # Clear from timeline all statuses from or mentionning target_account timeline_key = key(:home, account.id) timeline_status_ids = redis.zrange(timeline_key, 0, -1) - target_statuses = Status.where(id: timeline_status_ids, account: target_account) + statuses = Status.where(id: timeline_status_ids).select(:id, :reblog_of_id, :account_id).to_a + reblogged_ids = Status.where(id: statuses.map(&:reblog_of_id).compact, account: target_account).pluck(:id) + with_mentions_ids = Mention.active.where(status_id: statuses.flat_map { |s| [s.id, s.reblog_of_id] }.compact, account: target_account).pluck(:status_id) + target_statuses = statuses.filter do |status| + status.account_id == target_account.id || reblogged_ids.include?(status.reblog_of_id) || with_mentions_ids.include?(status.id) || with_mentions_ids.include?(status.reblog_of_id) + end target_statuses.each do |status| unpush_from_home(account, status) -- cgit From 7540e235a2b387ca08cf57f8942d1b190d242808 Mon Sep 17 00:00:00 2001 From: Takeshi Umeda Date: Mon, 20 Jul 2020 05:28:27 +0900 Subject: Fix movie width and frame_rate returning nil (#14357) * Fix movie width and frame_rate returning nil * Add StreamValidationError and raise * Fix code style --- app/lib/exceptions.rb | 1 + app/models/concerns/remotable.rb | 2 +- app/models/media_attachment.rb | 1 + app/services/update_account_service.rb | 2 +- 4 files changed, 4 insertions(+), 2 deletions(-) (limited to 'app/lib') diff --git a/app/lib/exceptions.rb b/app/lib/exceptions.rb index 3362576b0..7c8e77871 100644 --- a/app/lib/exceptions.rb +++ b/app/lib/exceptions.rb @@ -7,6 +7,7 @@ module Mastodon class HostValidationError < ValidationError; end class LengthValidationError < ValidationError; end class DimensionsValidationError < ValidationError; end + class StreamValidationError < ValidationError; end class RaceConditionError < Error; end class RateLimitExceededError < Error; end diff --git a/app/models/concerns/remotable.rb b/app/models/concerns/remotable.rb index c6d0c7f1f..56b9c0164 100644 --- a/app/models/concerns/remotable.rb +++ b/app/models/concerns/remotable.rb @@ -29,7 +29,7 @@ module Remotable rescue Mastodon::UnexpectedResponseError, HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError => e Rails.logger.debug "Error fetching remote #{attachment_name}: #{e}" raise e unless suppress_errors - rescue Paperclip::Errors::NotIdentifiedByImageMagickError, Addressable::URI::InvalidURIError, Mastodon::HostValidationError, Mastodon::LengthValidationError, Paperclip::Error, Mastodon::DimensionsValidationError => e + rescue Paperclip::Errors::NotIdentifiedByImageMagickError, Addressable::URI::InvalidURIError, Mastodon::HostValidationError, Mastodon::LengthValidationError, Paperclip::Error, Mastodon::DimensionsValidationError, Mastodon::StreamValidationError => e Rails.logger.debug "Error fetching remote #{attachment_name}: #{e}" end diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index 3f2e0ceb1..3d93ec75b 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -336,6 +336,7 @@ class MediaAttachment < ApplicationRecord return unless movie.valid? + raise Mastodon::StreamValidationError, 'Video has no video stream' if movie.width.nil? || movie.frame_rate.nil? raise Mastodon::DimensionsValidationError, "#{movie.width}x#{movie.height} videos are not supported" if movie.width * movie.height > MAX_VIDEO_MATRIX_LIMIT raise Mastodon::DimensionsValidationError, "#{movie.frame_rate.to_i}fps videos are not supported" if movie.frame_rate > MAX_VIDEO_FRAME_RATE end diff --git a/app/services/update_account_service.rb b/app/services/update_account_service.rb index 4172d5774..666db5c71 100644 --- a/app/services/update_account_service.rb +++ b/app/services/update_account_service.rb @@ -12,7 +12,7 @@ class UpdateAccountService < BaseService check_links(account) process_hashtags(account) end - rescue Mastodon::DimensionsValidationError => de + rescue Mastodon::DimensionsValidationError, Mastodon::StreamValidationError => de account.errors.add(:avatar, de.message) false end -- cgit From fcb3f259e5a36dc4ac5300aa715d583f3a577c2b Mon Sep 17 00:00:00 2001 From: Takeshi Umeda Date: Mon, 20 Jul 2020 18:25:26 +0900 Subject: Fix to add RedisLock to handle Announce activity (#14365) --- app/lib/activitypub/activity/announce.rb | 39 ++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 14 deletions(-) (limited to 'app/lib') diff --git a/app/lib/activitypub/activity/announce.rb b/app/lib/activitypub/activity/announce.rb index 34c646668..9e108985a 100644 --- a/app/lib/activitypub/activity/announce.rb +++ b/app/lib/activitypub/activity/announce.rb @@ -4,25 +4,32 @@ class ActivityPub::Activity::Announce < ActivityPub::Activity def perform return reject_payload! if delete_arrived_first?(@json['id']) || !related_to_local_activity? - original_status = status_from_object + RedisLock.acquire(lock_options) do |lock| + if lock.acquired? + original_status = status_from_object - return reject_payload! if original_status.nil? || !announceable?(original_status) + return reject_payload! if original_status.nil? || !announceable?(original_status) - status = Status.find_by(account: @account, reblog: original_status) + @status = Status.find_by(account: @account, reblog: original_status) - return status unless status.nil? + return @status unless @status.nil? - status = Status.create!( - account: @account, - reblog: original_status, - uri: @json['id'], - created_at: @json['published'], - override_timestamps: @options[:override_timestamps], - visibility: visibility_from_audience - ) + @status = Status.create!( + account: @account, + reblog: original_status, + uri: @json['id'], + created_at: @json['published'], + override_timestamps: @options[:override_timestamps], + visibility: visibility_from_audience + ) - distribute(status) - status + distribute(@status) + else + raise Mastodon::RaceConditionError + end + end + + @status end private @@ -54,4 +61,8 @@ class ActivityPub::Activity::Announce < ActivityPub::Activity def reblog_of_local_status? status_from_uri(object_uri)&.account&.local? end + + def lock_options + { redis: Redis.current, key: "announce:#{@object['id']}" } + end end -- cgit From bcf85b5208c936486550da0ce978098840218073 Mon Sep 17 00:00:00 2001 From: ThibG Date: Wed, 22 Jul 2020 11:43:17 +0200 Subject: Dereference object URIs in Create and Update messages (#14359) * Dereference object URIs in Create and Update messages Fixes #14353 Signed-off-by: Thibaut Girka * Refactor, and perform origin check *before* attempting to fetch object Co-authored-by: Fire Demon --- app/lib/activitypub/activity.rb | 28 ++++++++++++++++++++++++++++ app/lib/activitypub/activity/create.rb | 2 ++ app/lib/activitypub/activity/update.rb | 2 ++ 3 files changed, 32 insertions(+) (limited to 'app/lib') diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb index 0ce279d28..ab946470b 100644 --- a/app/lib/activitypub/activity.rb +++ b/app/lib/activitypub/activity.rb @@ -157,6 +157,34 @@ class ActivityPub::Activity fetch_remote_original_status end + def dereference_object! + return unless @object.is_a?(String) + return if invalid_origin?(@object) + + object = fetch_resource(@object, true, signed_fetch_account) + return unless object.present? && object.is_a?(Hash) && supported_context?(object) + + @object = object + end + + def signed_fetch_account + first_mentioned_local_account || first_local_follower + end + + def first_mentioned_local_account + audience = (as_array(@json['to']) + as_array(@json['cc'])).uniq + local_usernames = audience.select { |uri| ActivityPub::TagManager.instance.local_uri?(uri) } + .map { |uri| ActivityPub::TagManager.instance.uri_to_local_id(uri, :username) } + + return if local_usernames.empty? + + Account.local.where(username: local_usernames).first + end + + def first_local_follower + @account.followers.local.first + end + def follow_request_from_object @follow_request ||= FollowRequest.find_by(target_account: @account, uri: object_uri) unless object_uri.nil? end diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index e81452e3c..08dd98e94 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -2,6 +2,8 @@ class ActivityPub::Activity::Create < ActivityPub::Activity def perform + dereference_object! + case @object['type'] when 'EncryptedMessage' create_encrypted_message diff --git a/app/lib/activitypub/activity/update.rb b/app/lib/activitypub/activity/update.rb index 70035325b..018e2df54 100644 --- a/app/lib/activitypub/activity/update.rb +++ b/app/lib/activitypub/activity/update.rb @@ -4,6 +4,8 @@ class ActivityPub::Activity::Update < ActivityPub::Activity SUPPORTED_TYPES = %w(Application Group Organization Person Service).freeze def perform + dereference_object! + if equals_or_includes_any?(@object['type'], SUPPORTED_TYPES) update_account elsif equals_or_includes_any?(@object['type'], %w(Question)) -- cgit From 5d9acc0ce41aa0274fef2dd321a8fad1fb0665ea Mon Sep 17 00:00:00 2001 From: ThibG Date: Wed, 22 Jul 2020 11:45:35 +0200 Subject: Fix not handling Undo on some activity types when they aren't inlined (#14346) * Fix not handling Undo on some activity types when they aren't inlined When receiving an Undo for a non-inlined activity, try looking it up in database using the URI. The queries are ad-hoc because we don't have a global index of object URIs, and not all activity types are stored in database with an index on their URI. Announces are just statuses, and have an index on URIs, so this check can be done efficiently. Accepts cannot be handled at all because we don't record their URI at any point. Follows don't have an index on URI, but they have an index on the issuing account, which should make such queries largely manageable. Likes don't have an index on URI, they have an index on the issuing account, but the number of favs per account may be very high, so I decided not to handle that. Blocks don't have an index on URI, but they have an index on the issuing account, which should make such queries largely manageable. In all cases, if an Undo could not be handled properly, we call `delete_later!` because that does not require us to know more than the URI of the undone property. * Add tests * Make newer blocks overwrite older ones Allows re-synchronizing block info by re-blocking and un-blocking again when the original Undo Block has been lost. --- app/lib/activitypub/activity/block.rb | 7 +++- app/lib/activitypub/activity/undo.rb | 51 +++++++++++++++++++++++++++++ spec/lib/activitypub/activity/block_spec.rb | 22 +++++++++++++ spec/lib/activitypub/activity/undo_spec.rb | 35 ++++++++++++++++++-- 4 files changed, 112 insertions(+), 3 deletions(-) (limited to 'app/lib') diff --git a/app/lib/activitypub/activity/block.rb b/app/lib/activitypub/activity/block.rb index a17a2d50a..90477bf33 100644 --- a/app/lib/activitypub/activity/block.rb +++ b/app/lib/activitypub/activity/block.rb @@ -4,7 +4,12 @@ class ActivityPub::Activity::Block < ActivityPub::Activity def perform target_account = account_from_uri(object_uri) - return if target_account.nil? || !target_account.local? || @account.blocking?(target_account) + return if target_account.nil? || !target_account.local? + + if @account.blocking?(target_account) + @account.block_relationships.find_by(target_account: target_account).update(uri: @json['id']) if @json['id'].present? + return + end UnfollowService.new.call(target_account, @account) if target_account.following?(@account) diff --git a/app/lib/activitypub/activity/undo.rb b/app/lib/activitypub/activity/undo.rb index 599823c6e..9eff1b71c 100644 --- a/app/lib/activitypub/activity/undo.rb +++ b/app/lib/activitypub/activity/undo.rb @@ -13,11 +13,62 @@ class ActivityPub::Activity::Undo < ActivityPub::Activity undo_like when 'Block' undo_block + when nil + handle_reference end end private + def handle_reference + # Some implementations do not inline the object, and as we don't have a + # global index, we have to guess what object it is. + return if object_uri.nil? + + try_undo_announce || try_undo_accept || try_undo_follow || try_undo_like || try_undo_block || delete_later!(object_uri) + end + + def try_undo_announce + status = Status.where.not(reblog_of_id: nil).find_by(uri: object_uri, account: @account) + if status.present? + RemoveStatusService.new.call(status) + true + else + false + end + end + + def try_undo_accept + # We can't currently handle `Undo Accept` as we don't record `Accept`'s uri + false + end + + def try_undo_follow + follow = @account.follow_requests.find_by(uri: object_uri) || @account.active_relationships.find_by(uri: object_uri) + + if follow.present? + follow.destroy + true + else + false + end + end + + def try_undo_like + # There is an index on accounts, but an account may have *many* favs, so this may be too costly + false + end + + def try_undo_block + block = @account.block_relationships.find_by(uri: object_uri) + if block.present? + UnblockService.new.call(@account, block.target_account) + true + else + false + end + end + def undo_announce return if object_uri.nil? diff --git a/spec/lib/activitypub/activity/block_spec.rb b/spec/lib/activitypub/activity/block_spec.rb index 94d37356d..42bdfdc81 100644 --- a/spec/lib/activitypub/activity/block_spec.rb +++ b/spec/lib/activitypub/activity/block_spec.rb @@ -28,6 +28,28 @@ RSpec.describe ActivityPub::Activity::Block do end end + context 'when the recipient is already blocked' do + before do + sender.block!(recipient, uri: 'old') + end + + describe '#perform' do + subject { described_class.new(json, sender) } + + before do + subject.perform + end + + it 'creates a block from sender to recipient' do + expect(sender.blocking?(recipient)).to be true + end + + it 'sets the uri to that of last received block activity' do + expect(sender.block_relationships.find_by(target_account: recipient).uri).to eq 'foo' + end + end + end + context 'when the recipient follows the sender' do before do recipient.follow!(sender) diff --git a/spec/lib/activitypub/activity/undo_spec.rb b/spec/lib/activitypub/activity/undo_spec.rb index 9545e1f46..c0309e49d 100644 --- a/spec/lib/activitypub/activity/undo_spec.rb +++ b/spec/lib/activitypub/activity/undo_spec.rb @@ -50,6 +50,19 @@ RSpec.describe ActivityPub::Activity::Undo do expect(sender.reblogged?(status)).to be false end end + + context 'with only object uri' do + let(:object_json) { 'bar' } + + before do + Fabricate(:status, reblog: status, account: sender, uri: 'bar') + end + + it 'deletes the reblog by uri' do + subject.perform + expect(sender.reblogged?(status)).to be false + end + end end context 'with Accept' do @@ -91,13 +104,22 @@ RSpec.describe ActivityPub::Activity::Undo do end before do - sender.block!(recipient) + sender.block!(recipient, uri: 'bar') end it 'deletes block from sender to recipient' do subject.perform expect(sender.blocking?(recipient)).to be false end + + context 'with only object uri' do + let(:object_json) { 'bar' } + + it 'deletes block from sender to recipient' do + subject.perform + expect(sender.blocking?(recipient)).to be false + end + end end context 'with Follow' do @@ -113,13 +135,22 @@ RSpec.describe ActivityPub::Activity::Undo do end before do - sender.follow!(recipient) + sender.follow!(recipient, uri: 'bar') end it 'deletes follow from sender to recipient' do subject.perform expect(sender.following?(recipient)).to be false end + + context 'with only object uri' do + let(:object_json) { 'bar' } + + it 'deletes follow from sender to recipient' do + subject.perform + expect(sender.following?(recipient)).to be false + end + end end context 'with Like' do -- cgit