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