From 5edff32733eff2cbffbf614b31eea85da8dc3aaf Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 15 Apr 2020 20:33:24 +0200 Subject: [PATCH 1/7] Change delivery failure tracking to work with hostnames instead of URLs (#13437) --- .../activitypub/inboxes_controller.rb | 2 +- app/controllers/admin/instances_controller.rb | 2 +- app/lib/delivery_failure_tracker.rb | 36 +++++++++++-------- app/models/account.rb | 2 +- app/models/announcement.rb | 2 +- app/models/relay.rb | 4 +-- app/models/unavailable_domain.rb | 22 ++++++++++++ app/views/admin/accounts/show.html.haml | 4 +-- app/workers/activitypub/delivery_worker.rb | 2 +- ...200407201300_create_unavailable_domains.rb | 9 +++++ ...00407202420_migrate_unavailable_inboxes.rb | 16 +++++++++ db/schema.rb | 9 ++++- .../unavailable_domain_fabricator.rb | 3 ++ spec/lib/delivery_failure_tracker_spec.rb | 30 ++++++---------- spec/models/unavailable_domain_spec.rb | 4 +++ 15 files changed, 103 insertions(+), 44 deletions(-) create mode 100644 app/models/unavailable_domain.rb create mode 100644 db/migrate/20200407201300_create_unavailable_domains.rb create mode 100644 db/migrate/20200407202420_migrate_unavailable_inboxes.rb create mode 100644 spec/fabricators/unavailable_domain_fabricator.rb create mode 100644 spec/models/unavailable_domain_spec.rb diff --git a/app/controllers/activitypub/inboxes_controller.rb b/app/controllers/activitypub/inboxes_controller.rb index 291eec19a8..0a561e7f0f 100644 --- a/app/controllers/activitypub/inboxes_controller.rb +++ b/app/controllers/activitypub/inboxes_controller.rb @@ -49,7 +49,7 @@ class ActivityPub::InboxesController < ActivityPub::BaseController ResolveAccountWorker.perform_async(signed_request_account.acct) end - DeliveryFailureTracker.track_inverse_success!(signed_request_account) + DeliveryFailureTracker.reset!(signed_request_account.inbox_url) end def process_payload diff --git a/app/controllers/admin/instances_controller.rb b/app/controllers/admin/instances_controller.rb index 2fc0412075..1790becbf2 100644 --- a/app/controllers/admin/instances_controller.rb +++ b/app/controllers/admin/instances_controller.rb @@ -19,7 +19,7 @@ module Admin @followers_count = Follow.where(target_account: Account.where(domain: params[:id])).count @reports_count = Report.where(target_account: Account.where(domain: params[:id])).count @blocks_count = Block.where(target_account: Account.where(domain: params[:id])).count - @available = DeliveryFailureTracker.available?(Account.select(:shared_inbox_url).where(domain: params[:id]).first&.shared_inbox_url) + @available = DeliveryFailureTracker.available?(params[:id]) @media_storage = MediaAttachment.where(account: Account.where(domain: params[:id])).sum(:file_file_size) @private_comment = @domain_block&.private_comment @public_comment = @domain_block&.public_comment diff --git a/app/lib/delivery_failure_tracker.rb b/app/lib/delivery_failure_tracker.rb index 8d3be35def..25fa694d22 100644 --- a/app/lib/delivery_failure_tracker.rb +++ b/app/lib/delivery_failure_tracker.rb @@ -3,47 +3,53 @@ class DeliveryFailureTracker FAILURE_DAYS_THRESHOLD = 7 - def initialize(inbox_url) - @inbox_url = inbox_url + def initialize(url_or_host) + @host = url_or_host.start_with?('https://') || url_or_host.start_with?('http://') ? Addressable::URI.parse(url_or_host).normalized_host : url_or_host end def track_failure! Redis.current.sadd(exhausted_deliveries_key, today) - Redis.current.sadd('unavailable_inboxes', @inbox_url) if reached_failure_threshold? + UnavailableDomain.create(domain: @host) if reached_failure_threshold? end def track_success! Redis.current.del(exhausted_deliveries_key) - Redis.current.srem('unavailable_inboxes', @inbox_url) + UnavailableDomain.find_by(domain: @host)&.destroy end def days Redis.current.scard(exhausted_deliveries_key) || 0 end - class << self - def filter(arr) - arr.reject(&method(:unavailable?)) - end + def available? + !UnavailableDomain.where(domain: @host).exists? + end - def unavailable?(url) - Redis.current.sismember('unavailable_inboxes', url) + alias reset! track_success! + + class << self + def without_unavailable(urls) + unavailable_domains_map = Rails.cache.fetch('unavailable_domains') { UnavailableDomain.pluck(:domain).each_with_object({}) { |domain, hash| hash[domain] = true } } + + urls.reject do |url| + host = Addressable::URI.parse(url).normalized_host + unavailable_domains_map[host] + end end def available?(url) - !unavailable?(url) + new(url).available? end - def track_inverse_success!(from_account) - new(from_account.inbox_url).track_success! if from_account.inbox_url.present? - new(from_account.shared_inbox_url).track_success! if from_account.shared_inbox_url.present? + def reset!(url) + new(url).reset! end end private def exhausted_deliveries_key - "exhausted_deliveries:#{@inbox_url}" + "exhausted_deliveries:#{@host}" end def today diff --git a/app/models/account.rb b/app/models/account.rb index 6aceb809cd..dc14e85386 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -413,7 +413,7 @@ class Account < ApplicationRecord def inboxes urls = reorder(nil).where(protocol: :activitypub).pluck(Arel.sql("distinct coalesce(nullif(accounts.shared_inbox_url, ''), accounts.inbox_url)")) - DeliveryFailureTracker.filter(urls) + DeliveryFailureTracker.without_unavailable(urls) end def search_for(terms, limit = 10, offset = 0) diff --git a/app/models/announcement.rb b/app/models/announcement.rb index a4e427b494..c493604c22 100644 --- a/app/models/announcement.rb +++ b/app/models/announcement.rb @@ -14,7 +14,7 @@ # created_at :datetime not null # updated_at :datetime not null # published_at :datetime -# status_ids :bigint is an Array +# status_ids :bigint(8) is an Array # class Announcement < ApplicationRecord diff --git a/app/models/relay.rb b/app/models/relay.rb index 8c8a97db32..870f319795 100644 --- a/app/models/relay.rb +++ b/app/models/relay.rb @@ -27,7 +27,7 @@ class Relay < ApplicationRecord payload = Oj.dump(follow_activity(activity_id)) update!(state: :pending, follow_activity_id: activity_id) - DeliveryFailureTracker.new(inbox_url).track_success! + DeliveryFailureTracker.track_success!(inbox_url) ActivityPub::DeliveryWorker.perform_async(payload, some_local_account.id, inbox_url) end @@ -36,7 +36,7 @@ class Relay < ApplicationRecord payload = Oj.dump(unfollow_activity(activity_id)) update!(state: :idle, follow_activity_id: nil) - DeliveryFailureTracker.new(inbox_url).track_success! + DeliveryFailureTracker.track_success!(inbox_url) ActivityPub::DeliveryWorker.perform_async(payload, some_local_account.id, inbox_url) end diff --git a/app/models/unavailable_domain.rb b/app/models/unavailable_domain.rb new file mode 100644 index 0000000000..e2918b5860 --- /dev/null +++ b/app/models/unavailable_domain.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true +# == Schema Information +# +# Table name: unavailable_domains +# +# id :bigint(8) not null, primary key +# domain :string default(""), not null +# created_at :datetime not null +# updated_at :datetime not null +# + +class UnavailableDomain < ApplicationRecord + include DomainNormalizable + + after_commit :reset_cache! + + private + + def reset_cache! + Rails.cache.delete('unavailable_domains') + end +end diff --git a/app/views/admin/accounts/show.html.haml b/app/views/admin/accounts/show.html.haml index 965fd6fb63..408f94eed8 100644 --- a/app/views/admin/accounts/show.html.haml +++ b/app/views/admin/accounts/show.html.haml @@ -171,12 +171,12 @@ %th= t('admin.accounts.inbox_url') %td = @account.inbox_url - = fa_icon DeliveryFailureTracker.unavailable?(@account.inbox_url) ? 'times' : 'check' + = fa_icon DeliveryFailureTracker.available?(@account.inbox_url) ? 'check' : 'times' %tr %th= t('admin.accounts.shared_inbox_url') %td = @account.shared_inbox_url - = fa_icon DeliveryFailureTracker.unavailable?(@account.shared_inbox_url) ? 'times' : 'check' + = fa_icon DeliveryFailureTracker.available?(@account.shared_inbox_url) ? 'check': 'times' %div{ style: 'overflow: hidden' } %div{ style: 'float: right' } diff --git a/app/workers/activitypub/delivery_worker.rb b/app/workers/activitypub/delivery_worker.rb index 196af4af16..0f925658f5 100644 --- a/app/workers/activitypub/delivery_worker.rb +++ b/app/workers/activitypub/delivery_worker.rb @@ -12,7 +12,7 @@ class ActivityPub::DeliveryWorker HEADERS = { 'Content-Type' => 'application/activity+json' }.freeze def perform(json, source_account_id, inbox_url, options = {}) - return if DeliveryFailureTracker.unavailable?(inbox_url) + return unless DeliveryFailureTracker.available?(inbox_url) @options = options.with_indifferent_access @json = json diff --git a/db/migrate/20200407201300_create_unavailable_domains.rb b/db/migrate/20200407201300_create_unavailable_domains.rb new file mode 100644 index 0000000000..56b477da5d --- /dev/null +++ b/db/migrate/20200407201300_create_unavailable_domains.rb @@ -0,0 +1,9 @@ +class CreateUnavailableDomains < ActiveRecord::Migration[5.2] + def change + create_table :unavailable_domains do |t| + t.string :domain, default: '', null: false, index: { unique: true } + + t.timestamps + end + end +end diff --git a/db/migrate/20200407202420_migrate_unavailable_inboxes.rb b/db/migrate/20200407202420_migrate_unavailable_inboxes.rb new file mode 100644 index 0000000000..0dce26c6f1 --- /dev/null +++ b/db/migrate/20200407202420_migrate_unavailable_inboxes.rb @@ -0,0 +1,16 @@ +class MigrateUnavailableInboxes < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def up + urls = Redis.current.smembers('unavailable_inboxes') + + urls.each do |url| + host = Addressable::URI.parse(url).normalized_host + UnavailableDomain.create(domain: host) + end + + Redis.current.del(*(['unavailable_inboxes'] + Redis.current.keys('exhausted_deliveries:*'))) + end + + def down; end +end diff --git a/db/schema.rb b/db/schema.rb index 021ddac897..54e81bd3ff 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: 2020_03_12_185443) do +ActiveRecord::Schema.define(version: 2020_04_07_202420) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -769,6 +769,13 @@ ActiveRecord::Schema.define(version: 2020_03_12_185443) do t.index ["uri"], name: "index_tombstones_on_uri" end + create_table "unavailable_domains", force: :cascade do |t| + t.string "domain", default: "", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["domain"], name: "index_unavailable_domains_on_domain", unique: true + end + create_table "user_invite_requests", force: :cascade do |t| t.bigint "user_id" t.text "text" diff --git a/spec/fabricators/unavailable_domain_fabricator.rb b/spec/fabricators/unavailable_domain_fabricator.rb new file mode 100644 index 0000000000..f661b87c4a --- /dev/null +++ b/spec/fabricators/unavailable_domain_fabricator.rb @@ -0,0 +1,3 @@ +Fabricator(:unavailable_domain) do + domain { Faker::Internet.domain } +end diff --git a/spec/lib/delivery_failure_tracker_spec.rb b/spec/lib/delivery_failure_tracker_spec.rb index 39c8c7aafc..52a1a92f0a 100644 --- a/spec/lib/delivery_failure_tracker_spec.rb +++ b/spec/lib/delivery_failure_tracker_spec.rb @@ -22,11 +22,11 @@ describe DeliveryFailureTracker do describe '#track_failure!' do it 'marks URL as unavailable after 7 days of being called' do - 6.times { |i| Redis.current.sadd('exhausted_deliveries:http://example.com/inbox', i) } + 6.times { |i| Redis.current.sadd('exhausted_deliveries:example.com', i) } subject.track_failure! expect(subject.days).to eq 7 - expect(described_class.unavailable?('http://example.com/inbox')).to be true + expect(described_class.available?('http://example.com/inbox')).to be false end it 'repeated calls on the same day do not count' do @@ -37,35 +37,27 @@ describe DeliveryFailureTracker do end end - describe '.filter' do + describe '.without_unavailable' do before do - Redis.current.sadd('unavailable_inboxes', 'http://example.com/unavailable/inbox') + Fabricate(:unavailable_domain, domain: 'foo.bar') end it 'removes URLs that are unavailable' do - result = described_class.filter(['http://example.com/good/inbox', 'http://example.com/unavailable/inbox']) + results = described_class.without_unavailable(['http://example.com/good/inbox', 'http://foo.bar/unavailable/inbox']) - expect(result).to include('http://example.com/good/inbox') - expect(result).to_not include('http://example.com/unavailable/inbox') + expect(results).to include('http://example.com/good/inbox') + expect(results).to_not include('http://foo.bar/unavailable/inbox') end end - describe '.track_inverse_success!' do - let(:from_account) { Fabricate(:account, inbox_url: 'http://example.com/inbox', shared_inbox_url: 'http://example.com/shared/inbox') } - + describe '.reset!' do before do - Redis.current.sadd('unavailable_inboxes', 'http://example.com/inbox') - Redis.current.sadd('unavailable_inboxes', 'http://example.com/shared/inbox') - - described_class.track_inverse_success!(from_account) + Fabricate(:unavailable_domain, domain: 'foo.bar') + described_class.reset!('https://foo.bar/inbox') end it 'marks inbox URL as available again' do - expect(described_class.available?('http://example.com/inbox')).to be true - end - - it 'marks shared inbox URL as available again' do - expect(described_class.available?('http://example.com/shared/inbox')).to be true + expect(described_class.available?('http://foo.bar/inbox')).to be true end end end diff --git a/spec/models/unavailable_domain_spec.rb b/spec/models/unavailable_domain_spec.rb new file mode 100644 index 0000000000..3f2621034c --- /dev/null +++ b/spec/models/unavailable_domain_spec.rb @@ -0,0 +1,4 @@ +require 'rails_helper' + +RSpec.describe UnavailableDomain, type: :model do +end From 3825e1943f3e870ffe967f01d6ca4345d69f1a12 Mon Sep 17 00:00:00 2001 From: ThibG Date: Wed, 15 Apr 2020 20:33:53 +0200 Subject: [PATCH 2/7] Fix confusing error when failing to add an alias to an unknown account (#13480) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #13452, fixing broken `uri.nil?` test. Also remove the separate check for `uri` presence, as that would result in a “Please review 2 errors below” while only one would be listed. --- app/models/account_alias.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/account_alias.rb b/app/models/account_alias.rb index 3d0b5d188f..792e9e8d4d 100644 --- a/app/models/account_alias.rb +++ b/app/models/account_alias.rb @@ -16,7 +16,6 @@ class AccountAlias < ApplicationRecord belongs_to :account validates :acct, presence: true, domain: { acct: true } - validates :uri, presence: true validates :uri, uniqueness: { scope: :account_id } validate :validate_target_account @@ -47,7 +46,7 @@ class AccountAlias < ApplicationRecord end def validate_target_account - if uri.nil? + if uri.blank? errors.add(:acct, I18n.t('migrations.errors.not_found')) elsif ActivityPub::TagManager.instance.uri_for(account) == uri errors.add(:acct, I18n.t('migrations.errors.move_to_self')) From ea200a178e3b94299fe7990b7787e988b5678407 Mon Sep 17 00:00:00 2001 From: Takeshi Umeda Date: Thu, 16 Apr 2020 15:03:24 +0900 Subject: [PATCH 3/7] Fix migration 20200407202420_migrate_unavailable_inboxes (#13481) --- db/migrate/20200407202420_migrate_unavailable_inboxes.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/db/migrate/20200407202420_migrate_unavailable_inboxes.rb b/db/migrate/20200407202420_migrate_unavailable_inboxes.rb index 0dce26c6f1..92a3acb5d1 100644 --- a/db/migrate/20200407202420_migrate_unavailable_inboxes.rb +++ b/db/migrate/20200407202420_migrate_unavailable_inboxes.rb @@ -4,8 +4,13 @@ class MigrateUnavailableInboxes < ActiveRecord::Migration[5.2] def up urls = Redis.current.smembers('unavailable_inboxes') - urls.each do |url| - host = Addressable::URI.parse(url).normalized_host + hosts = urls.map do |url| + Addressable::URI.parse(url).normalized_host + end.compact.uniq + + UnavailableDomain.delete_all + + hosts.each do |host| UnavailableDomain.create(domain: host) end From 04c8d825f6a70b48216b7b78e59068931f9231b2 Mon Sep 17 00:00:00 2001 From: Takeshi Umeda Date: Thu, 16 Apr 2020 15:04:10 +0900 Subject: [PATCH 4/7] Fix DeliveryWorker not to call failure_tracker when inbox_url is unavailable (#13482) --- app/workers/activitypub/delivery_worker.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/workers/activitypub/delivery_worker.rb b/app/workers/activitypub/delivery_worker.rb index 0f925658f5..14e37dc34a 100644 --- a/app/workers/activitypub/delivery_worker.rb +++ b/app/workers/activitypub/delivery_worker.rb @@ -23,10 +23,12 @@ class ActivityPub::DeliveryWorker perform_request ensure - if @performed - failure_tracker.track_success! - else - failure_tracker.track_failure! + if @inbox_url.present? + if @performed + failure_tracker.track_success! + else + failure_tracker.track_failure! + end end end From ab8d7c0680d7f75826277be4c8eea1ebd396be8a Mon Sep 17 00:00:00 2001 From: Gurgen Hayrapetyan Date: Thu, 16 Apr 2020 22:16:20 +0400 Subject: [PATCH 5/7] Fix Poll fetchPoll action not being debounced. (#13485) * Fix Poll fetchPoll action not being debounced. * Fix unused import in the Poll component --- app/javascript/mastodon/components/poll.js | 5 +++-- .../mastodon/containers/poll_container.js | 15 ++++++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/app/javascript/mastodon/components/poll.js b/app/javascript/mastodon/components/poll.js index 7525a1030a..29da52ae77 100644 --- a/app/javascript/mastodon/components/poll.js +++ b/app/javascript/mastodon/components/poll.js @@ -4,7 +4,7 @@ import ImmutablePropTypes from 'react-immutable-proptypes'; import ImmutablePureComponent from 'react-immutable-pure-component'; import { defineMessages, injectIntl, FormattedMessage } from 'react-intl'; import classNames from 'classnames'; -import { vote, fetchPoll } from 'mastodon/actions/polls'; +import { vote } from 'mastodon/actions/polls'; import Motion from 'mastodon/features/ui/util/optional_motion'; import spring from 'react-motion/lib/spring'; import escapeTextContentForBrowser from 'escape-html'; @@ -30,6 +30,7 @@ class Poll extends ImmutablePureComponent { intl: PropTypes.object.isRequired, dispatch: PropTypes.func, disabled: PropTypes.bool, + refresh: PropTypes.func, }; state = { @@ -108,7 +109,7 @@ class Poll extends ImmutablePureComponent { return; } - this.props.dispatch(fetchPoll(this.props.poll.get('id'))); + this.props.refresh(); }; renderOption (option, optionIndex, showResults) { diff --git a/app/javascript/mastodon/containers/poll_container.js b/app/javascript/mastodon/containers/poll_container.js index cd7216de7a..1c2e8d2a7a 100644 --- a/app/javascript/mastodon/containers/poll_container.js +++ b/app/javascript/mastodon/containers/poll_container.js @@ -1,8 +1,21 @@ import { connect } from 'react-redux'; +import { debounce } from 'lodash'; + import Poll from 'mastodon/components/poll'; +import { fetchPoll } from 'mastodon/actions/polls'; + +const mapDispatchToProps = (dispatch, { pollId }) => ({ + refresh: debounce( + () => { + dispatch(fetchPoll(pollId)); + }, + 1000, + { leading: true }, + ), +}); const mapStateToProps = (state, { pollId }) => ({ poll: state.getIn(['polls', pollId]), }); -export default connect(mapStateToProps)(Poll); +export default connect(mapStateToProps, mapDispatchToProps)(Poll); From d18d6c29f38cb9830b900ed1e7068518f41b9b26 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 17 Apr 2020 15:14:24 +0200 Subject: [PATCH 6/7] Fix search not working due to proxy settings when using hidden services (#13488) Fix #13484 --- config/initializers/chewy.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/config/initializers/chewy.rb b/config/initializers/chewy.rb index 9ff0dccc1b..18d2f18c1f 100644 --- a/config/initializers/chewy.rb +++ b/config/initializers/chewy.rb @@ -23,3 +23,9 @@ module Chewy end end end + +# ElasticSearch uses Faraday internally. Faraday interprets the +# http_proxy env variable by default which leads to issues when +# Mastodon is run with hidden services enabled, because +# ElasticSearch is *not* supposed to be accessed through a proxy +Faraday.ignore_env_proxy = true From 89077fb65713ece348fdbee1986d1593383ba801 Mon Sep 17 00:00:00 2001 From: ThibG Date: Fri, 17 Apr 2020 19:54:58 +0200 Subject: [PATCH 7/7] Fix admin actions log crash when displaying updates of deleted announcements (#13489) Fixes #13487 --- app/helpers/admin/action_logs_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/admin/action_logs_helper.rb b/app/helpers/admin/action_logs_helper.rb index 88d6e4580d..8e398c3b26 100644 --- a/app/helpers/admin/action_logs_helper.rb +++ b/app/helpers/admin/action_logs_helper.rb @@ -47,7 +47,7 @@ module Admin::ActionLogsHelper I18n.t('admin.action_logs.deleted_status') end when 'Announcement' - truncate(attributes['text']) + truncate(attributes['text'].is_a?(Array) ? attributes['text'].last : attributes['text']) end end end