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 From 43c311b3a101d7364f10365c1a7a19374d539e93 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 27 Nov 2018 18:13:36 +0100 Subject: Fix nil error when no DNS addresses are found for host (#9379) --- app/lib/request.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'app/lib') diff --git a/app/lib/request.rb b/app/lib/request.rb index bb6ef4661..024fce88a 100644 --- a/app/lib/request.rb +++ b/app/lib/request.rb @@ -163,7 +163,11 @@ class Request end end - raise outer_e if outer_e + if outer_e + raise outer_e + else + raise SocketError, "No address for #{host}" + end end alias new open -- cgit From c39d7e7b2b80a23f8d4e1410bb1c2d6033f30af0 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 27 Nov 2018 19:46:05 +0100 Subject: Fix TLS handshake timeout not being enforced (#9381) Follow-up to #9329 --- app/lib/request.rb | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) (limited to 'app/lib') diff --git a/app/lib/request.rb b/app/lib/request.rb index 024fce88a..4a81773e3 100644 --- a/app/lib/request.rb +++ b/app/lib/request.rb @@ -4,6 +4,16 @@ require 'ipaddr' require 'socket' require 'resolv' +# Monkey-patch the HTTP.rb timeout class to avoid using a timeout block +# around the Socket#open method, since we use our own timeout blocks inside +# that method +class HTTP::Timeout::PerOperation + def connect(socket_class, host, port, nodelay = false) + @socket = socket_class.open(host, port) + @socket.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) if nodelay + end +end + class Request REQUEST_TARGET = '(request-target)' @@ -95,7 +105,11 @@ class Request end def timeout - { connect: nil, read: 10, write: 10 } + # 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 -- cgit From 53d0293d259f3b4e8fcfd96aca33867c6af3dd50 Mon Sep 17 00:00:00 2001 From: Thibaut Girka Date: Sun, 4 Nov 2018 12:02:04 +0100 Subject: Add database support for list show-reply preferences --- app/lib/feed_manager.rb | 3 ++- app/models/list.rb | 13 +++++++----- .../20181127165847_add_show_replies_to_lists.rb | 23 ++++++++++++++++++++++ db/schema.rb | 3 ++- 4 files changed, 35 insertions(+), 7 deletions(-) create mode 100644 db/migrate/20181127165847_add_show_replies_to_lists.rb (limited to 'app/lib') diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index 31ff53860..0cdb178c1 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -42,7 +42,8 @@ class FeedManager def push_to_list(list, status) 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? + should_filter &&= !list.show_all_replies? + should_filter &&= !(list.show_list_replies? && 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) diff --git a/app/models/list.rb b/app/models/list.rb index c9c94fca1..8493046e5 100644 --- a/app/models/list.rb +++ b/app/models/list.rb @@ -3,11 +3,12 @@ # # Table name: lists # -# id :bigint(8) not null, primary key -# account_id :bigint(8) not null -# title :string default(""), not null -# created_at :datetime not null -# updated_at :datetime not null +# id :bigint(8) not null, primary key +# account_id :bigint(8) not null +# title :string default(""), not null +# created_at :datetime not null +# updated_at :datetime not null +# replies_policy :integer default("list_replies"), not null # class List < ApplicationRecord @@ -15,6 +16,8 @@ class List < ApplicationRecord PER_ACCOUNT_LIMIT = 50 + enum replies_policy: [:list_replies, :all_replies, :no_replies], _prefix: :show + belongs_to :account, optional: true has_many :list_accounts, inverse_of: :list, dependent: :destroy diff --git a/db/migrate/20181127165847_add_show_replies_to_lists.rb b/db/migrate/20181127165847_add_show_replies_to_lists.rb new file mode 100644 index 000000000..f68c98daf --- /dev/null +++ b/db/migrate/20181127165847_add_show_replies_to_lists.rb @@ -0,0 +1,23 @@ +require Rails.root.join('lib', 'mastodon', 'migration_helpers') + +class AddShowRepliesToLists < ActiveRecord::Migration[5.2] + include Mastodon::MigrationHelpers + + disable_ddl_transaction! + + def up + safety_assured do + add_column_with_default( + :lists, + :replies_policy, + :integer, + allow_null: false, + default: 0 + ) + end + end + + def down + remove_column :lists, :replies_policy + end +end diff --git a/db/schema.rb b/db/schema.rb index cd70b7ed2..4db687137 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2018_11_27_130500) do +ActiveRecord::Schema.define(version: 2018_11_27_165847) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -293,6 +293,7 @@ ActiveRecord::Schema.define(version: 2018_11_27_130500) do t.string "title", default: "", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.integer "replies_policy", default: 0, null: false t.index ["account_id"], name: "index_lists_on_account_id" end -- cgit