From 216b85b053d091306e3311a21f5b050f70a75130 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 14 Dec 2020 09:06:34 +0100 Subject: Fix performance on instances list in admin UI (#15282) - Reduce duplicate queries - Remove n+1 queries - Add accounts count to detailed view - Add separate action log entry for updating existing domain blocks --- config/brakeman.ignore | 101 +++++++++++++++++++++++++++++++++++++++++-------- config/locales/en.yml | 3 ++ config/sidekiq.yml | 3 ++ 3 files changed, 92 insertions(+), 15 deletions(-) (limited to 'config') diff --git a/config/brakeman.ignore b/config/brakeman.ignore index baa993c78..dcbfd02b4 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -102,6 +102,37 @@ "confidence": "Weak", "note": "" }, + { + "warning_type": "Dynamic Render Path", + "warning_code": 15, + "fingerprint": "4704e8093e3e0561bf705f892e8fc6780419f8255f4440b1c0afd09339bd6446", + "check_name": "Render", + "message": "Render path contains parameter value", + "file": "app/views/admin/instances/index.html.haml", + "line": 39, + "link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/", + "code": "render(action => filtered_instances.page(params[:page]), {})", + "render_path": [ + { + "type": "controller", + "class": "Admin::InstancesController", + "method": "index", + "line": 10, + "file": "app/controllers/admin/instances_controller.rb", + "rendered": { + "name": "admin/instances/index", + "file": "app/views/admin/instances/index.html.haml" + } + } + ], + "location": { + "type": "template", + "template": "admin/instances/index" + }, + "user_input": "params[:page]", + "confidence": "Weak", + "note": "" + }, { "warning_type": "Redirect", "warning_code": 18, @@ -122,6 +153,26 @@ "confidence": "High", "note": "" }, + { + "warning_type": "SQL Injection", + "warning_code": 0, + "fingerprint": "6e4051854bb62e2ddbc671f82d6c2328892e1134b8b28105ecba9b0122540714", + "check_name": "SQL", + "message": "Possible SQL injection", + "file": "app/models/account.rb", + "line": 491, + "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", + "code": "find_by_sql([\" WITH first_degree AS (\\n SELECT target_account_id\\n FROM follows\\n WHERE account_id = ?\\n UNION ALL\\n SELECT ?\\n )\\n SELECT\\n accounts.*,\\n (count(f.id) + 1) * ts_rank_cd(#{textsearch}, #{query}, 32) AS rank\\n FROM accounts\\n LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = ?)\\n WHERE accounts.id IN (SELECT * FROM first_degree)\\n AND #{query} @@ #{textsearch}\\n AND accounts.suspended_at IS NULL\\n AND accounts.moved_to_account_id IS NULL\\n GROUP BY accounts.id\\n ORDER BY rank DESC\\n LIMIT ? OFFSET ?\\n\".squish, account.id, account.id, account.id, limit, offset])", + "render_path": null, + "location": { + "type": "method", + "class": "Account", + "method": "advanced_search_for" + }, + "user_input": "textsearch", + "confidence": "Medium", + "note": "" + }, { "warning_type": "SQL Injection", "warning_code": 0, @@ -163,23 +214,23 @@ "note": "" }, { - "warning_type": "Mass Assignment", - "warning_code": 105, - "fingerprint": "8f63dec68951d9bcf7eddb15af9392b2e1333003089c41fb76688dfd3579f394", - "check_name": "PermitAttributes", - "message": "Potentially dangerous key allowed for mass assignment", - "file": "app/controllers/api/v1/crypto/deliveries_controller.rb", - "line": 23, - "link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/", - "code": "params.require(:device).permit(:account_id, :device_id, :type, :body, :hmac)", + "warning_type": "SQL Injection", + "warning_code": 0, + "fingerprint": "9251d682c4e2840e1b2fea91e7d758efe2097ecb7f6255c065e3750d25eb178c", + "check_name": "SQL", + "message": "Possible SQL injection", + "file": "app/models/account.rb", + "line": 460, + "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", + "code": "find_by_sql([\" SELECT\\n accounts.*,\\n ts_rank_cd(#{textsearch}, #{query}, 32) AS rank\\n FROM accounts\\n WHERE #{query} @@ #{textsearch}\\n AND accounts.suspended_at IS NULL\\n AND accounts.moved_to_account_id IS NULL\\n ORDER BY rank DESC\\n LIMIT ? OFFSET ?\\n\".squish, limit, offset])", "render_path": null, "location": { "type": "method", - "class": "Api::V1::Crypto::DeliveriesController", - "method": "resource_params" + "class": "Account", + "method": "search_for" }, - "user_input": ":account_id", - "confidence": "High", + "user_input": "textsearch", + "confidence": "Medium", "note": "" }, { @@ -273,6 +324,26 @@ "confidence": "High", "note": "" }, + { + "warning_type": "SQL Injection", + "warning_code": 0, + "fingerprint": "e21d8fee7a5805761679877ca35ed1029c64c45ef3b4012a30262623e1ba8bb9", + "check_name": "SQL", + "message": "Possible SQL injection", + "file": "app/models/account.rb", + "line": 507, + "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", + "code": "find_by_sql([\" SELECT\\n accounts.*,\\n (count(f.id) + 1) * ts_rank_cd(#{textsearch}, #{query}, 32) AS rank\\n FROM accounts\\n LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = ?) OR (accounts.id = f.target_account_id AND f.account_id = ?)\\n WHERE #{query} @@ #{textsearch}\\n AND accounts.suspended_at IS NULL\\n AND accounts.moved_to_account_id IS NULL\\n GROUP BY accounts.id\\n ORDER BY rank DESC\\n LIMIT ? OFFSET ?\\n\".squish, account.id, account.id, limit, offset])", + "render_path": null, + "location": { + "type": "method", + "class": "Account", + "method": "advanced_search_for" + }, + "user_input": "textsearch", + "confidence": "Medium", + "note": "" + }, { "warning_type": "Mass Assignment", "warning_code": 105, @@ -294,6 +365,6 @@ "note": "" } ], - "updated": "2020-06-01 18:18:02 +0200", - "brakeman_version": "4.8.0" + "updated": "2020-12-07 01:17:13 +0100", + "brakeman_version": "4.10.0" } diff --git a/config/locales/en.yml b/config/locales/en.yml index 59f561aa3..f89f50e4d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -255,6 +255,7 @@ en: unsuspend_account: Unsuspend Account update_announcement: Update Announcement update_custom_emoji: Update Custom Emoji + update_domain_block: Update Domain Block update_status: Update Status actions: assigned_to_self_report: "%{name} assigned report %{target} to themselves" @@ -295,6 +296,7 @@ en: unsuspend_account: "%{name} unsuspended %{target}'s account" update_announcement: "%{name} updated announcement %{target}" update_custom_emoji: "%{name} updated emoji %{target}" + update_domain_block: "%{name} updated domain block for %{target}" update_status: "%{name} updated status by %{target}" deleted_status: "(deleted status)" empty: No logs found. @@ -437,6 +439,7 @@ en: instances: by_domain: Domain delivery_available: Delivery is available + empty: No domains found. known_accounts: one: "%{count} known account" other: "%{count} known accounts" diff --git a/config/sidekiq.yml b/config/sidekiq.yml index 5de25de23..a71c1098e 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -36,3 +36,6 @@ pghero_scheduler: cron: '0 0 * * *' class: Scheduler::PgheroScheduler + instance_refresh_scheduler: + cron: '0 * * * *' + class: Scheduler::InstanceRefreshScheduler -- cgit From 47e507fa61be6dc39dd9821e1d07c33e993cc246 Mon Sep 17 00:00:00 2001 From: ThibG Date: Mon, 14 Dec 2020 10:03:09 +0100 Subject: Add ability to require invite request text (#15326) Fixes #15273 Co-authored-by: Claire --- app/javascript/packs/admin.js | 26 ++++++++++++++++++++++++++ app/javascript/styles/mastodon/forms.scss | 15 ++++++++++----- app/models/form/admin_settings.rb | 2 ++ app/models/user.rb | 3 ++- app/views/about/_registration.html.haml | 2 +- app/views/admin/settings/edit.html.haml | 6 ++++++ app/views/auth/registrations/new.html.haml | 2 +- config/locales/en.yml | 3 +++ config/settings.yml | 1 + 9 files changed, 52 insertions(+), 8 deletions(-) (limited to 'config') diff --git a/app/javascript/packs/admin.js b/app/javascript/packs/admin.js index 1c44fb45b..8fd1b8a8e 100644 --- a/app/javascript/packs/admin.js +++ b/app/javascript/packs/admin.js @@ -67,10 +67,36 @@ const onEnableBootstrapTimelineAccountsChange = (target) => { delegate(document, '#form_admin_settings_enable_bootstrap_timeline_accounts', 'change', ({ target }) => onEnableBootstrapTimelineAccountsChange(target)); +const onChangeRegistrationMode = (target) => { + const enabled = target.value === 'approved'; + + [].forEach.call(document.querySelectorAll('#form_admin_settings_require_invite_text'), (input) => { + input.disabled = !enabled; + if (enabled) { + let element = input; + do { + element.classList.remove('disabled'); + element = element.parentElement; + } while (element && !element.classList.contains('fields-group')); + } else { + let element = input; + do { + element.classList.add('disabled'); + element = element.parentElement; + } while (element && !element.classList.contains('fields-group')); + } + }); +}; + +delegate(document, '#form_admin_settings_registrations_mode', 'change', ({ target }) => onChangeRegistrationMode(target)); + ready(() => { const domainBlockSeverityInput = document.getElementById('domain_block_severity'); if (domainBlockSeverityInput) onDomainBlockSeverityChange(domainBlockSeverityInput); const enableBootstrapTimelineAccounts = document.getElementById('form_admin_settings_enable_bootstrap_timeline_accounts'); if (enableBootstrapTimelineAccounts) onEnableBootstrapTimelineAccountsChange(enableBootstrapTimelineAccounts); + + const registrationMode = document.getElementById('form_admin_settings_registrations_mode'); + if (registrationMode) onChangeRegistrationMode(registrationMode); }); diff --git a/app/javascript/styles/mastodon/forms.scss b/app/javascript/styles/mastodon/forms.scss index 92d89e6f2..e0604303b 100644 --- a/app/javascript/styles/mastodon/forms.scss +++ b/app/javascript/styles/mastodon/forms.scss @@ -377,11 +377,6 @@ code { box-shadow: none; } - &:focus:invalid:not(:placeholder-shown), - &:required:invalid:not(:placeholder-shown) { - border-color: lighten($error-red, 12%); - } - &:required:valid { border-color: $valid-value-color; } @@ -397,6 +392,16 @@ code { } } + input[type=text], + input[type=number], + input[type=email], + input[type=password] { + &:focus:invalid:not(:placeholder-shown), + &:required:invalid:not(:placeholder-shown) { + border-color: lighten($error-red, 12%); + } + } + .input.field_with_errors { label { color: lighten($error-red, 12%); diff --git a/app/models/form/admin_settings.rb b/app/models/form/admin_settings.rb index 390836f28..e9f78da21 100644 --- a/app/models/form/admin_settings.rb +++ b/app/models/form/admin_settings.rb @@ -35,6 +35,7 @@ class Form::AdminSettings show_domain_blocks show_domain_blocks_rationale noindex + require_invite_text ).freeze BOOLEAN_KEYS = %i( @@ -51,6 +52,7 @@ class Form::AdminSettings trends trendable_by_default noindex + require_invite_text ).freeze UPLOAD_KEYS = %i( diff --git a/app/models/user.rb b/app/models/user.rb index 981dc6d47..6088f1994 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -82,7 +82,8 @@ class User < ApplicationRecord has_many :webauthn_credentials, dependent: :destroy has_one :invite_request, class_name: 'UserInviteRequest', inverse_of: :user, dependent: :destroy - accepts_nested_attributes_for :invite_request, reject_if: ->(attributes) { attributes['text'].blank? } + accepts_nested_attributes_for :invite_request, reject_if: ->(attributes) { attributes['text'].blank? && !Setting.require_invite_text } + validates :invite_request, presence: true, on: :create, if: -> { Setting.require_invite_text } validates :locale, inclusion: I18n.available_locales.map(&:to_s), if: :locale? validates_with BlacklistedEmailValidator, on: :create diff --git a/app/views/about/_registration.html.haml b/app/views/about/_registration.html.haml index 6160ca4d4..e4d614d71 100644 --- a/app/views/about/_registration.html.haml +++ b/app/views/about/_registration.html.haml @@ -16,7 +16,7 @@ - if approved_registrations? .fields-group = f.simple_fields_for :invite_request do |invite_request_fields| - = invite_request_fields.input :text, as: :text, wrapper: :with_block_label, required: false + = invite_request_fields.input :text, as: :text, wrapper: :with_block_label, required: Setting.require_invite_text .fields-group = f.input :agreement, as: :boolean, wrapper: :with_label, label: t('auth.checkbox_agreement_html', rules_path: about_more_path, terms_path: terms_path), required: true, disabled: closed_registrations? diff --git a/app/views/admin/settings/edit.html.haml b/app/views/admin/settings/edit.html.haml index 9e28766b1..a162490b5 100644 --- a/app/views/admin/settings/edit.html.haml +++ b/app/views/admin/settings/edit.html.haml @@ -43,6 +43,12 @@ %hr.spacer/ + .fields-group + = f.input :require_invite_text, as: :boolean, wrapper: :with_label, label: t('admin.settings.registrations.require_invite_text.title'), hint: t('admin.settings.registrations.require_invite_text.desc_html'), disabled: !approved_registrations? + .fields-group + + %hr.spacer/ + .fields-group = f.input :enable_bootstrap_timeline_accounts, as: :boolean, wrapper: :with_label, label: t('admin.settings.enable_bootstrap_timeline_accounts.title') .fields-group diff --git a/app/views/auth/registrations/new.html.haml b/app/views/auth/registrations/new.html.haml index de541847f..6981195ed 100644 --- a/app/views/auth/registrations/new.html.haml +++ b/app/views/auth/registrations/new.html.haml @@ -31,7 +31,7 @@ - if approved_registrations? && !@invite.present? .fields-group = f.simple_fields_for :invite_request, resource.invite_request || resource.build_invite_request do |invite_request_fields| - = invite_request_fields.input :text, as: :text, wrapper: :with_block_label, required: false + = invite_request_fields.input :text, as: :text, wrapper: :with_block_label, required: Setting.require_invite_text = f.input :invite_code, as: :hidden diff --git a/config/locales/en.yml b/config/locales/en.yml index f89f50e4d..114f86fd1 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -586,6 +586,9 @@ en: min_invite_role: disabled: No one title: Allow invitations by + require_invite_text: + desc_html: When registrations require manual approval, make the “Why do you want to join?” invite request text mandatory rather than optional + title: Require new users to fill an invite request text registrations_mode: modes: approved: Approval required for sign up diff --git a/config/settings.yml b/config/settings.yml index 217745f28..9cf68a096 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -70,6 +70,7 @@ defaults: &defaults spam_check_enabled: true show_domain_blocks: 'disabled' show_domain_blocks_rationale: 'disabled' + require_invite_text: false development: <<: *defaults -- cgit From a2ef002214572f8166413d989bf2f38768a6910d Mon Sep 17 00:00:00 2001 From: Takeshi Umeda Date: Tue, 15 Dec 2020 11:04:03 +0900 Subject: Fix to isolate the sidekiq process that runs the scheduler job (#15314) --- boxfile.yml | 1 + config/sidekiq.yml | 14 ++++++++++++++ 2 files changed, 15 insertions(+) (limited to 'config') diff --git a/boxfile.yml b/boxfile.yml index c4fd19ce6..c1d89bb15 100644 --- a/boxfile.yml +++ b/boxfile.yml @@ -110,6 +110,7 @@ worker.sidekiq: mailers: bundle exec sidekiq -c 5 -q mailers -L /app/log/sidekiq.log pull: bundle exec sidekiq -c 5 -q pull -L /app/log/sidekiq.log push: bundle exec sidekiq -c 5 -q push -L /app/log/sidekiq.log + scheduler: bundle exec sidekiq -c 5 -q scheduler -L /app/log/sidekiq.log writable_dirs: - tmp diff --git a/config/sidekiq.yml b/config/sidekiq.yml index a71c1098e..010923717 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -5,37 +5,51 @@ - [push, 4] - [mailers, 2] - [pull] + - [scheduler] +:scheduler: + :listened_queues_only: true :schedule: scheduled_statuses_scheduler: every: '5m' class: Scheduler::ScheduledStatusesScheduler + queue: scheduler trending_tags_scheduler: every: '5m' class: Scheduler::TrendingTagsScheduler + queue: scheduler media_cleanup_scheduler: cron: '<%= Random.rand(0..59) %> <%= Random.rand(3..5) %> * * *' class: Scheduler::MediaCleanupScheduler + queue: scheduler feed_cleanup_scheduler: cron: '<%= Random.rand(0..59) %> <%= Random.rand(0..2) %> * * *' class: Scheduler::FeedCleanupScheduler + queue: scheduler doorkeeper_cleanup_scheduler: cron: '<%= Random.rand(0..59) %> <%= Random.rand(0..2) %> * * 0' class: Scheduler::DoorkeeperCleanupScheduler + queue: scheduler user_cleanup_scheduler: cron: '<%= Random.rand(0..59) %> <%= Random.rand(4..6) %> * * *' class: Scheduler::UserCleanupScheduler + queue: scheduler ip_cleanup_scheduler: cron: '<%= Random.rand(0..59) %> <%= Random.rand(3..5) %> * * *' class: Scheduler::IpCleanupScheduler + queue: scheduler email_scheduler: cron: '0 10 * * 2' class: Scheduler::EmailScheduler + queue: scheduler backup_cleanup_scheduler: cron: '<%= Random.rand(0..59) %> <%= Random.rand(3..5) %> * * *' class: Scheduler::BackupCleanupScheduler + queue: scheduler pghero_scheduler: cron: '0 0 * * *' class: Scheduler::PgheroScheduler + queue: scheduler instance_refresh_scheduler: cron: '0 * * * *' class: Scheduler::InstanceRefreshScheduler + queue: scheduler -- cgit From 1390cc194bd07b85fe36e31a0cab22981315974d Mon Sep 17 00:00:00 2001 From: ThibG Date: Tue, 15 Dec 2020 04:30:15 +0100 Subject: Add indication to admin UI of whether a report has been forwarded (#13237) * Add indication to admin UI of whether a report has been forwarded * Rework how forwarded status is displayed Co-authored-by: Claire --- app/models/report.rb | 1 + app/services/report_service.rb | 3 ++- app/views/admin/reports/index.html.haml | 4 ++++ app/views/admin/reports/show.html.haml | 10 ++++++++++ config/locales/en.yml | 2 ++ db/migrate/20200309150742_add_forwarded_to_reports.rb | 5 +++++ db/schema.rb | 1 + 7 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20200309150742_add_forwarded_to_reports.rb (limited to 'config') diff --git a/app/models/report.rb b/app/models/report.rb index f31bcfd2e..cd08120e4 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -14,6 +14,7 @@ # target_account_id :bigint(8) not null # assigned_account_id :bigint(8) # uri :string +# forwarded :boolean # class Report < ApplicationRecord diff --git a/app/services/report_service.rb b/app/services/report_service.rb index 1e955c1e7..9d9c7d6c9 100644 --- a/app/services/report_service.rb +++ b/app/services/report_service.rb @@ -24,7 +24,8 @@ class ReportService < BaseService target_account: @target_account, status_ids: @status_ids, comment: @comment, - uri: @options[:uri] + uri: @options[:uri], + forwarded: ActiveModel::Type::Boolean.new.cast(@options[:forward]) ) end diff --git a/app/views/admin/reports/index.html.haml b/app/views/admin/reports/index.html.haml index bb441380e..721c55f71 100644 --- a/app/views/admin/reports/index.html.haml +++ b/app/views/admin/reports/index.html.haml @@ -59,6 +59,10 @@ = fa_icon('camera') = report.media_attachments.count + - if report.forwarded? + · + = t('admin.reports.forwarded_to', domain: target_account.domain) + .report-card__summary__item__assigned - if report.assigned_account.present? = admin_account_link_to report.assigned_account diff --git a/app/views/admin/reports/show.html.haml b/app/views/admin/reports/show.html.haml index 2681419ca..b060c553f 100644 --- a/app/views/admin/reports/show.html.haml +++ b/app/views/admin/reports/show.html.haml @@ -46,6 +46,16 @@ %td{ colspan: 2 } - if @report.action_taken? = table_link_to 'envelope-open', t('admin.reports.reopen'), admin_report_path(@report, outcome: 'reopen'), method: :put + - unless @report.target_account.local? + %tr + %th= t('admin.reports.forwarded') + %td{ colspan: 3 } + - if @report.forwarded.nil? + \- + - elsif @report.forwarded? + = t('simple_form.yes') + - else + = t('simple_form.no') - if !@report.action_taken_by_account.nil? %tr %th= t('admin.reports.action_taken_by') diff --git a/config/locales/en.yml b/config/locales/en.yml index 114f86fd1..3ff01ac86 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -517,6 +517,8 @@ en: comment: none: None created_at: Reported + forwarded: Forwarded + forwarded_to: Forwarded to %{domain} mark_as_resolved: Mark as resolved mark_as_unresolved: Mark as unresolved notes: diff --git a/db/migrate/20200309150742_add_forwarded_to_reports.rb b/db/migrate/20200309150742_add_forwarded_to_reports.rb new file mode 100644 index 000000000..df278240b --- /dev/null +++ b/db/migrate/20200309150742_add_forwarded_to_reports.rb @@ -0,0 +1,5 @@ +class AddForwardedToReports < ActiveRecord::Migration[5.2] + def change + add_column :reports, :forwarded, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 2f9d369be..55822a4b3 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -717,6 +717,7 @@ ActiveRecord::Schema.define(version: 2020_12_06_004238) do t.bigint "target_account_id", null: false t.bigint "assigned_account_id" t.string "uri" + t.boolean "forwarded" t.index ["account_id"], name: "index_reports_on_account_id" t.index ["target_account_id"], name: "index_reports_on_target_account_id" end -- cgit From 75d2762fdf600de6785f612caf182b32064a9bf6 Mon Sep 17 00:00:00 2001 From: Mashiro Date: Tue, 15 Dec 2020 13:28:14 +0800 Subject: Add "invite request content" display in user account admin page (#15265) * feat: display `invite_request_text` in admin's user account page * fix: move invite_request to the bottom of accounts page * fix: remove time display, remove formate, change code terminology * fix: remove escape --- app/views/admin/accounts/show.html.haml | 10 ++++++++++ config/locales/en.yml | 1 + 2 files changed, 11 insertions(+) (limited to 'config') diff --git a/app/views/admin/accounts/show.html.haml b/app/views/admin/accounts/show.html.haml index d5978eddd..ae527cc23 100644 --- a/app/views/admin/accounts/show.html.haml +++ b/app/views/admin/accounts/show.html.haml @@ -242,3 +242,13 @@ .actions = f.button :button, t('admin.account_moderation_notes.create'), type: :submit + + %hr.spacer/ + + - if @account.user&.invite_request&.text&.present? + %div.speech-bubble + %div.speech-bubble__bubble + = @account.user&.invite_request&.text + %div.speech-bubble__owner + = admin_account_link_to @account + = t('admin.accounts.invite_request_text') diff --git a/config/locales/en.yml b/config/locales/en.yml index 3ff01ac86..1f5798fcc 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -131,6 +131,7 @@ en: follows: Follows header: Header inbox_url: Inbox URL + invite_request_text: Reasons for joining invited_by: Invited by ip: IP joined: Joined -- cgit From 1045549f85041b13002801808b30a332c3a68c61 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 15 Dec 2020 12:55:29 +0100 Subject: Add stoplight for object storage failures, return HTTP 503 (#13043) --- app/controllers/api/base_controller.rb | 2 +- app/controllers/application_controller.rb | 2 +- app/lib/activitypub/activity/create.rb | 4 ++++ config/initializers/paperclip.rb | 11 +++++++++++ lib/paperclip/attachment_extensions.rb | 17 +++++++++++++++++ 5 files changed, 34 insertions(+), 2 deletions(-) (limited to 'config') diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb index fe199e689..85f4cc768 100644 --- a/app/controllers/api/base_controller.rb +++ b/app/controllers/api/base_controller.rb @@ -40,7 +40,7 @@ class Api::BaseController < ApplicationController render json: { error: 'This action is not allowed' }, status: 403 end - rescue_from Mastodon::RaceConditionError do + rescue_from Mastodon::RaceConditionError, Seahorse::Client::NetworkingError, Stoplight::Error::RedLight do render json: { error: 'There was a temporary problem serving your request, please try again' }, status: 503 end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 2201e463e..44616d6e5 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -28,7 +28,7 @@ class ApplicationController < ActionController::Base rescue_from ActiveRecord::RecordNotFound, with: :not_found rescue_from Mastodon::NotPermittedError, with: :forbidden rescue_from HTTP::Error, OpenSSL::SSL::SSLError, with: :internal_server_error - rescue_from Mastodon::RaceConditionError, with: :service_unavailable + rescue_from Mastodon::RaceConditionError, Seahorse::Client::NetworkingError, Stoplight::Error::RedLight, with: :service_unavailable rescue_from Mastodon::RateLimitExceededError, with: :too_many_requests before_action :store_current_location, except: :raise_not_found, unless: :devise_controller? diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index c77f237f9..612744676 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -228,6 +228,8 @@ class ActivityPub::Activity::Create < ActivityPub::Activity emoji ||= CustomEmoji.new(domain: @account.domain, shortcode: shortcode, uri: uri) emoji.image_remote_url = image_url emoji.save + rescue Seahorse::Client::NetworkingError + nil end def process_attachments @@ -250,6 +252,8 @@ class ActivityPub::Activity::Create < ActivityPub::Activity media_attachment.save rescue Mastodon::UnexpectedResponseError, HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError RedownloadMediaWorker.perform_in(rand(30..600).seconds, media_attachment.id) + rescue Seahorse::Client::NetworkingError + nil end end diff --git a/config/initializers/paperclip.rb b/config/initializers/paperclip.rb index 25adcd8d6..9ad7fd814 100644 --- a/config/initializers/paperclip.rb +++ b/config/initializers/paperclip.rb @@ -113,3 +113,14 @@ else end Paperclip.options[:content_type_mappings] = { csv: Import::FILE_TYPES } + +# In some places in the code, we rescue this exception, but we don't always +# load the S3 library, so it may be an undefined constant: + +unless defined?(Seahorse) + module Seahorse + module Client + class NetworkingError < StandardError; end + end + end +end diff --git a/lib/paperclip/attachment_extensions.rb b/lib/paperclip/attachment_extensions.rb index 752e79e65..94f7769b6 100644 --- a/lib/paperclip/attachment_extensions.rb +++ b/lib/paperclip/attachment_extensions.rb @@ -39,6 +39,23 @@ module Paperclip def default_url(style_name = default_style) @url_generator.for_as_default(style_name) end + + STOPLIGHT_THRESHOLD = 10 + STOPLIGHT_COOLDOWN = 30 + + # We overwrite this method to put a circuit breaker around + # calls to object storage, to stop hitting APIs that are slow + # to respond or don't respond at all and as such minimize the + # impact of object storage outages on application throughput + def save + Stoplight('object-storage') { super }.with_threshold(STOPLIGHT_THRESHOLD).with_cool_off_time(STOPLIGHT_COOLDOWN).with_error_handler do |error, handle| + if error.is_a?(Seahorse::Client::NetworkingError) + handle.call(error) + else + raise error + end + end.run + end end end -- cgit