diff --git a/app/controllers/activitypub/references_controller.rb b/app/controllers/activitypub/references_controller.rb index 7cd903eaf4..58c70e2771 100644 --- a/app/controllers/activitypub/references_controller.rb +++ b/app/controllers/activitypub/references_controller.rb @@ -5,8 +5,6 @@ class ActivityPub::ReferencesController < ActivityPub::BaseController include Authorization include AccountOwnedConcern - REFERENCES_LIMIT = 5 - before_action :require_signature!, if: :authorized_fetch_mode? before_action :set_status @@ -40,17 +38,21 @@ class ActivityPub::ReferencesController < ActivityPub::BaseController @results ||= begin references = @status.reference_objects.order(target_status_id: :asc) references = references.where('target_status_id > ?', page_params[:min_id]) if page_params[:min_id].present? - references = references.limit(limit_param(REFERENCES_LIMIT)) + references = references.limit(limit_param(references_limit)) references.pluck(:target_status_id) end end + def references_limit + StatusReference::REFERENCES_LIMIT + end + def pagination_min_id results.last end def records_continue? - results.size == limit_param(REFERENCES_LIMIT) + results.size == limit_param(references_limit) end def references_collection_presenter diff --git a/app/models/status_reference.rb b/app/models/status_reference.rb index 7bbd7b3232..79207291ac 100644 --- a/app/models/status_reference.rb +++ b/app/models/status_reference.rb @@ -14,6 +14,8 @@ # class StatusReference < ApplicationRecord + REFERENCES_LIMIT = 5 + belongs_to :status belongs_to :target_status, class_name: 'Status' diff --git a/app/services/activitypub/fetch_references_service.rb b/app/services/activitypub/fetch_references_service.rb index 682ec7eb16..92d9c1da3f 100644 --- a/app/services/activitypub/fetch_references_service.rb +++ b/app/services/activitypub/fetch_references_service.rb @@ -6,7 +6,7 @@ class ActivityPub::FetchReferencesService < BaseService def call(status, collection_or_uri) @account = status.account - collection_items(collection_or_uri)&.map { |item| value_or_id(item) } + collection_items(collection_or_uri)&.take(8)&.map { |item| value_or_id(item) } end private @@ -20,9 +20,9 @@ class ActivityPub::FetchReferencesService < BaseService case collection['type'] when 'Collection', 'CollectionPage' - collection['items'] + as_array(collection['items']) when 'OrderedCollection', 'OrderedCollectionPage' - collection['orderedItems'] + as_array(collection['orderedItems']) end end @@ -31,6 +31,19 @@ class ActivityPub::FetchReferencesService < BaseService return if unsupported_uri_scheme?(collection_or_uri) return if ActivityPub::TagManager.instance.local_uri?(collection_or_uri) - fetch_resource_without_id_validation(collection_or_uri, nil, true) + # NOTE: For backward compatibility reasons, Mastodon signs outgoing + # queries incorrectly by default. + # + # While this is relevant for all URLs with query strings, this is + # the only code path where this happens in practice. + # + # Therefore, retry with correct signatures if this fails. + begin + fetch_resource_without_id_validation(collection_or_uri, nil, true) + rescue Mastodon::UnexpectedResponseError => e + raise unless e.response && e.response.code == 401 && Addressable::URI.parse(collection_or_uri).query.present? + + fetch_resource_without_id_validation(collection_or_uri, nil, true, request_options: { with_query_string: true }) + end end end diff --git a/spec/services/activitypub/fetch_references_service_spec.rb b/spec/services/activitypub/fetch_references_service_spec.rb new file mode 100644 index 0000000000..90566818f6 --- /dev/null +++ b/spec/services/activitypub/fetch_references_service_spec.rb @@ -0,0 +1,128 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe ActivityPub::FetchReferencesService, type: :service do + subject { described_class.new.call(status, payload) } + + let(:actor) { Fabricate(:account, domain: 'example.com', uri: 'http://example.com/account') } + let(:status) { Fabricate(:status, account: actor) } + let(:collection_uri) { 'http://example.com/references/1' } + + let(:items) do + [ + 'http://example.com/self-references-1', + 'http://example.com/self-references-2', + 'http://example.com/self-references-3', + 'http://other.com/other-references-1', + 'http://other.com/other-references-2', + 'http://other.com/other-references-3', + 'http://example.com/self-references-4', + 'http://example.com/self-references-5', + 'http://example.com/self-references-6', + 'http://example.com/self-references-7', + 'http://example.com/self-references-8', + ] + end + + let(:payload) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'Collection', + id: collection_uri, + items: items, + }.with_indifferent_access + end + + describe '#call' do + context 'when the payload is a Collection with inlined replies' do + context 'when there is a single reference, with the array compacted away' do + let(:items) { 'http://example.com/self-references-1' } + + it 'a item is returned' do + expect(subject).to eq ['http://example.com/self-references-1'] + end + end + + context 'when passing the collection itself' do + it 'first 8 items are returned' do + expect(subject).to eq items.take(8) + end + end + + context 'when passing the URL to the collection' do + subject { described_class.new.call(status, collection_uri) } + + before do + stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload)) + end + + it 'first 8 items are returned' do + expect(subject).to eq items.take(8) + end + end + end + + context 'when the payload is an OrderedCollection with inlined references' do + let(:payload) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'OrderedCollection', + id: collection_uri, + orderedItems: items, + }.with_indifferent_access + end + + context 'when passing the collection itself' do + it 'first 8 items are returned' do + expect(subject).to eq items.take(8) + end + end + + context 'when passing the URL to the collection' do + subject { described_class.new.call(status, collection_uri) } + + before do + stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload)) + end + + it 'first 8 items are returned' do + expect(subject).to eq items.take(8) + end + end + end + + context 'when the payload is a paginated Collection with inlined references' do + let(:payload) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'Collection', + id: collection_uri, + first: { + type: 'CollectionPage', + partOf: collection_uri, + items: items, + }, + }.with_indifferent_access + end + + context 'when passing the collection itself' do + it 'first 8 items are returned' do + expect(subject).to eq items.take(8) + end + end + + context 'when passing the URL to the collection' do + subject { described_class.new.call(status, collection_uri) } + + before do + stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload)) + end + + it 'first 8 items are returned' do + expect(subject).to eq items.take(8) + end + end + end + end +end