From 8069fd636ba14059524303ce76abe2173c4f3d01 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 16 Nov 2018 15:02:18 +0100 Subject: Remove intermediary arrays when creating hash maps from results (#9291) --- app/lib/entity_cache.rb | 2 +- app/lib/formatter.rb | 4 ++-- app/lib/settings/scoped_settings.rb | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) (limited to 'app/lib') diff --git a/app/lib/entity_cache.rb b/app/lib/entity_cache.rb index 2aa37389c..8fff544a0 100644 --- a/app/lib/entity_cache.rb +++ b/app/lib/entity_cache.rb @@ -21,7 +21,7 @@ class EntityCache end unless uncached_ids.empty? - uncached = CustomEmoji.where(shortcode: shortcodes, domain: domain, disabled: false).map { |item| [item.shortcode, item] }.to_h + uncached = CustomEmoji.where(shortcode: shortcodes, domain: domain, disabled: false).each_with_object({}) { |item, h| h[item.shortcode] = item } uncached.each_value { |item| Rails.cache.write(to_key(:emoji, item.shortcode, domain), item, expires_in: MAX_EXPIRATION) } end diff --git a/app/lib/formatter.rb b/app/lib/formatter.rb index d13884ec8..05fd9eeb1 100644 --- a/app/lib/formatter.rb +++ b/app/lib/formatter.rb @@ -128,9 +128,9 @@ class Formatter return html if emojis.empty? emoji_map = if animate - emojis.map { |e| [e.shortcode, full_asset_url(e.image.url)] }.to_h + emojis.each_with_object({}) { |e, h| h[e.shortcode] = full_asset_url(e.image.url) } else - emojis.map { |e| [e.shortcode, full_asset_url(e.image.url(:static))] }.to_h + emojis.each_with_object({}) { |e, h| h[e.shortcode] = full_asset_url(e.image.url(:static)) } end i = -1 diff --git a/app/lib/settings/scoped_settings.rb b/app/lib/settings/scoped_settings.rb index 5ee30825d..3653ab114 100644 --- a/app/lib/settings/scoped_settings.rb +++ b/app/lib/settings/scoped_settings.rb @@ -31,7 +31,7 @@ module Settings def all_as_records vars = thing_scoped - records = vars.map { |r| [r.var, r] }.to_h + records = vars.each_with_object({}) { |r, h| h[r.var] = r } Setting.default_settings.each do |key, default_value| next if records.key?(key) || default_value.is_a?(Hash) @@ -65,7 +65,7 @@ module Settings class << self def default_settings - defaulting = DEFAULTING_TO_UNSCOPED.map { |k| [k, Setting[k]] }.to_h + defaulting = DEFAULTING_TO_UNSCOPED.each_with_object({}) { |k, h| h[k] = Setting[k] } Setting.default_settings.merge!(defaulting) end end -- cgit From 9311430ed7022e2e02fbd0b453adfc43356fdf9d Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 16 Nov 2018 19:46:23 +0100 Subject: Prevent multiple handlers for Delete of Actor from running (#9292) --- app/lib/activitypub/activity.rb | 6 ++++++ app/lib/activitypub/activity/delete.rb | 6 ++++-- 2 files changed, 10 insertions(+), 2 deletions(-) (limited to 'app/lib') diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb index 999954cb5..0a729011f 100644 --- a/app/lib/activitypub/activity.rb +++ b/app/lib/activitypub/activity.rb @@ -129,4 +129,10 @@ class ActivityPub::Activity ::FetchRemoteStatusService.new.call(@object['url']) end end + + def lock_or_return(key, expire_after = 7.days.seconds) + yield if redis.set(key, true, nx: true, ex: expire_after) + ensure + redis.del(key) + end end diff --git a/app/lib/activitypub/activity/delete.rb b/app/lib/activitypub/activity/delete.rb index 457047ac0..8270fed1b 100644 --- a/app/lib/activitypub/activity/delete.rb +++ b/app/lib/activitypub/activity/delete.rb @@ -12,8 +12,10 @@ class ActivityPub::Activity::Delete < ActivityPub::Activity private def delete_person - SuspendAccountService.new.call(@account) - @account.destroy! + lock_or_return("delete_in_progress:#{@account.id}") do + SuspendAccountService.new.call(@account) + @account.destroy! + end end def delete_note -- cgit From 384e953b75abdb15333bf3053cc3d27be88a0cef Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 21 Nov 2018 17:00:56 +0100 Subject: Revert connect timeout from 1s to 10s (#9319) The failure rate in Sidekiq is too high --- app/lib/request.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/lib') diff --git a/app/lib/request.rb b/app/lib/request.rb index 73b495ce1..fdaaf3636 100644 --- a/app/lib/request.rb +++ b/app/lib/request.rb @@ -94,7 +94,7 @@ class Request end def timeout - { connect: 1, read: 10, write: 10 } + { connect: 10, read: 10, write: 10 } end def http_client -- cgit From 466e3d710ce216030502c6a6b0dbd93becd13d02 Mon Sep 17 00:00:00 2001 From: ThibG Date: Wed, 21 Nov 2018 17:02:58 +0100 Subject: Include replies to list owner and replies to list members in list statuses (#9324) --- app/lib/feed_manager.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'app/lib') diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index 3d7db2721..31ff53860 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -40,7 +40,11 @@ class FeedManager end def push_to_list(list, status) - return false if status.reply? && status.in_reply_to_account_id != status.account_id + if status.reply? && status.in_reply_to_account_id != status.account_id + should_filter = status.in_reply_to_account_id != list.account_id + should_filter &&= !ListAccount.where(list_id: list.id, account_id: status.in_reply_to_account_id).exists? + return false if should_filter + end return false unless add_to_feed(:list, list.id, status) trim(:list, list.id) PushUpdateWorker.perform_async(list.account_id, status.id, "timeline:list:#{list.id}") if push_update_required?("timeline:list:#{list.id}") -- cgit From fd8145d2322d8b13b6aaa9cd2a61331ff660c9dd Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 22 Nov 2018 20:12:04 +0100 Subject: Fix connect timeout not being enforced (#9329) * Fix connect timeout not being enforced The loop was catching the timeout exception that should stop execution, so the next IP would no longer be within a timed block, which led to requests taking much longer than 10 seconds. * Use timeout on each IP attempt, but limit to 2 attempts * Fix code style issue * Do not break Request#perform if no block given * Update method stub in spec for Request * Move timeout inside the begin/rescue block * Use Resolv::DNS with timeout of 1 to get IP addresses * Update Request spec to stub Resolv::DNS instead of Addrinfo * Fix Resolve::DNS stubs in Request spec --- app/lib/request.rb | 32 +++++++++++++++++++++++--------- spec/lib/request_spec.rb | 17 +++++++++++------ 2 files changed, 34 insertions(+), 15 deletions(-) (limited to 'app/lib') diff --git a/app/lib/request.rb b/app/lib/request.rb index fdaaf3636..bb6ef4661 100644 --- a/app/lib/request.rb +++ b/app/lib/request.rb @@ -2,6 +2,7 @@ require 'ipaddr' require 'socket' +require 'resolv' class Request REQUEST_TARGET = '(request-target)' @@ -45,7 +46,7 @@ class Request end begin - yield response.extend(ClientLimit) + yield response.extend(ClientLimit) if block_given? ensure http_client.close end @@ -94,7 +95,7 @@ class Request end def timeout - { connect: 10, read: 10, write: 10 } + { connect: nil, read: 10, write: 10 } end def http_client @@ -139,16 +140,29 @@ class Request class Socket < TCPSocket class << self def open(host, *args) - return super host, *args if thru_hidden_service? host + return super(host, *args) if thru_hidden_service?(host) + outer_e = nil - Addrinfo.foreach(host, nil, nil, :SOCK_STREAM) do |address| - begin - raise Mastodon::HostValidationError if PrivateAddressCheck.private_address? IPAddr.new(address.ip_address) - return super address.ip_address, *args - rescue => e - outer_e = e + + Resolv::DNS.open do |dns| + dns.timeouts = 1 + + 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) + end + rescue => e + outer_e = e + end end end + raise outer_e if outer_e end diff --git a/spec/lib/request_spec.rb b/spec/lib/request_spec.rb index 8cc5d90ce..2d300f18d 100644 --- a/spec/lib/request_spec.rb +++ b/spec/lib/request_spec.rb @@ -48,9 +48,11 @@ describe Request do end it 'executes a HTTP request when the first address is private' do - allow(Addrinfo).to receive(:foreach).with('example.com', nil, nil, :SOCK_STREAM) - .and_yield(Addrinfo.new(["AF_INET", 0, "example.com", "0.0.0.0"], :PF_INET, :SOCK_STREAM)) - .and_yield(Addrinfo.new(["AF_INET6", 0, "example.com", "2001:4860:4860::8844"], :PF_INET6, :SOCK_STREAM)) + resolver = double + + allow(resolver).to receive(:getaddresses).with('example.com').and_return(%w(0.0.0.0 2001:4860:4860::8844)) + allow(resolver).to receive(:timeouts=).and_return(nil) + allow(Resolv::DNS).to receive(:open).and_yield(resolver) expect { |block| subject.perform &block }.to yield_control expect(a_request(:get, 'http://example.com')).to have_been_made.once @@ -81,9 +83,12 @@ describe Request do end it 'raises Mastodon::ValidationError' do - allow(Addrinfo).to receive(:foreach).with('example.com', nil, nil, :SOCK_STREAM) - .and_yield(Addrinfo.new(["AF_INET", 0, "example.com", "0.0.0.0"], :PF_INET, :SOCK_STREAM)) - .and_yield(Addrinfo.new(["AF_INET6", 0, "example.com", "2001:db8::face"], :PF_INET6, :SOCK_STREAM)) + resolver = double + + allow(resolver).to receive(:getaddresses).with('example.com').and_return(%w(0.0.0.0 2001:db8::face)) + allow(resolver).to receive(:timeouts=).and_return(nil) + allow(Resolv::DNS).to receive(:open).and_yield(resolver) + expect { subject.perform }.to raise_error Mastodon::ValidationError end end -- cgit