From e0bdaeab657d9a320aaf506d98ca82d41e7bfdd5 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 17 May 2022 14:52:26 +0200 Subject: [PATCH] Fix NoMethodError when resolving a link that redirects to a local post (#18314) * Fix NoMethodError when resolving a link that redirects to a local post * Fix tests --- .../fetch_remote_status_service.rb | 1 + .../fetch_remote_status_service_spec.rb | 40 +++++++++---------- .../fetch_remote_status_service_spec.rb | 6 +-- spec/services/resolve_url_service_spec.rb | 19 +++++++++ 4 files changed, 42 insertions(+), 24 deletions(-) diff --git a/app/services/activitypub/fetch_remote_status_service.rb b/app/services/activitypub/fetch_remote_status_service.rb index 9672b3d2b6a..80309824509 100644 --- a/app/services/activitypub/fetch_remote_status_service.rb +++ b/app/services/activitypub/fetch_remote_status_service.rb @@ -30,6 +30,7 @@ class ActivityPub::FetchRemoteStatusService < BaseService end return if activity_json.nil? || object_uri.nil? || !trustworthy_attribution?(@json['id'], actor_uri) + return ActivityPub::TagManager.instance.uri_to_resource(object_uri, Status) if ActivityPub::TagManager.instance.local_uri?(object_uri) actor = account_from_uri(actor_uri) diff --git a/spec/services/activitypub/fetch_remote_status_service_spec.rb b/spec/services/activitypub/fetch_remote_status_service_spec.rb index 943cb161d78..7359ca0b43b 100644 --- a/spec/services/activitypub/fetch_remote_status_service_spec.rb +++ b/spec/services/activitypub/fetch_remote_status_service_spec.rb @@ -3,16 +3,15 @@ require 'rails_helper' RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do include ActionView::Helpers::TextHelper - let!(:sender) { Fabricate(:account).tap { |account| account.update(uri: ActivityPub::TagManager.instance.uri_for(account)) } } + let!(:sender) { Fabricate(:account, domain: 'foo.bar', uri: 'https://foo.bar') } let!(:recipient) { Fabricate(:account) } - let!(:valid_domain) { Rails.configuration.x.local_domain } let(:existing_status) { nil } let(:note) do { '@context': 'https://www.w3.org/ns/activitystreams', - id: "https://#{valid_domain}/@foo/1234", + id: "https://foo.bar/@foo/1234", type: 'Note', content: 'Lorem ipsum', attributedTo: ActivityPub::TagManager.instance.uri_for(sender), @@ -22,7 +21,8 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do subject { described_class.new } before do - stub_request(:head, 'https://example.com/watch?v=12345').to_return(status: 404, body: '') + stub_request(:get, 'https://foo.bar/watch?v=12345').to_return(status: 404, body: '') + stub_request(:get, object[:id]).to_return(body: Oj.dump(object)) end describe '#call' do @@ -46,7 +46,7 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do let(:object) do { '@context': 'https://www.w3.org/ns/activitystreams', - id: "https://#{valid_domain}/@foo/1234", + id: "https://foo.bar/@foo/1234", type: 'Video', name: 'Nyan Cat 10 hours remix', attributedTo: ActivityPub::TagManager.instance.uri_for(sender), @@ -54,13 +54,13 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do { type: 'Link', mimeType: 'application/x-bittorrent', - href: "https://#{valid_domain}/12345.torrent", + href: "https://foo.bar/12345.torrent", }, { type: 'Link', mimeType: 'text/html', - href: "https://#{valid_domain}/watch?v=12345", + href: "https://foo.bar/watch?v=12345", }, ], } @@ -70,8 +70,8 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do status = sender.statuses.first expect(status).to_not be_nil - expect(status.url).to eq "https://#{valid_domain}/watch?v=12345" - expect(strip_tags(status.text)).to eq "Nyan Cat 10 hours remixhttps://#{valid_domain}/watch?v=12345" + expect(status.url).to eq "https://foo.bar/watch?v=12345" + expect(strip_tags(status.text)).to eq "Nyan Cat 10 hours remixhttps://foo.bar/watch?v=12345" end end @@ -79,7 +79,7 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do let(:object) do { '@context': 'https://www.w3.org/ns/activitystreams', - id: "https://#{valid_domain}/@foo/1234", + id: "https://foo.bar/@foo/1234", type: 'Audio', name: 'Nyan Cat 10 hours remix', attributedTo: ActivityPub::TagManager.instance.uri_for(sender), @@ -87,13 +87,13 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do { type: 'Link', mimeType: 'application/x-bittorrent', - href: "https://#{valid_domain}/12345.torrent", + href: "https://foo.bar/12345.torrent", }, { type: 'Link', mimeType: 'text/html', - href: "https://#{valid_domain}/watch?v=12345", + href: "https://foo.bar/watch?v=12345", }, ], } @@ -103,8 +103,8 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do status = sender.statuses.first expect(status).to_not be_nil - expect(status.url).to eq "https://#{valid_domain}/watch?v=12345" - expect(strip_tags(status.text)).to eq "Nyan Cat 10 hours remixhttps://#{valid_domain}/watch?v=12345" + expect(status.url).to eq "https://foo.bar/watch?v=12345" + expect(strip_tags(status.text)).to eq "Nyan Cat 10 hours remixhttps://foo.bar/watch?v=12345" end end @@ -112,7 +112,7 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do let(:object) do { '@context': 'https://www.w3.org/ns/activitystreams', - id: "https://#{valid_domain}/@foo/1234", + id: "https://foo.bar/@foo/1234", type: 'Event', name: "Let's change the world", attributedTo: ActivityPub::TagManager.instance.uri_for(sender) @@ -123,8 +123,8 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do status = sender.statuses.first expect(status).to_not be_nil - expect(status.url).to eq "https://#{valid_domain}/@foo/1234" - expect(strip_tags(status.text)).to eq "Let's change the worldhttps://#{valid_domain}/@foo/1234" + expect(status.url).to eq "https://foo.bar/@foo/1234" + expect(strip_tags(status.text)).to eq "Let's change the worldhttps://foo.bar/@foo/1234" end end @@ -154,7 +154,7 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do let(:object) do { '@context': 'https://www.w3.org/ns/activitystreams', - id: "https://#{valid_domain}/@foo/1234/create", + id: "https://foo.bar/@foo/1234/create", type: 'Create', actor: ActivityPub::TagManager.instance.uri_for(sender), object: note, @@ -174,7 +174,7 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do let(:object) do { '@context': 'https://www.w3.org/ns/activitystreams', - id: "https://#{valid_domain}/@foo/1234/create", + id: "https://foo.bar/@foo/1234/create", type: 'Create', actor: ActivityPub::TagManager.instance.uri_for(sender), object: { @@ -208,7 +208,7 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do let(:object) do { '@context': 'https://www.w3.org/ns/activitystreams', - id: "https://#{valid_domain}/@foo/1234/create", + id: "https://foo.bar/@foo/1234/create", type: 'Create', actor: ActivityPub::TagManager.instance.uri_for(sender), object: note.merge(updated: '2021-09-08T22:39:25Z'), diff --git a/spec/services/fetch_remote_status_service_spec.rb b/spec/services/fetch_remote_status_service_spec.rb index 0e63cc9eba9..fe5f1aed19c 100644 --- a/spec/services/fetch_remote_status_service_spec.rb +++ b/spec/services/fetch_remote_status_service_spec.rb @@ -1,14 +1,13 @@ require 'rails_helper' RSpec.describe FetchRemoteStatusService, type: :service do - let(:account) { Fabricate(:account) } + let(:account) { Fabricate(:account, domain: 'example.org', uri: 'https://example.org/foo') } let(:prefetched_body) { nil } - let(:valid_domain) { Rails.configuration.x.local_domain } let(:note) do { '@context': 'https://www.w3.org/ns/activitystreams', - id: "https://#{valid_domain}/@foo/1234", + id: "https://example.org/@foo/1234", type: 'Note', content: 'Lorem ipsum', attributedTo: ActivityPub::TagManager.instance.uri_for(account), @@ -20,7 +19,6 @@ RSpec.describe FetchRemoteStatusService, type: :service do let(:prefetched_body) { Oj.dump(note) } before do - account.update(uri: ActivityPub::TagManager.instance.uri_for(account)) subject end diff --git a/spec/services/resolve_url_service_spec.rb b/spec/services/resolve_url_service_spec.rb index 1b639dea983..b3e3defbff2 100644 --- a/spec/services/resolve_url_service_spec.rb +++ b/spec/services/resolve_url_service_spec.rb @@ -126,5 +126,24 @@ describe ResolveURLService, type: :service do end end end + + context 'searching for a link that redirects to a local public status' do + let(:account) { Fabricate(:account) } + let(:poster) { Fabricate(:account) } + let!(:status) { Fabricate(:status, account: poster, visibility: :public) } + let(:url) { 'https://link.to/foobar' } + let(:status_url) { ActivityPub::TagManager.instance.url_for(status) } + let(:uri) { ActivityPub::TagManager.instance.uri_for(status) } + + before do + stub_request(:get, url).to_return(status: 302, headers: { 'Location' => status_url }) + body = ActiveModelSerializers::SerializableResource.new(status, serializer: ActivityPub::NoteSerializer, adapter: ActivityPub::Adapter).to_json + stub_request(:get, status_url).to_return(body: body, headers: { 'Content-Type' => 'application/activity+json' }) + end + + it 'returns status by url' do + expect(subject.call(url, on_behalf_of: account)).to eq(status) + end + end end end