From 0d9ffe56fb59e0d1fce91265f44140d874c0bfba Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Tue, 2 Jul 2019 00:34:38 +0200
Subject: Add request pool to improve delivery performance (#10353)

* Add request pool to improve delivery performance

Fix #7909

* Ensure connection is closed when exception interrupts execution

* Remove Timeout#timeout from socket connection

* Fix infinite retrial loop on HTTP::ConnectionError

* Close sockets on failure, reduce idle time to 90 seconds

* Add MAX_REQUEST_POOL_SIZE option to limit concurrent connections to the same server

* Use a shared pool size, 512 by default, to stay below open file limit

* Add some tests

* Add more tests

* Reduce MAX_IDLE_TIME from 90 to 30 seconds, reap every 30 seconds

* Use a shared pool that returns preferred connection but re-purposes other ones when needed

* Fix wrong connection being returned on subsequent calls within the same thread

* Reduce mutex calls on flushes from 2 to 1 and add test for reaping
---
 app/lib/request.rb | 70 +++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 51 insertions(+), 19 deletions(-)

(limited to 'app/lib/request.rb')

diff --git a/app/lib/request.rb b/app/lib/request.rb
index e555ae6a1..af49d6c77 100644
--- a/app/lib/request.rb
+++ b/app/lib/request.rb
@@ -17,15 +17,21 @@ end
 class Request
   REQUEST_TARGET = '(request-target)'
 
+  # We enforce a 5s timeout on DNS resolving, 5s timeout on socket opening
+  # and 5s timeout on the TLS handshake, meaning the worst case should take
+  # about 15s in total
+  TIMEOUT = { connect: 5, read: 10, write: 10 }.freeze
+
   include RoutingHelper
 
   def initialize(verb, url, **options)
     raise ArgumentError if url.blank?
 
-    @verb    = verb
-    @url     = Addressable::URI.parse(url).normalize
-    @options = options.merge(use_proxy? ? Rails.configuration.x.http_client_proxy : { socket_class: Socket })
-    @headers = {}
+    @verb        = verb
+    @url         = Addressable::URI.parse(url).normalize
+    @http_client = options.delete(:http_client)
+    @options     = options.merge(use_proxy? ? Rails.configuration.x.http_client_proxy : { socket_class: Socket })
+    @headers     = {}
 
     raise Mastodon::HostValidationError, 'Instance does not support hidden service connections' if block_hidden_service?
 
@@ -50,15 +56,24 @@ class Request
 
   def perform
     begin
-      response = http_client.headers(headers).public_send(@verb, @url.to_s, @options)
+      response = http_client.public_send(@verb, @url.to_s, @options.merge(headers: headers))
     rescue => e
       raise e.class, "#{e.message} on #{@url}", e.backtrace[0]
     end
 
     begin
-      yield response.extend(ClientLimit) if block_given?
+      response = response.extend(ClientLimit)
+
+      # If we are using a persistent connection, we have to
+      # read every response to be able to move forward at all.
+      # However, simply calling #to_s or #flush may not be safe,
+      # as the response body, if malicious, could be too big
+      # for our memory. So we use the #body_with_limit method
+      response.body_with_limit if http_client.persistent?
+
+      yield response if block_given?
     ensure
-      http_client.close
+      http_client.close unless http_client.persistent?
     end
   end
 
@@ -76,6 +91,10 @@ class Request
 
       %w(http https).include?(parsed_url.scheme) && parsed_url.host.present?
     end
+
+    def http_client
+      HTTP.use(:auto_inflate).timeout(:per_operation, TIMEOUT.dup).follow(max_hops: 2)
+    end
   end
 
   private
@@ -116,16 +135,8 @@ class Request
     end
   end
 
-  def timeout
-    # We enforce a 1s timeout on DNS resolving, 10s timeout on socket opening
-    # and 5s timeout on the TLS handshake, meaning the worst case should take
-    # about 16s in total
-
-    { connect: 5, read: 10, write: 10 }
-  end
-
   def http_client
-    @http_client ||= HTTP.use(:auto_inflate).timeout(:per_operation, timeout).follow(max_hops: 2)
+    @http_client ||= Request.http_client
   end
 
   def use_proxy?
@@ -169,20 +180,41 @@ class Request
         return super(host, *args) if thru_hidden_service?(host)
 
         outer_e = nil
+        port    = args.first
 
         Resolv::DNS.open do |dns|
           dns.timeouts = 5
 
           addresses = dns.getaddresses(host).take(2)
-          time_slot = 10.0 / addresses.size
 
           addresses.each do |address|
             begin
               raise Mastodon::HostValidationError if PrivateAddressCheck.private_address?(IPAddr.new(address.to_s))
 
-              ::Timeout.timeout(time_slot, HTTP::TimeoutError) do
-                return super(address.to_s, *args)
+              sock     = ::Socket.new(::Socket::AF_INET, ::Socket::SOCK_STREAM, 0)
+              sockaddr = ::Socket.pack_sockaddr_in(port, address.to_s)
+
+              sock.setsockopt(::Socket::IPPROTO_TCP, ::Socket::TCP_NODELAY, 1)
+
+              begin
+                sock.connect_nonblock(sockaddr)
+              rescue IO::WaitWritable
+                if IO.select(nil, [sock], nil, Request::TIMEOUT[:connect])
+                  begin
+                    sock.connect_nonblock(sockaddr)
+                  rescue Errno::EISCONN
+                    # Yippee!
+                  rescue
+                    sock.close
+                    raise
+                  end
+                else
+                  sock.close
+                  raise HTTP::TimeoutError, "Connect timed out after #{Request::TIMEOUT[:connect]} seconds"
+                end
               end
+
+              return sock
             rescue => e
               outer_e = e
             end
-- 
cgit 


From ed544f3166a2a726a22736ee53d43c29d0f44773 Mon Sep 17 00:00:00 2001
From: ThibG <thib@sitedethib.com>
Date: Fri, 5 Jul 2019 22:03:00 +0200
Subject: Fix HTTP requests to IPv6 hosts (#11240)

---
 app/lib/request.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'app/lib/request.rb')

diff --git a/app/lib/request.rb b/app/lib/request.rb
index af49d6c77..e25b9026c 100644
--- a/app/lib/request.rb
+++ b/app/lib/request.rb
@@ -191,7 +191,7 @@ class Request
             begin
               raise Mastodon::HostValidationError if PrivateAddressCheck.private_address?(IPAddr.new(address.to_s))
 
-              sock     = ::Socket.new(::Socket::AF_INET, ::Socket::SOCK_STREAM, 0)
+              sock     = ::Socket.new(address.is_a?(Resolv::IPv6) ? ::Socket::AF_INET6 : ::Socket::AF_INET, ::Socket::SOCK_STREAM, 0)
               sockaddr = ::Socket.pack_sockaddr_in(port, address.to_s)
 
               sock.setsockopt(::Socket::IPPROTO_TCP, ::Socket::TCP_NODELAY, 1)
-- 
cgit 


From 58276715be8a7e6b518ebd33cd2d4fd82ae81b2c Mon Sep 17 00:00:00 2001
From: ThibG <thib@sitedethib.com>
Date: Sun, 7 Jul 2019 02:05:38 +0200
Subject: Fix support for HTTP proxies (#11245)

* Disable incorrect check for hidden services in Socket

Hidden services can only be accessed with an HTTP proxy, in which
case the host seen by the Socket class will be the proxy, not the
target host.

Hidden services are already filtered in `Request#initialize`.

* Use our Socket class to connect to HTTP proxies

Avoid the timeout logic being bypassed

* Add support for IP addresses in Request::Socket

* Refactor a bit, no need to keep the DNS resolver around
---
 app/lib/request.rb | 83 +++++++++++++++++++++++++++++++-----------------------
 1 file changed, 48 insertions(+), 35 deletions(-)

(limited to 'app/lib/request.rb')

diff --git a/app/lib/request.rb b/app/lib/request.rb
index e25b9026c..5f7075a3c 100644
--- a/app/lib/request.rb
+++ b/app/lib/request.rb
@@ -30,7 +30,8 @@ class Request
     @verb        = verb
     @url         = Addressable::URI.parse(url).normalize
     @http_client = options.delete(:http_client)
-    @options     = options.merge(use_proxy? ? Rails.configuration.x.http_client_proxy : { socket_class: Socket })
+    @options     = options.merge(socket_class: use_proxy? ? ProxySocket : Socket)
+    @options     = @options.merge(Rails.configuration.x.http_client_proxy) if use_proxy?
     @headers     = {}
 
     raise Mastodon::HostValidationError, 'Instance does not support hidden service connections' if block_hidden_service?
@@ -177,47 +178,49 @@ class Request
   class Socket < TCPSocket
     class << self
       def open(host, *args)
-        return super(host, *args) if thru_hidden_service?(host)
-
         outer_e = nil
         port    = args.first
 
-        Resolv::DNS.open do |dns|
-          dns.timeouts = 5
+        addresses = []
+        begin
+          addresses = [IPAddr.new(host)]
+        rescue IPAddr::InvalidAddressError
+          Resolv::DNS.open do |dns|
+            dns.timeouts = 5
+            addresses = dns.getaddresses(host).take(2)
+          end
+        end
 
-          addresses = dns.getaddresses(host).take(2)
+        addresses.each do |address|
+          begin
+            check_private_address(address)
+
+            sock     = ::Socket.new(address.is_a?(Resolv::IPv6) ? ::Socket::AF_INET6 : ::Socket::AF_INET, ::Socket::SOCK_STREAM, 0)
+            sockaddr = ::Socket.pack_sockaddr_in(port, address.to_s)
+
+            sock.setsockopt(::Socket::IPPROTO_TCP, ::Socket::TCP_NODELAY, 1)
 
-          addresses.each do |address|
             begin
-              raise Mastodon::HostValidationError if PrivateAddressCheck.private_address?(IPAddr.new(address.to_s))
-
-              sock     = ::Socket.new(address.is_a?(Resolv::IPv6) ? ::Socket::AF_INET6 : ::Socket::AF_INET, ::Socket::SOCK_STREAM, 0)
-              sockaddr = ::Socket.pack_sockaddr_in(port, address.to_s)
-
-              sock.setsockopt(::Socket::IPPROTO_TCP, ::Socket::TCP_NODELAY, 1)
-
-              begin
-                sock.connect_nonblock(sockaddr)
-              rescue IO::WaitWritable
-                if IO.select(nil, [sock], nil, Request::TIMEOUT[:connect])
-                  begin
-                    sock.connect_nonblock(sockaddr)
-                  rescue Errno::EISCONN
-                    # Yippee!
-                  rescue
-                    sock.close
-                    raise
-                  end
-                else
+              sock.connect_nonblock(sockaddr)
+            rescue IO::WaitWritable
+              if IO.select(nil, [sock], nil, Request::TIMEOUT[:connect])
+                begin
+                  sock.connect_nonblock(sockaddr)
+                rescue Errno::EISCONN
+                  # Yippee!
+                rescue
                   sock.close
-                  raise HTTP::TimeoutError, "Connect timed out after #{Request::TIMEOUT[:connect]} seconds"
+                  raise
                 end
+              else
+                sock.close
+                raise HTTP::TimeoutError, "Connect timed out after #{Request::TIMEOUT[:connect]} seconds"
               end
-
-              return sock
-            rescue => e
-              outer_e = e
             end
+
+            return sock
+          rescue => e
+            outer_e = e
           end
         end
 
@@ -230,11 +233,21 @@ class Request
 
       alias new open
 
-      def thru_hidden_service?(host)
-        Rails.configuration.x.access_to_hidden_service && /\.(onion|i2p)$/.match(host)
+      def check_private_address(address)
+        raise Mastodon::HostValidationError if PrivateAddressCheck.private_address?(IPAddr.new(address.to_s))
+      end
+    end
+  end
+
+  class ProxySocket < Socket
+    class << self
+      def check_private_address(_address)
+        # Accept connections to private addresses as HTTP proxies will usually
+        # be on local addresses
+        nil
       end
     end
   end
 
-  private_constant :ClientLimit, :Socket
+  private_constant :ClientLimit, :Socket, :ProxySocket
 end
-- 
cgit 


From 406b46395d6f79e87b286585f6b6867374d198c1 Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Sun, 7 Jul 2019 03:37:01 +0200
Subject: Fix URLs appearing twice in errors of ActivityPub::DeliveryWorker
 (#11231)

---
 app/lib/request.rb                         |  2 +-
 app/workers/activitypub/delivery_worker.rb | 14 +++++++++-----
 2 files changed, 10 insertions(+), 6 deletions(-)

(limited to 'app/lib/request.rb')

diff --git a/app/lib/request.rb b/app/lib/request.rb
index 5f7075a3c..322457ad7 100644
--- a/app/lib/request.rb
+++ b/app/lib/request.rb
@@ -59,7 +59,7 @@ class Request
     begin
       response = http_client.public_send(@verb, @url.to_s, @options.merge(headers: headers))
     rescue => e
-      raise e.class, "#{e.message} on #{@url}", e.backtrace[0]
+      raise e.class, "#{e.message} on #{@url}", e.backtrace
     end
 
     begin
diff --git a/app/workers/activitypub/delivery_worker.rb b/app/workers/activitypub/delivery_worker.rb
index 818fd8f5d..8b52b8e49 100644
--- a/app/workers/activitypub/delivery_worker.rb
+++ b/app/workers/activitypub/delivery_worker.rb
@@ -18,13 +18,15 @@ class ActivityPub::DeliveryWorker
     @source_account = Account.find(source_account_id)
     @inbox_url      = inbox_url
     @host           = Addressable::URI.parse(inbox_url).normalized_site
+    @performed      = false
 
     perform_request
-
-    failure_tracker.track_success!
-  rescue => e
-    failure_tracker.track_failure!
-    raise e.class, "Delivery failed for #{inbox_url}: #{e.message}", e.backtrace[0]
+  ensure
+    if @performed
+      failure_tracker.track_success!
+    else
+      failure_tracker.track_failure!
+    end
   end
 
   private
@@ -40,6 +42,8 @@ class ActivityPub::DeliveryWorker
       request_pool.with(@host) do |http_client|
         build_request(http_client).perform do |response|
           raise Mastodon::UnexpectedResponseError, response unless response_successful?(response) || response_error_unsalvageable?(response)
+
+          @performed = true
         end
       end
     end
-- 
cgit 


From 5d3feed191bcbe2769512119752b426108152fe9 Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Wed, 10 Jul 2019 18:59:28 +0200
Subject: Refactor fetching of remote resources (#11251)

---
 app/helpers/jsonld_helper.rb                       | 47 ++++++-----
 app/lib/request.rb                                 |  2 +-
 .../activitypub/fetch_remote_status_service.rb     | 20 ++---
 app/services/fetch_atom_service.rb                 | 93 ---------------------
 app/services/fetch_link_card_service.rb            |  2 +-
 app/services/fetch_remote_account_service.rb       |  2 +-
 app/services/fetch_remote_status_service.rb        |  2 +-
 app/services/fetch_resource_service.rb             | 68 +++++++++++++++
 app/services/resolve_url_service.rb                | 47 ++++-------
 app/workers/activitypub/delivery_worker.rb         | 16 ++--
 spec/services/fetch_atom_service_spec.rb           | 96 ----------------------
 spec/services/fetch_remote_account_service_spec.rb |  1 +
 spec/services/fetch_resource_service_spec.rb       | 96 ++++++++++++++++++++++
 spec/services/resolve_url_service_spec.rb          | 44 ++--------
 14 files changed, 231 insertions(+), 305 deletions(-)
 delete mode 100644 app/services/fetch_atom_service.rb
 create mode 100644 app/services/fetch_resource_service.rb
 delete mode 100644 spec/services/fetch_atom_service_spec.rb
 create mode 100644 spec/services/fetch_resource_service_spec.rb

(limited to 'app/lib/request.rb')

diff --git a/app/helpers/jsonld_helper.rb b/app/helpers/jsonld_helper.rb
index 5b4011275..34a657e06 100644
--- a/app/helpers/jsonld_helper.rb
+++ b/app/helpers/jsonld_helper.rb
@@ -16,13 +16,15 @@ module JsonLdHelper
   # The url attribute can be a string, an array of strings, or an array of objects.
   # The objects could include a mimeType. Not-included mimeType means it's text/html.
   def url_to_href(value, preferred_type = nil)
-    single_value = if value.is_a?(Array) && !value.first.is_a?(String)
-                     value.find { |link| preferred_type.nil? || ((link['mimeType'].presence || 'text/html') == preferred_type) }
-                   elsif value.is_a?(Array)
-                     value.first
-                   else
-                     value
-                   end
+    single_value = begin
+      if value.is_a?(Array) && !value.first.is_a?(String)
+        value.find { |link| preferred_type.nil? || ((link['mimeType'].presence || 'text/html') == preferred_type) }
+      elsif value.is_a?(Array)
+        value.first
+      else
+        value
+      end
+    end
 
     if single_value.nil? || single_value.is_a?(String)
       single_value
@@ -64,7 +66,9 @@ module JsonLdHelper
   def fetch_resource(uri, id, on_behalf_of = nil)
     unless id
       json = fetch_resource_without_id_validation(uri, on_behalf_of)
+
       return unless json
+
       uri = json['id']
     end
 
@@ -74,24 +78,26 @@ module JsonLdHelper
 
   def fetch_resource_without_id_validation(uri, on_behalf_of = nil, raise_on_temporary_error = false)
     build_request(uri, on_behalf_of).perform do |response|
-      unless response_successful?(response) || response_error_unsalvageable?(response) || !raise_on_temporary_error
-        raise Mastodon::UnexpectedResponseError, response
-      end
+      raise Mastodon::UnexpectedResponseError, response unless response_successful?(response) || response_error_unsalvageable?(response) || !raise_on_temporary_error
+
       return body_to_json(response.body_with_limit) if response.code == 200
     end
+
     # If request failed, retry without doing it on behalf of a user
     return if on_behalf_of.nil?
+
     build_request(uri).perform do |response|
-      unless response_successful?(response) || response_error_unsalvageable?(response) || !raise_on_temporary_error
-        raise Mastodon::UnexpectedResponseError, response
-      end
+      raise Mastodon::UnexpectedResponseError, response unless response_successful?(response) || response_error_unsalvageable?(response) || !raise_on_temporary_error
+
       response.code == 200 ? body_to_json(response.body_with_limit) : nil
     end
   end
 
   def body_to_json(body, compare_id: nil)
     json = body.is_a?(String) ? Oj.load(body, mode: :strict) : body
+
     return if compare_id.present? && json['id'] != compare_id
+
     json
   rescue Oj::ParseError
     nil
@@ -105,35 +111,34 @@ module JsonLdHelper
     end
   end
 
-  private
-
   def response_successful?(response)
     (200...300).cover?(response.code)
   end
 
   def response_error_unsalvageable?(response)
-    (400...500).cover?(response.code) && response.code != 429
+    response.code == 501 || ((400...500).cover?(response.code) && ![401, 408, 429].include?(response.code))
   end
 
   def build_request(uri, on_behalf_of = nil)
-    request = Request.new(:get, uri)
-    request.on_behalf_of(on_behalf_of) if on_behalf_of
-    request.add_headers('Accept' => 'application/activity+json, application/ld+json')
-    request
+    Request.new(:get, uri).tap do |request|
+      request.on_behalf_of(on_behalf_of) if on_behalf_of
+      request.add_headers('Accept' => 'application/activity+json, application/ld+json')
+    end
   end
 
   def load_jsonld_context(url, _options = {}, &_block)
     json = Rails.cache.fetch("jsonld:context:#{url}", expires_in: 30.days, raw: true) do
       request = Request.new(:get, url)
       request.add_headers('Accept' => 'application/ld+json')
-
       request.perform do |res|
         raise JSON::LD::JsonLdError::LoadingDocumentFailed unless res.code == 200 && res.mime_type == 'application/ld+json'
+
         res.body_with_limit
       end
     end
 
     doc = JSON::LD::API::RemoteDocument.new(url, json)
+
     block_given? ? yield(doc) : doc
   end
 end
diff --git a/app/lib/request.rb b/app/lib/request.rb
index 322457ad7..1fd3f5190 100644
--- a/app/lib/request.rb
+++ b/app/lib/request.rb
@@ -41,7 +41,7 @@ class Request
   end
 
   def on_behalf_of(account, key_id_format = :acct, sign_with: nil)
-    raise ArgumentError unless account.local?
+    raise ArgumentError, 'account must be local' unless account&.local?
 
     @account       = account
     @keypair       = sign_with.present? ? OpenSSL::PKey::RSA.new(sign_with) : @account.keypair
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
diff --git a/app/workers/activitypub/delivery_worker.rb b/app/workers/activitypub/delivery_worker.rb
index 8b52b8e49..5457d9d4b 100644
--- a/app/workers/activitypub/delivery_worker.rb
+++ b/app/workers/activitypub/delivery_worker.rb
@@ -2,6 +2,7 @@
 
 class ActivityPub::DeliveryWorker
   include Sidekiq::Worker
+  include JsonLdHelper
 
   STOPLIGHT_FAILURE_THRESHOLD = 10
   STOPLIGHT_COOLDOWN = 60
@@ -32,9 +33,10 @@ class ActivityPub::DeliveryWorker
   private
 
   def build_request(http_client)
-    request = Request.new(:post, @inbox_url, body: @json, http_client: http_client)
-    request.on_behalf_of(@source_account, :uri, sign_with: @options[:sign_with])
-    request.add_headers(HEADERS)
+    Request.new(:post, @inbox_url, body: @json, http_client: http_client).tap do |request|
+      request.on_behalf_of(@source_account, :uri, sign_with: @options[:sign_with])
+      request.add_headers(HEADERS)
+    end
   end
 
   def perform_request
@@ -53,14 +55,6 @@ class ActivityPub::DeliveryWorker
          .run
   end
 
-  def response_successful?(response)
-    (200...300).cover?(response.code)
-  end
-
-  def response_error_unsalvageable?(response)
-    response.code == 501 || ((400...500).cover?(response.code) && ![401, 408, 429].include?(response.code))
-  end
-
   def failure_tracker
     @failure_tracker ||= DeliveryFailureTracker.new(@inbox_url)
   end
diff --git a/spec/services/fetch_atom_service_spec.rb b/spec/services/fetch_atom_service_spec.rb
deleted file mode 100644
index 495540004..000000000
--- a/spec/services/fetch_atom_service_spec.rb
+++ /dev/null
@@ -1,96 +0,0 @@
-require 'rails_helper'
-
-RSpec.describe FetchAtomService, type: :service do
-  describe '#call' do
-    let(:url) { 'http://example.com' }
-    subject { FetchAtomService.new.call(url) }
-
-    context 'url is blank' do
-      let(:url) { '' }
-      it { is_expected.to be_nil }
-    end
-
-    context 'request failed' do
-      before do
-        WebMock.stub_request(:get, url).to_return(status: 500, body: '', headers: {})
-      end
-
-      it { is_expected.to be_nil }
-    end
-
-    context 'raise OpenSSL::SSL::SSLError' do
-      before do
-        allow(Request).to receive_message_chain(:new, :add_headers, :perform).and_raise(OpenSSL::SSL::SSLError)
-      end
-
-      it 'output log and return nil' do
-        expect_any_instance_of(ActiveSupport::Logger).to receive(:debug).with('SSL error: OpenSSL::SSL::SSLError')
-        is_expected.to be_nil
-      end
-    end
-
-    context 'raise HTTP::ConnectionError' do
-      before do
-        allow(Request).to receive_message_chain(:new, :add_headers, :perform).and_raise(HTTP::ConnectionError)
-      end
-
-      it 'output log and return nil' do
-        expect_any_instance_of(ActiveSupport::Logger).to receive(:debug).with('HTTP ConnectionError: HTTP::ConnectionError')
-        is_expected.to be_nil
-      end
-    end
-
-    context 'response success' do
-      let(:body) { '' }
-      let(:headers) { { 'Content-Type' => content_type } }
-      let(:json) {
-        { id: 1,
-          '@context': ActivityPub::TagManager::CONTEXT,
-          type: 'Note',
-        }.to_json
-      }
-
-      before do
-        WebMock.stub_request(:get, url).to_return(status: 200, body: body, headers: headers)
-      end
-
-      context 'content type is application/atom+xml' do
-        let(:content_type) { 'application/atom+xml' }
-
-        it { is_expected.to eq [url, { :prefetched_body => "" }, :ostatus] }
-      end
-
-      context 'content_type is activity+json' do
-        let(:content_type) { 'application/activity+json; charset=utf-8' }
-        let(:body) { json }
-
-        it { is_expected.to eq [1, { prefetched_body: body, id: true }, :activitypub] }
-      end
-
-      context 'content_type is ld+json with profile' do
-        let(:content_type) { 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"' }
-        let(:body) { json }
-
-        it { is_expected.to eq [1, { prefetched_body: body, id: true }, :activitypub] }
-      end
-
-      before do
-        WebMock.stub_request(:get, url).to_return(status: 200, body: body, headers: headers)
-        WebMock.stub_request(:get, 'http://example.com/foo').to_return(status: 200, body: json, headers: { 'Content-Type' => 'application/activity+json' })
-      end
-
-      context 'has link header' do
-        let(:headers) { { 'Link' => '<http://example.com/foo>; rel="alternate"; type="application/activity+json"', } }
-
-        it { is_expected.to eq [1, { prefetched_body: json, id: true }, :activitypub] }
-      end
-
-      context 'content type is text/html' do
-        let(:content_type) { 'text/html' }
-        let(:body) { '<html><head><link rel="alternate" href="http://example.com/foo" type="application/activity+json"/></head></html>' }
-
-        it { is_expected.to eq [1, { prefetched_body: json, id: true }, :activitypub] }
-      end
-    end
-  end
-end
diff --git a/spec/services/fetch_remote_account_service_spec.rb b/spec/services/fetch_remote_account_service_spec.rb
index 37e9910d4..ee7325be2 100644
--- a/spec/services/fetch_remote_account_service_spec.rb
+++ b/spec/services/fetch_remote_account_service_spec.rb
@@ -4,6 +4,7 @@ RSpec.describe FetchRemoteAccountService, type: :service do
   let(:url) { 'https://example.com/alice' }
   let(:prefetched_body) { nil }
   let(:protocol) { :ostatus }
+
   subject { FetchRemoteAccountService.new.call(url, prefetched_body, protocol) }
 
   let(:actor) do
diff --git a/spec/services/fetch_resource_service_spec.rb b/spec/services/fetch_resource_service_spec.rb
new file mode 100644
index 000000000..17c192c44
--- /dev/null
+++ b/spec/services/fetch_resource_service_spec.rb
@@ -0,0 +1,96 @@
+require 'rails_helper'
+
+RSpec.describe FetchResourceService, type: :service do
+  let!(:representative) { Fabricate(:account) }
+
+  describe '#call' do
+    let(:url) { 'http://example.com' }
+    subject { described_class.new.call(url) }
+
+    context 'url is blank' do
+      let(:url) { '' }
+      it { is_expected.to be_nil }
+    end
+
+    context 'request failed' do
+      before do
+        WebMock.stub_request(:get, url).to_return(status: 500, body: '', headers: {})
+      end
+
+      it { is_expected.to be_nil }
+    end
+
+    context 'raise OpenSSL::SSL::SSLError' do
+      before do
+        allow(Request).to receive_message_chain(:new, :add_headers, :perform).and_raise(OpenSSL::SSL::SSLError)
+      end
+
+      it 'return nil' do
+        is_expected.to be_nil
+      end
+    end
+
+    context 'raise HTTP::ConnectionError' do
+      before do
+        allow(Request).to receive_message_chain(:new, :add_headers, :perform).and_raise(HTTP::ConnectionError)
+      end
+
+      it 'return nil' do
+        is_expected.to be_nil
+      end
+    end
+
+    context 'response success' do
+      let(:body) { '' }
+      let(:headers) { { 'Content-Type' => content_type } }
+      let(:json) {
+        { id: 1,
+          '@context': ActivityPub::TagManager::CONTEXT,
+          type: 'Note',
+        }.to_json
+      }
+
+      before do
+        WebMock.stub_request(:get, url).to_return(status: 200, body: body, headers: headers)
+      end
+
+      context 'content type is application/atom+xml' do
+        let(:content_type) { 'application/atom+xml' }
+
+        it { is_expected.to eq nil }
+      end
+
+      context 'content_type is activity+json' do
+        let(:content_type) { 'application/activity+json; charset=utf-8' }
+        let(:body) { json }
+
+        it { is_expected.to eq [1, { prefetched_body: body, id: true }, :activitypub] }
+      end
+
+      context 'content_type is ld+json with profile' do
+        let(:content_type) { 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"' }
+        let(:body) { json }
+
+        it { is_expected.to eq [1, { prefetched_body: body, id: true }, :activitypub] }
+      end
+
+      before do
+        WebMock.stub_request(:get, url).to_return(status: 200, body: body, headers: headers)
+        WebMock.stub_request(:get, 'http://example.com/foo').to_return(status: 200, body: json, headers: { 'Content-Type' => 'application/activity+json' })
+      end
+
+      context 'has link header' do
+        let(:headers) { { 'Link' => '<http://example.com/foo>; rel="alternate"; type="application/activity+json"', } }
+
+        it { is_expected.to eq [1, { prefetched_body: json, id: true }, :activitypub] }
+      end
+
+      context 'content type is text/html' do
+        let(:content_type) { 'text/html' }
+        let(:body) { '<html><head><link rel="alternate" href="http://example.com/foo" type="application/activity+json"/></head></html>' }
+
+        it { is_expected.to eq [1, { prefetched_body: json, id: true }, :activitypub] }
+      end
+    end
+  end
+end
diff --git a/spec/services/resolve_url_service_spec.rb b/spec/services/resolve_url_service_spec.rb
index 7bb5d1940..aa4204637 100644
--- a/spec/services/resolve_url_service_spec.rb
+++ b/spec/services/resolve_url_service_spec.rb
@@ -6,48 +6,14 @@ describe ResolveURLService, type: :service do
   subject { described_class.new }
 
   describe '#call' do
-    it 'returns nil when there is no atom url' do
-      url = 'http://example.com/missing-atom'
+    it 'returns nil when there is no resource url' do
+      url     = 'http://example.com/missing-resource'
       service = double
-      allow(FetchAtomService).to receive(:new).and_return service
-      allow(service).to receive(:call).with(url).and_return(nil)
-
-      result = subject.call(url)
-      expect(result).to be_nil
-    end
-
-    it 'fetches remote accounts for feed types' do
-      url = 'http://example.com/atom-feed'
-      service = double
-      allow(FetchAtomService).to receive(:new).and_return service
-      feed_url = 'http://feed-url'
-      feed_content = '<feed>contents</feed>'
-      allow(service).to receive(:call).with(url).and_return([feed_url, { prefetched_body: feed_content }])
-
-      account_service = double
-      allow(FetchRemoteAccountService).to receive(:new).and_return(account_service)
-      allow(account_service).to receive(:call)
-
-      _result = subject.call(url)
 
-      expect(account_service).to have_received(:call).with(feed_url, feed_content, nil)
-    end
-
-    it 'fetches remote statuses for entry types' do
-      url = 'http://example.com/atom-entry'
-      service = double
-      allow(FetchAtomService).to receive(:new).and_return service
-      feed_url = 'http://feed-url'
-      feed_content = '<entry>contents</entry>'
-      allow(service).to receive(:call).with(url).and_return([feed_url, { prefetched_body: feed_content }])
-
-      account_service = double
-      allow(FetchRemoteStatusService).to receive(:new).and_return(account_service)
-      allow(account_service).to receive(:call)
-
-      _result = subject.call(url)
+      allow(FetchResourceService).to receive(:new).and_return service
+      allow(service).to receive(:call).with(url).and_return(nil)
 
-      expect(account_service).to have_received(:call).with(feed_url, feed_content, nil)
+      expect(subject.call(url)).to be_nil
     end
   end
 end
-- 
cgit 


From 4e8dcc5dbbf625b7268ed10d36122de985da6bdc Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Thu, 11 Jul 2019 14:49:55 +0200
Subject: Add HTTP signatures to all outgoing ActivityPub GET requests (#11284)

---
 app/helpers/jsonld_helper.rb                       | 13 ++---
 app/lib/request.rb                                 |  4 +-
 app/services/fetch_resource_service.rb             |  2 +-
 .../concerns/signature_verification_spec.rb        |  2 +-
 spec/services/fetch_remote_account_service_spec.rb |  1 +
 spec/services/fetch_resource_service_spec.rb       | 61 +++++++++++++---------
 6 files changed, 43 insertions(+), 40 deletions(-)

(limited to 'app/lib/request.rb')

diff --git a/app/helpers/jsonld_helper.rb b/app/helpers/jsonld_helper.rb
index 34a657e06..83a5b2462 100644
--- a/app/helpers/jsonld_helper.rb
+++ b/app/helpers/jsonld_helper.rb
@@ -77,19 +77,12 @@ module JsonLdHelper
   end
 
   def fetch_resource_without_id_validation(uri, on_behalf_of = nil, raise_on_temporary_error = false)
-    build_request(uri, on_behalf_of).perform do |response|
-      raise Mastodon::UnexpectedResponseError, response unless response_successful?(response) || response_error_unsalvageable?(response) || !raise_on_temporary_error
-
-      return body_to_json(response.body_with_limit) if response.code == 200
-    end
-
-    # If request failed, retry without doing it on behalf of a user
-    return if on_behalf_of.nil?
+    on_behalf_of ||= Account.representative
 
-    build_request(uri).perform do |response|
+    build_request(uri, on_behalf_of).perform do |response|
       raise Mastodon::UnexpectedResponseError, response unless response_successful?(response) || response_error_unsalvageable?(response) || !raise_on_temporary_error
 
-      response.code == 200 ? body_to_json(response.body_with_limit) : nil
+      body_to_json(response.body_with_limit) if response.code == 200
     end
   end
 
diff --git a/app/lib/request.rb b/app/lib/request.rb
index 1fd3f5190..9d874fe2c 100644
--- a/app/lib/request.rb
+++ b/app/lib/request.rb
@@ -40,8 +40,8 @@ class Request
     set_digest! if options.key?(:body)
   end
 
-  def on_behalf_of(account, key_id_format = :acct, sign_with: nil)
-    raise ArgumentError, 'account must be local' unless account&.local?
+  def on_behalf_of(account, key_id_format = :uri, sign_with: nil)
+    raise ArgumentError, 'account must not be nil' if account.nil?
 
     @account       = account
     @keypair       = sign_with.present? ? OpenSSL::PKey::RSA.new(sign_with) : @account.keypair
diff --git a/app/services/fetch_resource_service.rb b/app/services/fetch_resource_service.rb
index c0473f3ad..3676d899d 100644
--- a/app/services/fetch_resource_service.rb
+++ b/app/services/fetch_resource_service.rb
@@ -23,7 +23,7 @@ class FetchResourceService < BaseService
   end
 
   def perform_request(&block)
-    Request.new(:get, @url).add_headers('Accept' => ACCEPT_HEADER).perform(&block)
+    Request.new(:get, @url).add_headers('Accept' => ACCEPT_HEADER).on_behalf_of(Account.representative).perform(&block)
   end
 
   def process_response(response, terminal = false)
diff --git a/spec/controllers/concerns/signature_verification_spec.rb b/spec/controllers/concerns/signature_verification_spec.rb
index 720690097..1fa19f54d 100644
--- a/spec/controllers/concerns/signature_verification_spec.rb
+++ b/spec/controllers/concerns/signature_verification_spec.rb
@@ -38,7 +38,7 @@ describe ApplicationController, type: :controller do
   end
 
   context 'with signature header' do
-    let!(:author) { Fabricate(:account) }
+    let!(:author) { Fabricate(:account, domain: 'example.com', uri: 'https://example.com/actor') }
 
     context 'without body' do
       before do
diff --git a/spec/services/fetch_remote_account_service_spec.rb b/spec/services/fetch_remote_account_service_spec.rb
index ee7325be2..b37445861 100644
--- a/spec/services/fetch_remote_account_service_spec.rb
+++ b/spec/services/fetch_remote_account_service_spec.rb
@@ -4,6 +4,7 @@ RSpec.describe FetchRemoteAccountService, type: :service do
   let(:url) { 'https://example.com/alice' }
   let(:prefetched_body) { nil }
   let(:protocol) { :ostatus }
+  let!(:representative) { Fabricate(:account) }
 
   subject { FetchRemoteAccountService.new.call(url, prefetched_body, protocol) }
 
diff --git a/spec/services/fetch_resource_service_spec.rb b/spec/services/fetch_resource_service_spec.rb
index 17c192c44..98630966b 100644
--- a/spec/services/fetch_resource_service_spec.rb
+++ b/spec/services/fetch_resource_service_spec.rb
@@ -5,69 +5,78 @@ RSpec.describe FetchResourceService, type: :service do
 
   describe '#call' do
     let(:url) { 'http://example.com' }
+
     subject { described_class.new.call(url) }
 
-    context 'url is blank' do
+    context 'with blank url' do
       let(:url) { '' }
       it { is_expected.to be_nil }
     end
 
-    context 'request failed' do
+    context 'when request fails' do
       before do
-        WebMock.stub_request(:get, url).to_return(status: 500, body: '', headers: {})
+        stub_request(:get, url).to_return(status: 500, body: '', headers: {})
       end
 
       it { is_expected.to be_nil }
     end
 
-    context 'raise OpenSSL::SSL::SSLError' do
+    context 'when OpenSSL::SSL::SSLError is raised' do
       before do
-        allow(Request).to receive_message_chain(:new, :add_headers, :perform).and_raise(OpenSSL::SSL::SSLError)
+        allow(Request).to receive_message_chain(:new, :add_headers, :on_behalf_of, :perform).and_raise(OpenSSL::SSL::SSLError)
       end
 
-      it 'return nil' do
-        is_expected.to be_nil
-      end
+      it { is_expected.to be_nil }
     end
 
-    context 'raise HTTP::ConnectionError' do
+    context 'when HTTP::ConnectionError is raised' do
       before do
-        allow(Request).to receive_message_chain(:new, :add_headers, :perform).and_raise(HTTP::ConnectionError)
+        allow(Request).to receive_message_chain(:new, :add_headers, :on_behalf_of, :perform).and_raise(HTTP::ConnectionError)
       end
 
-      it 'return nil' do
-        is_expected.to be_nil
-      end
+      it { is_expected.to be_nil }
     end
 
-    context 'response success' do
+    context 'when request succeeds' do
       let(:body) { '' }
-      let(:headers) { { 'Content-Type' => content_type } }
-      let(:json) {
-        { id: 1,
+
+      let(:content_type) { 'application/json' }
+
+      let(:headers) do
+        { 'Content-Type' => content_type }
+      end
+
+      let(:json) do
+        {
+          id: 1,
           '@context': ActivityPub::TagManager::CONTEXT,
           type: 'Note',
         }.to_json
-      }
+      end
 
       before do
-        WebMock.stub_request(:get, url).to_return(status: 200, body: body, headers: headers)
+        stub_request(:get, url).to_return(status: 200, body: body, headers: headers)
+      end
+
+      it 'signs request' do
+        subject
+        expect(a_request(:get, url).with(headers: { 'Signature' => /keyId="#{Regexp.escape(ActivityPub::TagManager.instance.uri_for(representative) + '#main-key')}"/ })).to have_been_made
       end
 
-      context 'content type is application/atom+xml' do
+      context 'when content type is application/atom+xml' do
         let(:content_type) { 'application/atom+xml' }
 
         it { is_expected.to eq nil }
       end
 
-      context 'content_type is activity+json' do
+      context 'when content type is activity+json' do
         let(:content_type) { 'application/activity+json; charset=utf-8' }
         let(:body) { json }
 
         it { is_expected.to eq [1, { prefetched_body: body, id: true }, :activitypub] }
       end
 
-      context 'content_type is ld+json with profile' do
+      context 'when content type is ld+json with profile' do
         let(:content_type) { 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"' }
         let(:body) { json }
 
@@ -75,17 +84,17 @@ RSpec.describe FetchResourceService, type: :service do
       end
 
       before do
-        WebMock.stub_request(:get, url).to_return(status: 200, body: body, headers: headers)
-        WebMock.stub_request(:get, 'http://example.com/foo').to_return(status: 200, body: json, headers: { 'Content-Type' => 'application/activity+json' })
+        stub_request(:get, url).to_return(status: 200, body: body, headers: headers)
+        stub_request(:get, 'http://example.com/foo').to_return(status: 200, body: json, headers: { 'Content-Type' => 'application/activity+json' })
       end
 
-      context 'has link header' do
+      context 'when link header is present' do
         let(:headers) { { 'Link' => '<http://example.com/foo>; rel="alternate"; type="application/activity+json"', } }
 
         it { is_expected.to eq [1, { prefetched_body: json, id: true }, :activitypub] }
       end
 
-      context 'content type is text/html' do
+      context 'when content type is text/html' do
         let(:content_type) { 'text/html' }
         let(:body) { '<html><head><link rel="alternate" href="http://example.com/foo" type="application/activity+json"/></head></html>' }
 
-- 
cgit 


From 1653b587778db0df201507115d19c0530096a5e3 Mon Sep 17 00:00:00 2001
From: ThibG <thib@sitedethib.com>
Date: Thu, 5 Sep 2019 05:32:53 +0200
Subject: Attempt to concurrently connect to remote IP addresses (#11757)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Attempt to concurrently connect to remote IP addresses

* Reduce code length to please CodeClimate 🤷
---
 app/lib/request.rb | 52 +++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 17 deletions(-)

(limited to 'app/lib/request.rb')

diff --git a/app/lib/request.rb b/app/lib/request.rb
index 9d874fe2c..42ccc6513 100644
--- a/app/lib/request.rb
+++ b/app/lib/request.rb
@@ -191,6 +191,9 @@ class Request
           end
         end
 
+        socks = []
+        addr_by_socket = {}
+
         addresses.each do |address|
           begin
             check_private_address(address)
@@ -200,30 +203,45 @@ class Request
 
             sock.setsockopt(::Socket::IPPROTO_TCP, ::Socket::TCP_NODELAY, 1)
 
-            begin
-              sock.connect_nonblock(sockaddr)
-            rescue IO::WaitWritable
-              if IO.select(nil, [sock], nil, Request::TIMEOUT[:connect])
-                begin
-                  sock.connect_nonblock(sockaddr)
-                rescue Errno::EISCONN
-                  # Yippee!
-                rescue
-                  sock.close
-                  raise
-                end
-              else
-                sock.close
-                raise HTTP::TimeoutError, "Connect timed out after #{Request::TIMEOUT[:connect]} seconds"
-              end
-            end
+            sock.connect_nonblock(sockaddr)
 
+            # If that hasn't raised an exception, we somehow managed to connect
+            # immediately, close pending sockets and return immediately
+            socks.each(&:close)
             return sock
+          rescue IO::WaitWritable
+            socks << sock
+            addr_by_socket[sock] = sockaddr
           rescue => e
             outer_e = e
           end
         end
 
+        until socks.empty?
+          _, available_socks, = IO.select(nil, socks, nil, Request::TIMEOUT[:connect])
+
+          if available_socks.nil?
+            socks.each(&:close)
+            raise HTTP::TimeoutError, "Connect timed out after #{Request::TIMEOUT[:connect]} seconds"
+          end
+
+          available_socks.each do |sock|
+            socks.delete(sock)
+
+            begin
+              sock.connect_nonblock(addr_by_socket[sock])
+            rescue Errno::EISCONN
+            rescue => e
+              sock.close
+              outer_e = e
+              next
+            end
+
+            socks.each(&:close)
+            return sock
+          end
+        end
+
         if outer_e
           raise outer_e
         else
-- 
cgit