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
This commit is contained in:
Claire 2022-05-17 14:52:26 +02:00 committed by GitHub
parent c78a622ba4
commit e0bdaeab65
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 42 additions and 24 deletions

View File

@ -30,6 +30,7 @@ class ActivityPub::FetchRemoteStatusService < BaseService
end end
return if activity_json.nil? || object_uri.nil? || !trustworthy_attribution?(@json['id'], actor_uri) 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) actor = account_from_uri(actor_uri)

View File

@ -3,16 +3,15 @@ require 'rails_helper'
RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do
include ActionView::Helpers::TextHelper 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!(:recipient) { Fabricate(:account) }
let!(:valid_domain) { Rails.configuration.x.local_domain }
let(:existing_status) { nil } let(:existing_status) { nil }
let(:note) do let(:note) do
{ {
'@context': 'https://www.w3.org/ns/activitystreams', '@context': 'https://www.w3.org/ns/activitystreams',
id: "https://#{valid_domain}/@foo/1234", id: "https://foo.bar/@foo/1234",
type: 'Note', type: 'Note',
content: 'Lorem ipsum', content: 'Lorem ipsum',
attributedTo: ActivityPub::TagManager.instance.uri_for(sender), attributedTo: ActivityPub::TagManager.instance.uri_for(sender),
@ -22,7 +21,8 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do
subject { described_class.new } subject { described_class.new }
before do 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 end
describe '#call' do describe '#call' do
@ -46,7 +46,7 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do
let(:object) do let(:object) do
{ {
'@context': 'https://www.w3.org/ns/activitystreams', '@context': 'https://www.w3.org/ns/activitystreams',
id: "https://#{valid_domain}/@foo/1234", id: "https://foo.bar/@foo/1234",
type: 'Video', type: 'Video',
name: 'Nyan Cat 10 hours remix', name: 'Nyan Cat 10 hours remix',
attributedTo: ActivityPub::TagManager.instance.uri_for(sender), attributedTo: ActivityPub::TagManager.instance.uri_for(sender),
@ -54,13 +54,13 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do
{ {
type: 'Link', type: 'Link',
mimeType: 'application/x-bittorrent', mimeType: 'application/x-bittorrent',
href: "https://#{valid_domain}/12345.torrent", href: "https://foo.bar/12345.torrent",
}, },
{ {
type: 'Link', type: 'Link',
mimeType: 'text/html', 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 status = sender.statuses.first
expect(status).to_not be_nil expect(status).to_not be_nil
expect(status.url).to eq "https://#{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://#{valid_domain}/watch?v=12345" expect(strip_tags(status.text)).to eq "Nyan Cat 10 hours remixhttps://foo.bar/watch?v=12345"
end end
end end
@ -79,7 +79,7 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do
let(:object) do let(:object) do
{ {
'@context': 'https://www.w3.org/ns/activitystreams', '@context': 'https://www.w3.org/ns/activitystreams',
id: "https://#{valid_domain}/@foo/1234", id: "https://foo.bar/@foo/1234",
type: 'Audio', type: 'Audio',
name: 'Nyan Cat 10 hours remix', name: 'Nyan Cat 10 hours remix',
attributedTo: ActivityPub::TagManager.instance.uri_for(sender), attributedTo: ActivityPub::TagManager.instance.uri_for(sender),
@ -87,13 +87,13 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do
{ {
type: 'Link', type: 'Link',
mimeType: 'application/x-bittorrent', mimeType: 'application/x-bittorrent',
href: "https://#{valid_domain}/12345.torrent", href: "https://foo.bar/12345.torrent",
}, },
{ {
type: 'Link', type: 'Link',
mimeType: 'text/html', 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 status = sender.statuses.first
expect(status).to_not be_nil expect(status).to_not be_nil
expect(status.url).to eq "https://#{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://#{valid_domain}/watch?v=12345" expect(strip_tags(status.text)).to eq "Nyan Cat 10 hours remixhttps://foo.bar/watch?v=12345"
end end
end end
@ -112,7 +112,7 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do
let(:object) do let(:object) do
{ {
'@context': 'https://www.w3.org/ns/activitystreams', '@context': 'https://www.w3.org/ns/activitystreams',
id: "https://#{valid_domain}/@foo/1234", id: "https://foo.bar/@foo/1234",
type: 'Event', type: 'Event',
name: "Let's change the world", name: "Let's change the world",
attributedTo: ActivityPub::TagManager.instance.uri_for(sender) attributedTo: ActivityPub::TagManager.instance.uri_for(sender)
@ -123,8 +123,8 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do
status = sender.statuses.first status = sender.statuses.first
expect(status).to_not be_nil expect(status).to_not be_nil
expect(status.url).to eq "https://#{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://#{valid_domain}/@foo/1234" expect(strip_tags(status.text)).to eq "Let's change the worldhttps://foo.bar/@foo/1234"
end end
end end
@ -154,7 +154,7 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do
let(:object) do let(:object) do
{ {
'@context': 'https://www.w3.org/ns/activitystreams', '@context': 'https://www.w3.org/ns/activitystreams',
id: "https://#{valid_domain}/@foo/1234/create", id: "https://foo.bar/@foo/1234/create",
type: 'Create', type: 'Create',
actor: ActivityPub::TagManager.instance.uri_for(sender), actor: ActivityPub::TagManager.instance.uri_for(sender),
object: note, object: note,
@ -174,7 +174,7 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do
let(:object) do let(:object) do
{ {
'@context': 'https://www.w3.org/ns/activitystreams', '@context': 'https://www.w3.org/ns/activitystreams',
id: "https://#{valid_domain}/@foo/1234/create", id: "https://foo.bar/@foo/1234/create",
type: 'Create', type: 'Create',
actor: ActivityPub::TagManager.instance.uri_for(sender), actor: ActivityPub::TagManager.instance.uri_for(sender),
object: { object: {
@ -208,7 +208,7 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do
let(:object) do let(:object) do
{ {
'@context': 'https://www.w3.org/ns/activitystreams', '@context': 'https://www.w3.org/ns/activitystreams',
id: "https://#{valid_domain}/@foo/1234/create", id: "https://foo.bar/@foo/1234/create",
type: 'Create', type: 'Create',
actor: ActivityPub::TagManager.instance.uri_for(sender), actor: ActivityPub::TagManager.instance.uri_for(sender),
object: note.merge(updated: '2021-09-08T22:39:25Z'), object: note.merge(updated: '2021-09-08T22:39:25Z'),

View File

@ -1,14 +1,13 @@
require 'rails_helper' require 'rails_helper'
RSpec.describe FetchRemoteStatusService, type: :service do 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(:prefetched_body) { nil }
let(:valid_domain) { Rails.configuration.x.local_domain }
let(:note) do let(:note) do
{ {
'@context': 'https://www.w3.org/ns/activitystreams', '@context': 'https://www.w3.org/ns/activitystreams',
id: "https://#{valid_domain}/@foo/1234", id: "https://example.org/@foo/1234",
type: 'Note', type: 'Note',
content: 'Lorem ipsum', content: 'Lorem ipsum',
attributedTo: ActivityPub::TagManager.instance.uri_for(account), attributedTo: ActivityPub::TagManager.instance.uri_for(account),
@ -20,7 +19,6 @@ RSpec.describe FetchRemoteStatusService, type: :service do
let(:prefetched_body) { Oj.dump(note) } let(:prefetched_body) { Oj.dump(note) }
before do before do
account.update(uri: ActivityPub::TagManager.instance.uri_for(account))
subject subject
end end

View File

@ -126,5 +126,24 @@ describe ResolveURLService, type: :service do
end end
end 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
end end