]> cat aescling's git repositories - mastodon.git/commitdiff
Remove dependency on goldfinger gem (#14919)
authorEugen Rochko <eugen@zeonfederated.com>
Wed, 7 Oct 2020 22:34:57 +0000 (00:34 +0200)
committerGitHub <noreply@github.com>
Wed, 7 Oct 2020 22:34:57 +0000 (00:34 +0200)
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:
Gemfile
Gemfile.lock
app/helpers/webfinger_helper.rb
app/lib/webfinger.rb [new file with mode: 0644]
app/models/account_alias.rb
app/models/account_migration.rb
app/models/form/redirect.rb
app/models/remote_follow.rb
app/services/activitypub/fetch_remote_account_service.rb
app/services/process_mentions_service.rb
app/services/resolve_account_service.rb
app/views/well_known/host_meta/show.xml.ruby
spec/controllers/remote_follow_controller_spec.rb
spec/controllers/well_known/host_meta_controller_spec.rb
spec/lib/feed_manager_spec.rb

diff --git a/Gemfile b/Gemfile
index 3a92b4627daaac831ec241d5b9f78133670d641e..0fc631e2acaded2092d90d89b8b7f58948cd1602 100644 (file)
--- 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'
index ae304e61e5cd800f0498129c602ac27bb13cf701..21f5b1b4cc4df40874486aa1b39bdc35c4407102 100644 (file)
@@ -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)
index ab7ca469811f12e44d87550abc9ea10ddc97a09b..482f4e19eabef0e0a1bbbd43fddf63fb1fd9ed54 100644 (file)
@@ -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 (file)
index 0000000..b2374c4
--- /dev/null
@@ -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
index 792e9e8d4da7574bd2e7acdd079fe6488d9f02fb..3d659142a05548022c60f04e72591c836cdecde3 100644 (file)
@@ -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
 
index 681b5b2cd0bc9e76d1f08e2d1a8c72dd7a26d3f3..4fae98ed7260d525ed7bddabde9990b91f252d0c 100644 (file)
@@ -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
 
index a7961f8e8aa00e7b311a3742644e3f79c89ebf61..19ee9faedd046bb420e66a93d123064cf30e8abb 100644 (file)
@@ -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
 
index 30b84f7d529574b8f6689014f80dadecaf911c6d..911c067133c35fe63a18cc789d497a495985ac6f 100644 (file)
@@ -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
 
index 83fbf6d07d32bd155883406d69e886792499e0c3..e5bd0c47c8ee5ef0b9bbf530f7e2ced0ae63ecdc 100644 (file)
@@ -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
 
index 12f0f1b0852f587cdfb7132b111c50c58538c774..0a710f8436165c435592d9e4c45c26d9da544000 100644 (file)
@@ -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
index ba77552c6c02796170cc55d04f80317bbeec4642..3f7bb7cc52c86265195d714634ef1c322cabc915 100644 (file)
@@ -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
index 0a6bdc322fb130363c190603be1e6c8e6402cce5..b4e867c5f88a7c53631639890d9cbdbb250c4103 100644 (file)
@@ -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
index 3ef8f14d9f68c36386e19627380947d856b04b47..7312dde582e92b94ac5a2440a2d8742b0c28982b 100644 (file)
@@ -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)
index b43ae19d87a2922d202cd65357e7496c0bf52c7d..643ba9cd3283aa1b1df379a428364a69395c263e 100644 (file)
@@ -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
index d9c17470f5a65a01ca539ea559bea6e87615f032..7d775a86d7be742e603ae2a45349bd41b09eb5a9 100644 (file)
@@ -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