Skip to content

Commit

Permalink
Fix followers synchronization mechanism not working when URI has empt…
Browse files Browse the repository at this point in the history
…y path (mastodon#16510)

* Fix followers synchronization mechanism not working when URI has empty path

To my knowledge, there is no current implementation on the fediverse
that can use bare domains (e.g., actor is at https://example.org instead of
something like https://example.org/actor) that also plans to support the
followers synchronization mechanism. However, Mastodon's current implementation
would exclude such accounts from followers list.

Also adds tests and rename them to reflect the proper method names.

* Move url prefix regexp to its own constant
  • Loading branch information
ClearlyClaire committed Jan 31, 2022
1 parent 2c83b90 commit 03f0e98
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 35 deletions.
3 changes: 2 additions & 1 deletion app/models/account.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ class Account < ApplicationRecord

USERNAME_RE = /[a-z0-9_]+([a-z0-9_\.-]+[a-z0-9_]+)?/i
MENTION_RE = /(?<=^|[^\/[:word:]])@((#{USERNAME_RE})(?:@[[:word:]\.\-]+[[:word:]]+)?)/i
URL_PREFIX_RE = /\Ahttp(s?):\/\/[^\/]+/

include AccountAssociations
include AccountAvatar
Expand Down Expand Up @@ -379,7 +380,7 @@ def preferred_inbox_url
def synchronization_uri_prefix
return 'local' if local?

@synchronization_uri_prefix ||= uri[/http(s?):\/\/[^\/]+\//]
@synchronization_uri_prefix ||= "#{uri[URL_PREFIX_RE]}/"
end

class Field < ActiveModelSerializers::Model
Expand Down
9 changes: 6 additions & 3 deletions app/models/concerns/account_interactions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -251,10 +251,13 @@ def lists_for_local_distribution
.where('users.current_sign_in_at > ?', User::ACTIVE_DURATION.ago)
end

def remote_followers_hash(url_prefix)
Rails.cache.fetch("followers_hash:#{id}:#{url_prefix}") do
def remote_followers_hash(url)
url_prefix = url[Account::URL_PREFIX_RE]
return if url_prefix.blank?

Rails.cache.fetch("followers_hash:#{id}:#{url_prefix}/") do
digest = "\x00" * 32
followers.where(Account.arel_table[:uri].matches(url_prefix + '%', false, true)).pluck_each(:uri) do |uri|
followers.where(Account.arel_table[:uri].matches("#{Account.sanitize_sql_like(url_prefix)}/%", false, true)).or(followers.where(uri: url_prefix)).pluck_each(:uri) do |uri|
Xorcist.xor!(digest, Digest::SHA256.digest(uri))
end
digest.unpack('H*')[0]
Expand Down
6 changes: 1 addition & 5 deletions app/workers/activitypub/delivery_worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,7 @@ def build_request(http_client)
end

def synchronization_header
"collectionId=\"#{account_followers_url(@source_account)}\", digest=\"#{@source_account.remote_followers_hash(inbox_url_prefix)}\", url=\"#{account_followers_synchronization_url(@source_account)}\""
end

def inbox_url_prefix
@inbox_url[/http(s?):\/\/[^\/]+\//]
"collectionId=\"#{account_followers_url(@source_account)}\", digest=\"#{@source_account.remote_followers_hash(@inbox_url)}\", url=\"#{account_followers_synchronization_url(@source_account)}\""
end

def perform_request
Expand Down
61 changes: 36 additions & 25 deletions spec/models/concerns/account_interactions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -539,46 +539,57 @@
end
end

describe '#followers_hash' do
describe '#remote_followers_hash' do
let(:me) { Fabricate(:account, username: 'Me') }
let(:remote_1) { Fabricate(:account, username: 'alice', domain: 'example.org', uri: 'https://example.org/users/alice') }
let(:remote_2) { Fabricate(:account, username: 'bob', domain: 'example.org', uri: 'https://example.org/users/bob') }
let(:remote_3) { Fabricate(:account, username: 'eve', domain: 'foo.org', uri: 'https://foo.org/users/eve') }
let(:remote_3) { Fabricate(:account, username: 'instance-actor', domain: 'example.org', uri: 'https://example.org') }
let(:remote_4) { Fabricate(:account, username: 'eve', domain: 'foo.org', uri: 'https://foo.org/users/eve') }

before do
remote_1.follow!(me)
remote_2.follow!(me)
remote_3.follow!(me)
remote_4.follow!(me)
me.follow!(remote_1)
end

context 'on a local user' do
it 'returns correct hash for remote domains' do
expect(me.remote_followers_hash('https://example.org/')).to eq '707962e297b7bd94468a21bc8e506a1bcea607a9142cd64e27c9b106b2a5f6ec'
expect(me.remote_followers_hash('https://foo.org/')).to eq 'ccb9c18a67134cfff9d62c7f7e7eb88e6b803446c244b84265565f4eba29df0e'
end
it 'returns correct hash for remote domains' do
expect(me.remote_followers_hash('https://example.org/')).to eq '20aecbe774b3d61c25094370baf370012b9271c5b172ecedb05caff8d79ef0c7'
expect(me.remote_followers_hash('https://foo.org/')).to eq 'ccb9c18a67134cfff9d62c7f7e7eb88e6b803446c244b84265565f4eba29df0e'
expect(me.remote_followers_hash('https://foo.org.evil.com/')).to eq '0000000000000000000000000000000000000000000000000000000000000000'
expect(me.remote_followers_hash('https://foo')).to eq '0000000000000000000000000000000000000000000000000000000000000000'
end

it 'invalidates cache as needed when removing or adding followers' do
expect(me.remote_followers_hash('https://example.org/')).to eq '707962e297b7bd94468a21bc8e506a1bcea607a9142cd64e27c9b106b2a5f6ec'
remote_1.unfollow!(me)
expect(me.remote_followers_hash('https://example.org/')).to eq '241b00794ce9b46aa864f3220afadef128318da2659782985bac5ed5bd436bff'
remote_1.follow!(me)
expect(me.remote_followers_hash('https://example.org/')).to eq '707962e297b7bd94468a21bc8e506a1bcea607a9142cd64e27c9b106b2a5f6ec'
end
it 'invalidates cache as needed when removing or adding followers' do
expect(me.remote_followers_hash('https://example.org/')).to eq '20aecbe774b3d61c25094370baf370012b9271c5b172ecedb05caff8d79ef0c7'
remote_3.unfollow!(me)
expect(me.remote_followers_hash('https://example.org/')).to eq '707962e297b7bd94468a21bc8e506a1bcea607a9142cd64e27c9b106b2a5f6ec'
remote_1.unfollow!(me)
expect(me.remote_followers_hash('https://example.org/')).to eq '241b00794ce9b46aa864f3220afadef128318da2659782985bac5ed5bd436bff'
remote_1.follow!(me)
expect(me.remote_followers_hash('https://example.org/')).to eq '707962e297b7bd94468a21bc8e506a1bcea607a9142cd64e27c9b106b2a5f6ec'
end
end

context 'on a remote user' do
it 'returns correct hash for remote domains' do
expect(remote_1.local_followers_hash).to eq Digest::SHA256.hexdigest(ActivityPub::TagManager.instance.uri_for(me))
end
describe '#local_followers_hash' do
let(:me) { Fabricate(:account, username: 'Me') }
let(:remote_1) { Fabricate(:account, username: 'alice', domain: 'example.org', uri: 'https://example.org/users/alice') }

it 'invalidates cache as needed when removing or adding followers' do
expect(remote_1.local_followers_hash).to eq Digest::SHA256.hexdigest(ActivityPub::TagManager.instance.uri_for(me))
me.unfollow!(remote_1)
expect(remote_1.local_followers_hash).to eq '0000000000000000000000000000000000000000000000000000000000000000'
me.follow!(remote_1)
expect(remote_1.local_followers_hash).to eq Digest::SHA256.hexdigest(ActivityPub::TagManager.instance.uri_for(me))
end
before do
me.follow!(remote_1)
end

it 'returns correct hash for local users' do
expect(remote_1.local_followers_hash).to eq Digest::SHA256.hexdigest(ActivityPub::TagManager.instance.uri_for(me))
end

it 'invalidates cache as needed when removing or adding followers' do
expect(remote_1.local_followers_hash).to eq Digest::SHA256.hexdigest(ActivityPub::TagManager.instance.uri_for(me))
me.unfollow!(remote_1)
expect(remote_1.local_followers_hash).to eq '0000000000000000000000000000000000000000000000000000000000000000'
me.follow!(remote_1)
expect(remote_1.local_followers_hash).to eq Digest::SHA256.hexdigest(ActivityPub::TagManager.instance.uri_for(me))
end
end

Expand Down
2 changes: 1 addition & 1 deletion spec/workers/activitypub/delivery_worker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
let(:payload) { 'test' }

before do
allow_any_instance_of(Account).to receive(:remote_followers_hash).with('https://example.com/').and_return('somehash')
allow_any_instance_of(Account).to receive(:remote_followers_hash).with('https://example.com/api').and_return('somehash')
end

describe 'perform' do
Expand Down

0 comments on commit 03f0e98

Please sign in to comment.