Refactor fetching of remote resources (#11251)

7 files changed, 97 insertions, 137 deletions

diff --git a/app/services/activitypub/fetch_remote_status_service.rb b/app/services/activitypub/fetch_remote_status_service.rb
index 469821032..cf4f62899 100644
--- a/app/services/activitypub/fetch_remote_status_service.rb
+++ b/app/services/activitypub/fetch_remote_status_service.rb
@@ -5,18 +5,18 @@ class ActivityPub::FetchRemoteStatusService < BaseService
 
   # Should be called when uri has already been checked for locality
   def call(uri, id: true, prefetched_body: nil, on_behalf_of: nil)
-    @json = if prefetched_body.nil?
-              fetch_resource(uri, id, on_behalf_of)
-            else
-              body_to_json(prefetched_body, compare_id: id ? uri : nil)
-            end
+    @json = begin
+      if prefetched_body.nil?
+        fetch_resource(uri, id, on_behalf_of)
+      else
+        body_to_json(prefetched_body, compare_id: id ? uri : nil)
+      end
+    end
 
-    return unless supported_context? && expected_type?
-
-    return if actor_id.nil? || !trustworthy_attribution?(@json['id'], actor_id)
+    return if !(supported_context? && expected_type?) || actor_id.nil? || !trustworthy_attribution?(@json['id'], actor_id)
 
     actor = ActivityPub::TagManager.instance.uri_to_resource(actor_id, Account)
-    actor = ActivityPub::FetchRemoteAccountService.new.call(actor_id, id: true) if actor.nil? || needs_update(actor)
+    actor = ActivityPub::FetchRemoteAccountService.new.call(actor_id, id: true) if actor.nil? || needs_update?(actor)
 
     return if actor.nil? || actor.suspended?
 
@@ -46,7 +46,7 @@ class ActivityPub::FetchRemoteStatusService < BaseService
     equals_or_includes_any?(@json['type'], ActivityPub::Activity::Create::SUPPORTED_TYPES + ActivityPub::Activity::Create::CONVERTED_TYPES)
   end
 
-  def needs_update(actor)
+  def needs_update?(actor)
     actor.possibly_stale?
   end
 end
diff --git a/app/services/fetch_atom_service.rb b/app/services/fetch_atom_service.rb
deleted file mode 100644
index d6508a988..000000000
--- a/app/services/fetch_atom_service.rb
+++ /dev/null
@@ -1,93 +0,0 @@
-# frozen_string_literal: true
-
-class FetchAtomService < BaseService
-  include JsonLdHelper
-
-  def call(url)
-    return if url.blank?
-
-    result = process(url)
-
-    # retry without ActivityPub
-    result ||= process(url) if @unsupported_activity
-
-    result
-  rescue OpenSSL::SSL::SSLError => e
-    Rails.logger.debug "SSL error: #{e}"
-    nil
-  rescue HTTP::ConnectionError => e
-    Rails.logger.debug "HTTP ConnectionError: #{e}"
-    nil
-  end
-
-  private
-
-  def process(url, terminal = false)
-    @url = url
-    perform_request { |response| process_response(response, terminal) }
-  end
-
-  def perform_request(&block)
-    accept = 'text/html'
-    accept = 'application/activity+json, application/ld+json; profile="https://www.w3.org/ns/activitystreams", application/atom+xml, ' + accept unless @unsupported_activity
-
-    Request.new(:get, @url).add_headers('Accept' => accept).perform(&block)
-  end
-
-  def process_response(response, terminal = false)
-    return nil if response.code != 200
-
-    if response.mime_type == 'application/atom+xml'
-      [@url, { prefetched_body: response.body_with_limit }, :ostatus]
-    elsif ['application/activity+json', 'application/ld+json'].include?(response.mime_type)
-      body = response.body_with_limit
-      json = body_to_json(body)
-      if supported_context?(json) && equals_or_includes_any?(json['type'], ActivityPub::FetchRemoteAccountService::SUPPORTED_TYPES) && json['inbox'].present?
-        [json['id'], { prefetched_body: body, id: true }, :activitypub]
-      elsif supported_context?(json) && expected_type?(json)
-        [json['id'], { prefetched_body: body, id: true }, :activitypub]
-      else
-        @unsupported_activity = true
-        nil
-      end
-    elsif !terminal
-      link_header = response['Link'] && parse_link_header(response)
-
-      if link_header&.find_link(%w(rel alternate))
-        process_link_headers(link_header)
-      elsif response.mime_type == 'text/html'
-        process_html(response)
-      end
-    end
-  end
-
-  def expected_type?(json)
-    equals_or_includes_any?(json['type'], ActivityPub::Activity::Create::SUPPORTED_TYPES + ActivityPub::Activity::Create::CONVERTED_TYPES)
-  end
-
-  def process_html(response)
-    page = Nokogiri::HTML(response.body_with_limit)
-
-    json_link = page.xpath('//link[@rel="alternate"]').find { |link| ['application/activity+json', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'].include?(link['type']) }
-    atom_link = page.xpath('//link[@rel="alternate"]').find { |link| link['type'] == 'application/atom+xml' }
-
-    result ||= process(json_link['href'], terminal: true) unless json_link.nil? || @unsupported_activity
-    result ||= process(atom_link['href'], terminal: true) unless atom_link.nil?
-
-    result
-  end
-
-  def process_link_headers(link_header)
-    json_link = link_header.find_link(%w(rel alternate), %w(type application/activity+json)) || link_header.find_link(%w(rel alternate), ['type', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'])
-    atom_link = link_header.find_link(%w(rel alternate), %w(type application/atom+xml))
-
-    result ||= process(json_link.href, terminal: true) unless json_link.nil? || @unsupported_activity
-    result ||= process(atom_link.href, terminal: true) unless atom_link.nil?
-
-    result
-  end
-
-  def parse_link_header(response)
-    LinkHeader.parse(response['Link'].is_a?(Array) ? response['Link'].first : response['Link'])
-  end
-end
diff --git a/app/services/fetch_link_card_service.rb b/app/services/fetch_link_card_service.rb
index 75fbd0e8c..4e75c370f 100644
--- a/app/services/fetch_link_card_service.rb
+++ b/app/services/fetch_link_card_service.rb
@@ -29,7 +29,7 @@ class FetchLinkCardService < BaseService
     end
 
     attach_card if @card&.persisted?
-  rescue HTTP::Error, Addressable::URI::InvalidURIError, Mastodon::HostValidationError, Mastodon::LengthValidationError => e
+  rescue HTTP::Error, OpenSSL::SSL::SSLError, Addressable::URI::InvalidURIError, Mastodon::HostValidationError, Mastodon::LengthValidationError => e
     Rails.logger.debug "Error fetching link #{@url}: #{e}"
     nil
   end
diff --git a/app/services/fetch_remote_account_service.rb b/app/services/fetch_remote_account_service.rb
index a7f95603d..3cd06e30f 100644
--- a/app/services/fetch_remote_account_service.rb
+++ b/app/services/fetch_remote_account_service.rb
@@ -3,7 +3,7 @@
 class FetchRemoteAccountService < BaseService
   def call(url, prefetched_body = nil, protocol = :ostatus)
     if prefetched_body.nil?
-      resource_url, resource_options, protocol = FetchAtomService.new.call(url)
+      resource_url, resource_options, protocol = FetchResourceService.new.call(url)
     else
       resource_url     = url
       resource_options = { prefetched_body: prefetched_body }
diff --git a/app/services/fetch_remote_status_service.rb b/app/services/fetch_remote_status_service.rb
index aac39dfd5..208dc7809 100644
--- a/app/services/fetch_remote_status_service.rb
+++ b/app/services/fetch_remote_status_service.rb
@@ -3,7 +3,7 @@
 class FetchRemoteStatusService < BaseService
   def call(url, prefetched_body = nil, protocol = :ostatus)
     if prefetched_body.nil?
-      resource_url, resource_options, protocol = FetchAtomService.new.call(url)
+      resource_url, resource_options, protocol = FetchResourceService.new.call(url)
     else
       resource_url     = url
       resource_options = { prefetched_body: prefetched_body }
diff --git a/app/services/fetch_resource_service.rb b/app/services/fetch_resource_service.rb
new file mode 100644
index 000000000..c0473f3ad
--- /dev/null
+++ b/app/services/fetch_resource_service.rb
@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+
+class FetchResourceService < BaseService
+  include JsonLdHelper
+
+  ACCEPT_HEADER = 'application/activity+json, application/ld+json; profile="https://www.w3.org/ns/activitystreams", text/html'
+
+  def call(url)
+    return if url.blank?
+
+    process(url)
+  rescue HTTP::Error, OpenSSL::SSL::SSLError, Addressable::URI::InvalidURIError, Mastodon::HostValidationError, Mastodon::LengthValidationError => e
+    Rails.logger.debug "Error fetching resource #{@url}: #{e}"
+    nil
+  end
+
+  private
+
+  def process(url, terminal = false)
+    @url = url
+
+    perform_request { |response| process_response(response, terminal) }
+  end
+
+  def perform_request(&block)
+    Request.new(:get, @url).add_headers('Accept' => ACCEPT_HEADER).perform(&block)
+  end
+
+  def process_response(response, terminal = false)
+    return nil if response.code != 200
+
+    if ['application/activity+json', 'application/ld+json'].include?(response.mime_type)
+      body = response.body_with_limit
+      json = body_to_json(body)
+
+      [json['id'], { prefetched_body: body, id: true }, :activitypub] if supported_context?(json) && (equals_or_includes_any?(json['type'], ActivityPub::FetchRemoteAccountService::SUPPORTED_TYPES) || expected_type?(json))
+    elsif !terminal
+      link_header = response['Link'] && parse_link_header(response)
+
+      if link_header&.find_link(%w(rel alternate))
+        process_link_headers(link_header)
+      elsif response.mime_type == 'text/html'
+        process_html(response)
+      end
+    end
+  end
+
+  def expected_type?(json)
+    equals_or_includes_any?(json['type'], ActivityPub::Activity::Create::SUPPORTED_TYPES + ActivityPub::Activity::Create::CONVERTED_TYPES)
+  end
+
+  def process_html(response)
+    page      = Nokogiri::HTML(response.body_with_limit)
+    json_link = page.xpath('//link[@rel="alternate"]').find { |link| ['application/activity+json', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'].include?(link['type']) }
+
+    process(json_link['href'], terminal: true) unless json_link.nil?
+  end
+
+  def process_link_headers(link_header)
+    json_link = link_header.find_link(%w(rel alternate), %w(type application/activity+json)) || link_header.find_link(%w(rel alternate), ['type', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'])
+
+    process(json_link.href, terminal: true) unless json_link.nil?
+  end
+
+  def parse_link_header(response)
+    LinkHeader.parse(response['Link'].is_a?(Array) ? response['Link'].first : response['Link'])
+  end
+end
diff --git a/app/services/resolve_url_service.rb b/app/services/resolve_url_service.rb
index f941b489a..80381c16b 100644
--- a/app/services/resolve_url_service.rb
+++ b/app/services/resolve_url_service.rb
@@ -4,64 +4,49 @@ class ResolveURLService < BaseService
   include JsonLdHelper
   include Authorization
 
-  attr_reader :url
-
   def call(url, on_behalf_of: nil)
-    @url = url
+    @url          = url
     @on_behalf_of = on_behalf_of
 
-    return process_local_url if local_url?
-
-    process_url unless fetched_atom_feed.nil?
+    if local_url?
+      process_local_url
+    elsif !fetched_resource.nil?
+      process_url
+    end
   end
 
   private
 
   def process_url
     if equals_or_includes_any?(type, ActivityPub::FetchRemoteAccountService::SUPPORTED_TYPES)
-      FetchRemoteAccountService.new.call(atom_url, body, protocol)
+      FetchRemoteAccountService.new.call(resource_url, body, protocol)
     elsif equals_or_includes_any?(type, ActivityPub::Activity::Create::SUPPORTED_TYPES + ActivityPub::Activity::Create::CONVERTED_TYPES)
-      FetchRemoteStatusService.new.call(atom_url, body, protocol)
+      FetchRemoteStatusService.new.call(resource_url, body, protocol)
     end
   end
 
-  def fetched_atom_feed
-    @_fetched_atom_feed ||= FetchAtomService.new.call(url)
+  def fetched_resource
+    @fetched_resource ||= FetchResourceService.new.call(@url)
   end
 
-  def atom_url
-    fetched_atom_feed.first
+  def resource_url
+    fetched_resource.first
   end
 
   def body
-    fetched_atom_feed.second[:prefetched_body]
+    fetched_resource.second[:prefetched_body]
   end
 
   def protocol
-    fetched_atom_feed.third
+    fetched_resource.third
   end
 
   def type
     return json_data['type'] if protocol == :activitypub
-
-    case xml_root
-    when 'feed'
-      'Person'
-    when 'entry'
-      'Note'
-    end
   end
 
   def json_data
-    @_json_data ||= body_to_json(body)
-  end
-
-  def xml_root
-    xml_data.root.name
-  end
-
-  def xml_data
-    @_xml_data ||= Nokogiri::XML(body, nil, 'utf-8')
+    @json_data ||= body_to_json(body)
   end
 
   def local_url?
@@ -83,10 +68,10 @@ class ResolveURLService < BaseService
 
   def check_local_status(status)
     return if status.nil?
+
     authorize_with @on_behalf_of, status, :show?
     status
   rescue Mastodon::NotPermittedError
-    # Do not disclose the existence of status the user is not authorized to see
     nil
   end
 end