Remove dependency on goldfinger gem (#14919)

There are edge cases where requests to certain hosts timeout when
using the vanilla HTTP.rb gem, which the goldfinger gem uses. Now
that we no longer need to support OStatus servers, webfinger logic
is so simple that there is no point encapsulating it in a gem, so
we can just use our own Request class. With that, we benefit from
more robust timeout code and IPv4/IPv6 resolution.

Fix #14091

15 files changed, 114 insertions, 67 deletions

diff --git a/Gemfile b/Gemfile
index 3a92b4627..0fc631e2a 100644
--- a/Gemfile
+++ b/Gemfile
@@ -54,7 +54,6 @@ gem 'doorkeeper', '~> 5.4'
 gem 'ed25519', '~> 1.2'
 gem 'fast_blank', '~> 1.0'
 gem 'fastimage'
-gem 'goldfinger', '~> 2.1'
 gem 'hiredis', '~> 0.6'
 gem 'redis-namespace', '~> 1.8'
 gem 'health_check', git: 'https://github.com/ianheggie/health_check', ref: '0b799ead604f900ed50685e9b2d469cd2befba5b'
diff --git a/Gemfile.lock b/Gemfile.lock
index ae304e61e..21f5b1b4c 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -240,11 +240,6 @@ GEM
       ruby-progressbar (~> 1.4)
     globalid (0.4.2)
       activesupport (>= 4.2.0)
-    goldfinger (2.1.1)
-      addressable (~> 2.5)
-      http (~> 4.0)
-      nokogiri (~> 1.8)
-      oj (~> 3.0)
     hamlit (2.13.0)
       temple (>= 0.8.2)
       thor
@@ -714,7 +709,6 @@ DEPENDENCIES
   fog-core (<= 2.1.0)
   fog-openstack (~> 0.3)
   fuubar (~> 2.5)
-  goldfinger (~> 2.1)
   hamlit-rails (~> 0.2)
   health_check!
   hiredis (~> 0.6)
diff --git a/app/helpers/webfinger_helper.rb b/app/helpers/webfinger_helper.rb
index ab7ca4698..482f4e19e 100644
--- a/app/helpers/webfinger_helper.rb
+++ b/app/helpers/webfinger_helper.rb
@@ -1,38 +1,7 @@
 # frozen_string_literal: true
 
-# Monkey-patch on monkey-patch.
-# Because it conflicts with the request.rb patch.
-class HTTP::Timeout::PerOperationOriginal < HTTP::Timeout::PerOperation
-  def connect(socket_class, host, port, nodelay = false)
-    ::Timeout.timeout(@connect_timeout, HTTP::TimeoutError) do
-      @socket = socket_class.open(host, port)
-      @socket.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) if nodelay
-    end
-  end
-end
-
 module WebfingerHelper
   def webfinger!(uri)
-    hidden_service_uri = /\.(onion|i2p)(:\d+)?$/.match(uri)
-
-    raise Mastodon::HostValidationError, 'Instance does not support hidden service connections' if !Rails.configuration.x.access_to_hidden_service && hidden_service_uri
-
-    opts = {
-      ssl: !hidden_service_uri,
-
-      headers: {
-        'User-Agent': Mastodon::Version.user_agent,
-      },
-
-      timeout_class: HTTP::Timeout::PerOperationOriginal,
-
-      timeout_options: {
-        write_timeout: 10,
-        connect_timeout: 5,
-        read_timeout: 10,
-      },
-    }
-
-    Goldfinger::Client.new(uri, opts.merge(Rails.configuration.x.http_client_proxy)).finger
+    Webfinger.new(uri).perform
   end
 end
diff --git a/app/lib/webfinger.rb b/app/lib/webfinger.rb
new file mode 100644
index 000000000..b2374c494
--- /dev/null
+++ b/app/lib/webfinger.rb
@@ -0,0 +1,93 @@
+# frozen_string_literal: true
+
+class Webfinger
+  class Error < StandardError; end
+
+  class Response
+    def initialize(body)
+      @json = Oj.load(body, mode: :strict)
+    end
+
+    def subject
+      @json['subject']
+    end
+
+    def link(rel, attribute)
+      links.dig(rel, attribute)
+    end
+
+    private
+
+    def links
+      @links ||= @json['links'].map { |link| [link['rel'], link] }.to_h
+    end
+  end
+
+  def initialize(uri)
+    _, @domain = uri.split('@')
+
+    raise ArgumentError, 'Webfinger requested for local account' if @domain.nil?
+
+    @uri = uri
+  end
+
+  def perform
+    Response.new(body_from_webfinger)
+  rescue Oj::ParseError
+    raise Webfinger::Error, "Invalid JSON in response for #{@uri}"
+  rescue Addressable::URI::InvalidURIError
+    raise Webfinger::Error, "Invalid URI for #{@uri}"
+  end
+
+  private
+
+  def body_from_webfinger(url = standard_url, use_fallback = true)
+    webfinger_request(url).perform do |res|
+      if res.code == 200
+        res.body_with_limit
+      elsif res.code == 404 && use_fallback
+        body_from_host_meta
+      else
+        raise Webfinger::Error, "Request for #{@uri} returned HTTP #{res.code}"
+      end
+    end
+  end
+
+  def body_from_host_meta
+    host_meta_request.perform do |res|
+      if res.code == 200
+        body_from_webfinger(url_from_template(res.body_with_limit), false)
+      else
+        raise Webfinger::Error, "Request for #{@uri} returned HTTP #{res.code}"
+      end
+    end
+  end
+
+  def url_from_template(str)
+    link = Nokogiri::XML(str).at_xpath('//xmlns:Link[@rel="lrdd"]')
+
+    if link.present?
+      link['template'].gsub('{uri}', @uri)
+    else
+      raise Webfinger::Error, "Request for #{@uri} returned host-meta without link to Webfinger"
+    end
+  rescue Nokogiri::XML::XPath::SyntaxError
+    raise Webfinger::Error, "Invalid XML encountered in host-meta for #{@uri}"
+  end
+
+  def host_meta_request
+    Request.new(:get, host_meta_url).add_headers('Accept' => 'application/xrd+xml, application/xml, text/xml')
+  end
+
+  def webfinger_request(url)
+    Request.new(:get, url).add_headers('Accept' => 'application/jrd+json, application/json')
+  end
+
+  def standard_url
+    "https://#{@domain}/.well-known/webfinger?resource=#{@uri}"
+  end
+
+  def host_meta_url
+    "https://#{@domain}/.well-known/host-meta"
+  end
+end
diff --git a/app/models/account_alias.rb b/app/models/account_alias.rb
index 792e9e8d4..3d659142a 100644
--- a/app/models/account_alias.rb
+++ b/app/models/account_alias.rb
@@ -33,7 +33,7 @@ class AccountAlias < ApplicationRecord
   def set_uri
     target_account = ResolveAccountService.new.call(acct)
     self.uri       = ActivityPub::TagManager.instance.uri_for(target_account) unless target_account.nil?
-  rescue Goldfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error
+  rescue Webfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error
     # Validation will take care of it
   end
 
diff --git a/app/models/account_migration.rb b/app/models/account_migration.rb
index 681b5b2cd..4fae98ed7 100644
--- a/app/models/account_migration.rb
+++ b/app/models/account_migration.rb
@@ -54,7 +54,7 @@ class AccountMigration < ApplicationRecord
 
   def set_target_account
     self.target_account = ResolveAccountService.new.call(acct)
-  rescue Goldfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error
+  rescue Webfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error
     # Validation will take care of it
   end
 
diff --git a/app/models/form/redirect.rb b/app/models/form/redirect.rb
index a7961f8e8..19ee9faed 100644
--- a/app/models/form/redirect.rb
+++ b/app/models/form/redirect.rb
@@ -32,7 +32,7 @@ class Form::Redirect
 
   def set_target_account
     @target_account = ResolveAccountService.new.call(acct)
-  rescue Goldfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error
+  rescue Webfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error
     # Validation will take care of it
   end
 
diff --git a/app/models/remote_follow.rb b/app/models/remote_follow.rb
index 30b84f7d5..911c06713 100644
--- a/app/models/remote_follow.rb
+++ b/app/models/remote_follow.rb
@@ -56,7 +56,7 @@ class RemoteFollow
 
     if domain.nil?
       @addressable_template = Addressable::Template.new("#{authorize_interaction_url}?uri={uri}")
-    elsif redirect_url_link.nil? || redirect_url_link.template.nil?
+    elsif redirect_uri_template.nil?
       missing_resource_error
     else
       @addressable_template = Addressable::Template.new(redirect_uri_template)
@@ -64,16 +64,12 @@ class RemoteFollow
   end
 
   def redirect_uri_template
-    redirect_url_link.template
-  end
-
-  def redirect_url_link
-    acct_resource&.link('http://ostatus.org/schema/1.0/subscribe')
+    acct_resource&.link('http://ostatus.org/schema/1.0/subscribe', 'template')
   end
 
   def acct_resource
     @acct_resource ||= webfinger!("acct:#{acct}")
-  rescue Goldfinger::Error, HTTP::ConnectionError
+  rescue Webfinger::Error, HTTP::ConnectionError
     nil
   end
 
diff --git a/app/services/activitypub/fetch_remote_account_service.rb b/app/services/activitypub/fetch_remote_account_service.rb
index 83fbf6d07..e5bd0c47c 100644
--- a/app/services/activitypub/fetch_remote_account_service.rb
+++ b/app/services/activitypub/fetch_remote_account_service.rb
@@ -39,17 +39,16 @@ class ActivityPub::FetchRemoteAccountService < BaseService
     webfinger                            = webfinger!("acct:#{@username}@#{@domain}")
     confirmed_username, confirmed_domain = split_acct(webfinger.subject)
 
-    return webfinger.link('self')&.href == @uri if @username.casecmp(confirmed_username).zero? && @domain.casecmp(confirmed_domain).zero?
+    return webfinger.link('self', 'href') == @uri if @username.casecmp(confirmed_username).zero? && @domain.casecmp(confirmed_domain).zero?
 
     webfinger                            = webfinger!("acct:#{confirmed_username}@#{confirmed_domain}")
     @username, @domain                   = split_acct(webfinger.subject)
-    self_reference                       = webfinger.link('self')
 
     return false unless @username.casecmp(confirmed_username).zero? && @domain.casecmp(confirmed_domain).zero?
-    return false if self_reference&.href != @uri
+    return false if webfinger.link('self', 'href') != @uri
 
     true
-  rescue Goldfinger::Error
+  rescue Webfinger::Error
     false
   end
 
diff --git a/app/services/process_mentions_service.rb b/app/services/process_mentions_service.rb
index 12f0f1b08..0a710f843 100644
--- a/app/services/process_mentions_service.rb
+++ b/app/services/process_mentions_service.rb
@@ -29,7 +29,7 @@ class ProcessMentionsService < BaseService
       if mention_undeliverable?(mentioned_account)
         begin
           mentioned_account = resolve_account_service.call(Regexp.last_match(1))
-        rescue Goldfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::UnexpectedResponseError
+        rescue Webfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::UnexpectedResponseError
           mentioned_account = nil
         end
       end
diff --git a/app/services/resolve_account_service.rb b/app/services/resolve_account_service.rb
index ba77552c6..3f7bb7cc5 100644
--- a/app/services/resolve_account_service.rb
+++ b/app/services/resolve_account_service.rb
@@ -26,11 +26,10 @@ class ResolveAccountService < BaseService
 
     @account ||= Account.find_remote(@username, @domain)
 
-    return @account if @account&.local? || !webfinger_update_due?
+    return @account if @account&.local? || @domain.nil? || !webfinger_update_due?
 
     # At this point we are in need of a Webfinger query, which may
     # yield us a different username/domain through a redirect
-
     process_webfinger!(@uri)
 
     # Because the username/domain pair may be different than what
@@ -47,7 +46,7 @@ class ResolveAccountService < BaseService
     # either needs to be created, or updated from fresh data
 
     process_account!
-  rescue Goldfinger::Error, WebfingerRedirectError, Oj::ParseError => e
+  rescue Webfinger::Error, WebfingerRedirectError, Oj::ParseError => e
     Rails.logger.debug "Webfinger query for #{@uri} failed: #{e}"
     nil
   end
@@ -118,11 +117,11 @@ class ResolveAccountService < BaseService
   end
 
   def activitypub_ready?
-    !@webfinger.link('self').nil? && ['application/activity+json', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'].include?(@webfinger.link('self').type)
+    ['application/activity+json', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'].include?(@webfinger.link('self', 'type'))
   end
 
   def actor_url
-    @actor_url ||= @webfinger.link('self').href
+    @actor_url ||= @webfinger.link('self', 'href')
   end
 
   def actor_json
diff --git a/app/views/well_known/host_meta/show.xml.ruby b/app/views/well_known/host_meta/show.xml.ruby
index 0a6bdc322..b4e867c5f 100644
--- a/app/views/well_known/host_meta/show.xml.ruby
+++ b/app/views/well_known/host_meta/show.xml.ruby
@@ -5,7 +5,6 @@ doc << Ox::Element.new('XRD').tap do |xrd|
 
   xrd << Ox::Element.new('Link').tap do |link|
     link['rel']      = 'lrdd'
-    link['type']     = 'application/xrd+xml'
     link['template'] = @webfinger_template
   end
 end
diff --git a/spec/controllers/remote_follow_controller_spec.rb b/spec/controllers/remote_follow_controller_spec.rb
index 3ef8f14d9..7312dde58 100644
--- a/spec/controllers/remote_follow_controller_spec.rb
+++ b/spec/controllers/remote_follow_controller_spec.rb
@@ -43,8 +43,7 @@ describe RemoteFollowController do
         end
 
         it 'renders new when template is nil' do
-          link_with_nil_template = double(template: nil)
-          resource_with_link = double(link: link_with_nil_template)
+          resource_with_link = double(link: nil)
           allow_any_instance_of(WebfingerHelper).to receive(:webfinger!).with('acct:user@example.com').and_return(resource_with_link)
           post :create, params: { account_username: @account.to_param, remote_follow: { acct: 'user@example.com' } }
 
@@ -55,8 +54,7 @@ describe RemoteFollowController do
 
       context 'when webfinger values are good' do
         before do
-          link_with_template = double(template: 'http://example.com/follow_me?acct={uri}')
-          resource_with_link = double(link: link_with_template)
+          resource_with_link = double(link: 'http://example.com/follow_me?acct={uri}')
           allow_any_instance_of(WebfingerHelper).to receive(:webfinger!).with('acct:user@example.com').and_return(resource_with_link)
           post :create, params: { account_username: @account.to_param, remote_follow: { acct: 'user@example.com' } }
         end
@@ -78,8 +76,8 @@ describe RemoteFollowController do
         expect(response).to render_template(:new)
       end
 
-      it 'renders new with error when goldfinger fails' do
-        allow_any_instance_of(WebfingerHelper).to receive(:webfinger!).with('acct:user@example.com').and_raise(Goldfinger::Error)
+      it 'renders new with error when webfinger fails' do
+        allow_any_instance_of(WebfingerHelper).to receive(:webfinger!).with('acct:user@example.com').and_raise(Webfinger::Error)
         post :create, params: { account_username: @account.to_param, remote_follow: { acct: 'user@example.com' } }
 
         expect(response).to render_template(:new)
diff --git a/spec/controllers/well_known/host_meta_controller_spec.rb b/spec/controllers/well_known/host_meta_controller_spec.rb
index b43ae19d8..643ba9cd3 100644
--- a/spec/controllers/well_known/host_meta_controller_spec.rb
+++ b/spec/controllers/well_known/host_meta_controller_spec.rb
@@ -12,7 +12,7 @@ describe WellKnown::HostMetaController, type: :controller do
       expect(response.body).to eq <<XML
 <?xml version="1.0" encoding="UTF-8"?>
 <XRD xmlns="http://docs.oasis-open.org/ns/xri/xrd-1.0">
-  <Link rel="lrdd" type="application/xrd+xml" template="https://cb6e6126.ngrok.io/.well-known/webfinger?resource={uri}"/>
+  <Link rel="lrdd" template="https://cb6e6126.ngrok.io/.well-known/webfinger?resource={uri}"/>
 </XRD>
 XML
     end
diff --git a/spec/lib/feed_manager_spec.rb b/spec/lib/feed_manager_spec.rb
index d9c17470f..7d775a86d 100644
--- a/spec/lib/feed_manager_spec.rb
+++ b/spec/lib/feed_manager_spec.rb
@@ -108,6 +108,7 @@ RSpec.describe FeedManager do
 
       it 'returns false for status by followee mentioning another account' do
         bob.follow!(alice)
+        jeff.follow!(alice)
         status = PostStatusService.new.call(alice, text: 'Hey @jeff')
         expect(FeedManager.instance.filter?(:home, status, bob)).to be false
       end