diff --git a/CHANGELOG.md b/CHANGELOG.md index b17dcdeef..aaedba113 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,54 @@ Changelog All notable changes to this project will be documented in this file. +## [4.1.3] - 2023-07-06 + +### Added + +- Add fallback redirection when getting a webfinger query `LOCAL_DOMAIN@LOCAL_DOMAIN` ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/23600)) + +### Changed + +- Change OpenGraph-based embeds to allow fullscreen ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/25058)) +- Change AccessTokensVacuum to also delete expired tokens ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/24868)) +- Change profile updates to be sent to recently-mentioned servers ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/24852)) +- Change automatic post deletion thresholds and load detection ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/24614)) +- Change `/api/v1/statuses/:id/history` to always return at least one item ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/25510)) +- Change auto-linking to allow carets in URL query params ([renchap](https://github.com/mastodon/mastodon/pull/25216)) + +### Removed + +- Remove invalid `X-Frame-Options: ALLOWALL` ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/25070)) + +### Fixed + +- Fix wrong view being displayed when a webhook fails validation ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/25464)) +- Fix soft-deleted post cleanup scheduler overwhelming the streaming server ([ThisIsMissEm](https://github.com/mastodon/mastodon/pull/25519)) +- Fix incorrect pagination headers in `/api/v2/admin/accounts` ([danielmbrasil](https://github.com/mastodon/mastodon/pull/25477)) +- Fix multiple inefficiencies in automatic post cleanup worker ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/24607), [ClearlyClaire](https://github.com/mastodon/mastodon/pull/24785), [ClearlyClaire](https://github.com/mastodon/mastodon/pull/24840)) +- Fix performance of streaming by parsing message JSON once ([ThisIsMissEm](https://github.com/mastodon/mastodon/pull/25278), [ThisIsMissEm](https://github.com/mastodon/mastodon/pull/25361)) +- Fix CSP headers when `S3_ALIAS_HOST` includes a path component ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/25273)) +- Fix `tootctl accounts approve --number N` not aproving N earliest registrations ([danielmbrasil](https://github.com/mastodon/mastodon/pull/24605)) +- Fix reports not being closed when performing batch suspensions ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/24988)) +- Fix being able to vote on your own polls ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/25015)) +- Fix race condition when reblogging a status ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/25016)) +- Fix “Authorized applications” inefficiently and incorrectly getting last use date ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/25060)) +- Fix “Authorized applications” crashing when listing apps with certain admin API scopes ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/25713)) +- Fix multiple N+1s in ConversationsController ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/25134), [ClearlyClaire](https://github.com/mastodon/mastodon/pull/25399), [ClearlyClaire](https://github.com/mastodon/mastodon/pull/25499)) +- Fix user archive takeouts when using OpenStack Swift ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/24431)) +- Fix searching for remote content by URL not working under certain conditions ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/25637)) +- Fix inefficiencies in indexing content for search ([VyrCossont](https://github.com/mastodon/mastodon/pull/24285), [VyrCossont](https://github.com/mastodon/mastodon/pull/24342)) + +### Security + +- Add finer permission requirements for managing webhooks ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/25463)) +- Update dependencies +- Add hardening headers for user-uploaded files ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/25756)) +- Fix verified links possibly hiding important parts of the URL (CVE-2023-36462) +- Fix timeout handling of outbound HTTP requests (CVE-2023-36461) +- Fix arbitrary file creation through media processing (CVE-2023-36460) +- Fix possible XSS in preview cards (CVE-2023-36459) + ## [4.1.2] - 2023-04-04 ### Fixed diff --git a/Gemfile.lock b/Gemfile.lock index 987f7d80d..dd2fd7344 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -10,40 +10,40 @@ GIT GEM remote: https://rubygems.org/ specs: - actioncable (6.1.7.2) - actionpack (= 6.1.7.2) - activesupport (= 6.1.7.2) + actioncable (6.1.7.4) + actionpack (= 6.1.7.4) + activesupport (= 6.1.7.4) nio4r (~> 2.0) websocket-driver (>= 0.6.1) - actionmailbox (6.1.7.2) - actionpack (= 6.1.7.2) - activejob (= 6.1.7.2) - activerecord (= 6.1.7.2) - activestorage (= 6.1.7.2) - activesupport (= 6.1.7.2) + actionmailbox (6.1.7.4) + actionpack (= 6.1.7.4) + activejob (= 6.1.7.4) + activerecord (= 6.1.7.4) + activestorage (= 6.1.7.4) + activesupport (= 6.1.7.4) mail (>= 2.7.1) - actionmailer (6.1.7.2) - actionpack (= 6.1.7.2) - actionview (= 6.1.7.2) - activejob (= 6.1.7.2) - activesupport (= 6.1.7.2) + actionmailer (6.1.7.4) + actionpack (= 6.1.7.4) + actionview (= 6.1.7.4) + activejob (= 6.1.7.4) + activesupport (= 6.1.7.4) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 2.0) - actionpack (6.1.7.2) - actionview (= 6.1.7.2) - activesupport (= 6.1.7.2) + actionpack (6.1.7.4) + actionview (= 6.1.7.4) + activesupport (= 6.1.7.4) rack (~> 2.0, >= 2.0.9) rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.2.0) - actiontext (6.1.7.2) - actionpack (= 6.1.7.2) - activerecord (= 6.1.7.2) - activestorage (= 6.1.7.2) - activesupport (= 6.1.7.2) + actiontext (6.1.7.4) + actionpack (= 6.1.7.4) + activerecord (= 6.1.7.4) + activestorage (= 6.1.7.4) + activesupport (= 6.1.7.4) nokogiri (>= 1.8.5) - actionview (6.1.7.2) - activesupport (= 6.1.7.2) + actionview (6.1.7.4) + activesupport (= 6.1.7.4) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) @@ -54,22 +54,22 @@ GEM case_transform (>= 0.2) jsonapi-renderer (>= 0.1.1.beta1, < 0.3) active_record_query_trace (1.8) - activejob (6.1.7.2) - activesupport (= 6.1.7.2) + activejob (6.1.7.4) + activesupport (= 6.1.7.4) globalid (>= 0.3.6) - activemodel (6.1.7.2) - activesupport (= 6.1.7.2) - activerecord (6.1.7.2) - activemodel (= 6.1.7.2) - activesupport (= 6.1.7.2) - activestorage (6.1.7.2) - actionpack (= 6.1.7.2) - activejob (= 6.1.7.2) - activerecord (= 6.1.7.2) - activesupport (= 6.1.7.2) + activemodel (6.1.7.4) + activesupport (= 6.1.7.4) + activerecord (6.1.7.4) + activemodel (= 6.1.7.4) + activesupport (= 6.1.7.4) + activestorage (6.1.7.4) + actionpack (= 6.1.7.4) + activejob (= 6.1.7.4) + activerecord (= 6.1.7.4) + activesupport (= 6.1.7.4) marcel (~> 1.0) mini_mime (>= 1.1.0) - activesupport (6.1.7.2) + activesupport (6.1.7.4) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 1.6, < 2) minitest (>= 5.1) @@ -173,7 +173,7 @@ GEM cocoon (1.2.15) coderay (1.1.3) color_diff (0.1) - concurrent-ruby (1.2.0) + concurrent-ruby (1.2.2) connection_pool (2.3.0) cose (1.2.1) cbor (~> 0.5.9) @@ -206,7 +206,7 @@ GEM docile (1.4.0) domain_name (0.5.20190701) unf (>= 0.0.5, < 1.0.0) - doorkeeper (5.6.4) + doorkeeper (5.6.6) railties (>= 5) dotenv (2.8.1) dotenv-rails (2.8.1) @@ -388,7 +388,7 @@ GEM loofah (2.19.1) crass (~> 1.0.2) nokogiri (>= 1.5.9) - mail (2.8.0.1) + mail (2.8.1) mini_mime (>= 0.1.1) net-imap net-pop @@ -405,12 +405,12 @@ GEM mime-types-data (~> 3.2015) mime-types-data (3.2022.0105) mini_mime (1.1.2) - mini_portile2 (2.8.1) + mini_portile2 (2.8.2) minitest (5.17.0) msgpack (1.6.0) multi_json (1.15.0) multipart-post (2.1.1) - net-imap (0.3.4) + net-imap (0.3.6) date net-protocol net-ldap (0.17.1) @@ -423,8 +423,8 @@ GEM net-smtp (0.3.3) net-protocol net-ssh (7.0.1) - nio4r (2.5.8) - nokogiri (1.14.1) + nio4r (2.5.9) + nokogiri (1.14.5) mini_portile2 (~> 2.8.0) racc (~> 1.4) nsa (0.2.8) @@ -497,7 +497,7 @@ GEM activesupport (>= 3.0.0) raabro (1.4.0) racc (1.6.2) - rack (2.2.6.2) + rack (2.2.7) rack-attack (6.6.1) rack (>= 1.0, < 3) rack-cors (1.1.1) @@ -512,20 +512,20 @@ GEM rack rack-test (2.0.2) rack (>= 1.3) - rails (6.1.7.2) - actioncable (= 6.1.7.2) - actionmailbox (= 6.1.7.2) - actionmailer (= 6.1.7.2) - actionpack (= 6.1.7.2) - actiontext (= 6.1.7.2) - actionview (= 6.1.7.2) - activejob (= 6.1.7.2) - activemodel (= 6.1.7.2) - activerecord (= 6.1.7.2) - activestorage (= 6.1.7.2) - activesupport (= 6.1.7.2) + rails (6.1.7.4) + actioncable (= 6.1.7.4) + actionmailbox (= 6.1.7.4) + actionmailer (= 6.1.7.4) + actionpack (= 6.1.7.4) + actiontext (= 6.1.7.4) + actionview (= 6.1.7.4) + activejob (= 6.1.7.4) + activemodel (= 6.1.7.4) + activerecord (= 6.1.7.4) + activestorage (= 6.1.7.4) + activesupport (= 6.1.7.4) bundler (>= 1.15.0) - railties (= 6.1.7.2) + railties (= 6.1.7.4) sprockets-rails (>= 2.0.0) rails-controller-testing (1.0.5) actionpack (>= 5.0.1.rc1) @@ -541,9 +541,9 @@ GEM railties (>= 6.0.0, < 7) rails-settings-cached (0.6.6) rails (>= 4.2.0) - railties (6.1.7.2) - actionpack (= 6.1.7.2) - activesupport (= 6.1.7.2) + railties (6.1.7.4) + actionpack (= 6.1.7.4) + activesupport (= 6.1.7.4) method_source rake (>= 12.2) thor (~> 1.0) @@ -689,9 +689,9 @@ GEM unicode-display_width (>= 1.1.1, < 3) terrapin (0.6.0) climate_control (>= 0.0.3, < 1.0) - thor (1.2.1) + thor (1.2.2) tilt (2.0.11) - timeout (0.3.1) + timeout (0.3.2) tpm-key_attestation (0.11.0) bindata (~> 2.4) openssl (> 2.0, < 3.1) @@ -754,7 +754,7 @@ GEM xorcist (1.1.3) xpath (3.2.0) nokogiri (~> 1.8) - zeitwerk (2.6.6) + zeitwerk (2.6.8) PLATFORMS ruby diff --git a/app/controllers/admin/webhooks_controller.rb b/app/controllers/admin/webhooks_controller.rb index d6fb1a4ea..76062ddf7 100644 --- a/app/controllers/admin/webhooks_controller.rb +++ b/app/controllers/admin/webhooks_controller.rb @@ -20,6 +20,7 @@ module Admin authorize :webhook, :create? @webhook = Webhook.new(resource_params) + @webhook.current_account = current_account if @webhook.save redirect_to admin_webhook_path(@webhook) @@ -39,10 +40,12 @@ module Admin def update authorize @webhook, :update? + @webhook.current_account = current_account + if @webhook.update(resource_params) redirect_to admin_webhook_path(@webhook) else - render :show + render :edit end end diff --git a/app/controllers/api/v1/conversations_controller.rb b/app/controllers/api/v1/conversations_controller.rb index 6c7583403..818ba6ebb 100644 --- a/app/controllers/api/v1/conversations_controller.rb +++ b/app/controllers/api/v1/conversations_controller.rb @@ -11,7 +11,7 @@ class Api::V1::ConversationsController < Api::BaseController def index @conversations = paginated_conversations - render json: @conversations, each_serializer: REST::ConversationSerializer + render json: @conversations, each_serializer: REST::ConversationSerializer, relationships: StatusRelationshipsPresenter.new(@conversations.map(&:last_status), current_user&.account_id) end def read @@ -32,6 +32,19 @@ class Api::V1::ConversationsController < Api::BaseController def paginated_conversations AccountConversation.where(account: current_account) + .includes( + account: :account_stat, + last_status: [ + :media_attachments, + :preview_cards, + :status_stat, + :tags, + { + active_mentions: [account: :account_stat], + account: :account_stat, + }, + ] + ) .to_a_paginated_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id)) end diff --git a/app/controllers/api/v1/statuses/histories_controller.rb b/app/controllers/api/v1/statuses/histories_controller.rb index 7fe73a6f5..b1c19987a 100644 --- a/app/controllers/api/v1/statuses/histories_controller.rb +++ b/app/controllers/api/v1/statuses/histories_controller.rb @@ -7,11 +7,15 @@ class Api::V1::Statuses::HistoriesController < Api::BaseController before_action :set_status def show - render json: @status.edits.includes(:account, status: [:account]), each_serializer: REST::StatusEditSerializer + render json: status_edits, each_serializer: REST::StatusEditSerializer end private + def status_edits + @status.edits.includes(:account, status: [:account]).to_a.presence || [@status.build_snapshot(at_time: @status.edited_at || @status.created_at)] + end + def set_status @status = Status.find(params[:status_id]) authorize @status, :show? diff --git a/app/controllers/api/v1/statuses/reblogs_controller.rb b/app/controllers/api/v1/statuses/reblogs_controller.rb index 1be15a5a4..a4079a16d 100644 --- a/app/controllers/api/v1/statuses/reblogs_controller.rb +++ b/app/controllers/api/v1/statuses/reblogs_controller.rb @@ -2,6 +2,8 @@ class Api::V1::Statuses::ReblogsController < Api::BaseController include Authorization + include Redisable + include Lockable before_action -> { doorkeeper_authorize! :write, :'write:statuses' } before_action :require_user! @@ -10,7 +12,9 @@ class Api::V1::Statuses::ReblogsController < Api::BaseController override_rate_limit_headers :create, family: :statuses def create - @status = ReblogService.new.call(current_account, @reblog, reblog_params) + with_lock("reblog:#{current_account.id}:#{@reblog.id}") do + @status = ReblogService.new.call(current_account, @reblog, reblog_params) + end render json: @status, serializer: REST::StatusSerializer end diff --git a/app/controllers/api/v2/admin/accounts_controller.rb b/app/controllers/api/v2/admin/accounts_controller.rb index b25831aa0..bc8f8b6f3 100644 --- a/app/controllers/api/v2/admin/accounts_controller.rb +++ b/app/controllers/api/v2/admin/accounts_controller.rb @@ -18,6 +18,14 @@ class Api::V2::Admin::AccountsController < Api::V1::Admin::AccountsController private + def next_path + api_v2_admin_accounts_url(pagination_params(max_id: pagination_max_id)) if records_continue? + end + + def prev_path + api_v2_admin_accounts_url(pagination_params(min_id: pagination_since_id)) unless @accounts.empty? + end + def filtered_accounts AccountFilter.new(translated_filter_params).results end diff --git a/app/controllers/backups_controller.rb b/app/controllers/backups_controller.rb index 0687b62c5..5891da6f6 100644 --- a/app/controllers/backups_controller.rb +++ b/app/controllers/backups_controller.rb @@ -13,7 +13,7 @@ class BackupsController < ApplicationController when :s3 redirect_to @backup.dump.expiring_url(10) when :fog - if Paperclip::Attachment.default_options.dig(:storage, :fog_credentials, :openstack_temp_url_key).present? + if Paperclip::Attachment.default_options.dig(:fog_credentials, :openstack_temp_url_key).present? redirect_to @backup.dump.expiring_url(Time.now.utc + 10) else redirect_to full_asset_url(@backup.dump.url) diff --git a/app/controllers/media_controller.rb b/app/controllers/media_controller.rb index 3cdd97f06..a90c585ac 100644 --- a/app/controllers/media_controller.rb +++ b/app/controllers/media_controller.rb @@ -46,6 +46,6 @@ class MediaController < ApplicationController end def allow_iframing - response.headers['X-Frame-Options'] = 'ALLOWALL' + response.headers.delete('X-Frame-Options') end end diff --git a/app/controllers/oauth/authorized_applications_controller.rb b/app/controllers/oauth/authorized_applications_controller.rb index 45151cdd7..63afc4c06 100644 --- a/app/controllers/oauth/authorized_applications_controller.rb +++ b/app/controllers/oauth/authorized_applications_controller.rb @@ -8,6 +8,8 @@ class Oauth::AuthorizedApplicationsController < Doorkeeper::AuthorizedApplicatio before_action :require_not_suspended!, only: :destroy before_action :set_body_classes + before_action :set_last_used_at_by_app, only: :index, unless: -> { request.format == :json } + skip_before_action :require_functional! include Localized @@ -30,4 +32,14 @@ class Oauth::AuthorizedApplicationsController < Doorkeeper::AuthorizedApplicatio def require_not_suspended! forbidden if current_account.suspended? end + + def set_last_used_at_by_app + @last_used_at_by_app = Doorkeeper::AccessToken + .select('DISTINCT ON (application_id) application_id, last_used_at') + .where(resource_owner_id: current_resource_owner.id) + .where.not(last_used_at: nil) + .order(application_id: :desc, last_used_at: :desc) + .pluck(:application_id, :last_used_at) + .to_h + end end diff --git a/app/controllers/statuses_controller.rb b/app/controllers/statuses_controller.rb index 0e0783b4b..33defaa1c 100644 --- a/app/controllers/statuses_controller.rb +++ b/app/controllers/statuses_controller.rb @@ -43,7 +43,7 @@ class StatusesController < ApplicationController return not_found if @status.hidden? || @status.reblog? expires_in 180, public: true - response.headers['X-Frame-Options'] = 'ALLOWALL' + response.headers.delete('X-Frame-Options') render layout: 'embedded' end diff --git a/app/controllers/well_known/webfinger_controller.rb b/app/controllers/well_known/webfinger_controller.rb index 2b296ea3b..f83a62a1f 100644 --- a/app/controllers/well_known/webfinger_controller.rb +++ b/app/controllers/well_known/webfinger_controller.rb @@ -18,7 +18,14 @@ module WellKnown private def set_account - @account = Account.find_local!(username_from_resource) + username = username_from_resource + @account = begin + if username == Rails.configuration.x.local_domain + Account.representative + else + Account.find_local!(username) + end + end end def username_from_resource diff --git a/app/helpers/formatting_helper.rb b/app/helpers/formatting_helper.rb index c70931489..ce87c3524 100644 --- a/app/helpers/formatting_helper.rb +++ b/app/helpers/formatting_helper.rb @@ -58,6 +58,10 @@ module FormattingHelper end def account_field_value_format(field, with_rel_me: true) - html_aware_format(field.value, field.account.local?, with_rel_me: with_rel_me, with_domains: true, multiline: false) + if field.verified? && !field.account.local? + TextFormatter.shortened_link(field.value_for_verification) + else + html_aware_format(field.value, field.account.local?, with_rel_me: with_rel_me, with_domains: true, multiline: false) + end end end diff --git a/app/lib/account_reach_finder.rb b/app/lib/account_reach_finder.rb index 706ce8c1f..481e25439 100644 --- a/app/lib/account_reach_finder.rb +++ b/app/lib/account_reach_finder.rb @@ -6,7 +6,7 @@ class AccountReachFinder end def inboxes - (followers_inboxes + reporters_inboxes + relay_inboxes).uniq + (followers_inboxes + reporters_inboxes + recently_mentioned_inboxes + relay_inboxes).uniq end private @@ -19,6 +19,13 @@ class AccountReachFinder Account.where(id: @account.targeted_reports.select(:account_id)).inboxes end + def recently_mentioned_inboxes + cutoff_id = Mastodon::Snowflake.id_at(2.days.ago, with_random: false) + recent_statuses = @account.statuses.recent.where(id: cutoff_id...).limit(200) + + Account.joins(:mentions).where(mentions: { status: recent_statuses }).inboxes.take(2000) + end + def relay_inboxes Relay.enabled.pluck(:inbox_url) end diff --git a/app/lib/application_extension.rb b/app/lib/application_extension.rb index d61ec0e6e..4de69c1ea 100644 --- a/app/lib/application_extension.rb +++ b/app/lib/application_extension.rb @@ -9,10 +9,6 @@ module ApplicationExtension validates :redirect_uri, length: { maximum: 2_000 } end - def most_recently_used_access_token - @most_recently_used_access_token ||= access_tokens.where.not(last_used_at: nil).order(last_used_at: :desc).first - end - def confirmation_redirect_uri redirect_uri.lines.first.strip end diff --git a/app/lib/link_details_extractor.rb b/app/lib/link_details_extractor.rb index 2e0672abe..9d7748425 100644 --- a/app/lib/link_details_extractor.rb +++ b/app/lib/link_details_extractor.rb @@ -140,7 +140,7 @@ class LinkDetailsExtractor end def html - player_url.present? ? content_tag(:iframe, nil, src: player_url, width: width, height: height, allowtransparency: 'true', scrolling: 'no', frameborder: '0') : nil + player_url.present? ? content_tag(:iframe, nil, src: player_url, width: width, height: height, allowfullscreen: 'true', allowtransparency: 'true', scrolling: 'no', frameborder: '0') : nil end def width diff --git a/app/lib/request.rb b/app/lib/request.rb index 0508169dc..c76ec6b64 100644 --- a/app/lib/request.rb +++ b/app/lib/request.rb @@ -7,11 +7,48 @@ 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 +# +# Also changes how the read timeout behaves so that it is cumulative (closer +# to HTTP::Timeout::Global, but still having distinct timeouts for other +# operation types) 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 + + # Reset deadline when the connection is re-used for different requests + def reset_counter + @deadline = nil + end + + # Read data from the socket + def readpartial(size, buffer = nil) + @deadline ||= Process.clock_gettime(Process::CLOCK_MONOTONIC) + @read_timeout + + timeout = false + loop do + result = @socket.read_nonblock(size, buffer, exception: false) + + return :eof if result.nil? + + remaining_time = @deadline - Process.clock_gettime(Process::CLOCK_MONOTONIC) + raise HTTP::TimeoutError, "Read timed out after #{@read_timeout} seconds" if timeout || remaining_time <= 0 + return result if result != :wait_readable + + # marking the socket for timeout. Why is this not being raised immediately? + # it seems there is some race-condition on the network level between calling + # #read_nonblock and #wait_readable, in which #read_nonblock signalizes waiting + # for reads, and when waiting for x seconds, it returns nil suddenly without completing + # the x seconds. In a normal case this would be a timeout on wait/read, but it can + # also mean that the socket has been closed by the server. Therefore we "mark" the + # socket for timeout and try to read more bytes. If it returns :eof, it's all good, no + # timeout. Else, the first timeout was a proper timeout. + # This hack has to be done because io/wait#wait_readable doesn't provide a value for when + # the socket is closed by the server, and HTTP::Parser doesn't provide the limit for the chunks. + timeout = true unless @socket.to_io.wait_readable(remaining_time) + end + end end class Request diff --git a/app/lib/scope_parser.rb b/app/lib/scope_parser.rb index d268688c8..45eb3c7b9 100644 --- a/app/lib/scope_parser.rb +++ b/app/lib/scope_parser.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class ScopeParser < Parslet::Parser - rule(:term) { match('[a-z]').repeat(1).as(:term) } + rule(:term) { match('[a-z_]').repeat(1).as(:term) } rule(:colon) { str(':') } rule(:access) { (str('write') | str('read')).as(:access) } rule(:namespace) { str('admin').as(:namespace) } diff --git a/app/lib/text_formatter.rb b/app/lib/text_formatter.rb index 48e2fc233..cdf8a48f7 100644 --- a/app/lib/text_formatter.rb +++ b/app/lib/text_formatter.rb @@ -48,6 +48,26 @@ class TextFormatter html.html_safe # rubocop:disable Rails/OutputSafety end + class << self + include ERB::Util + + def shortened_link(url, rel_me: false) + url = Addressable::URI.parse(url).to_s + rel = rel_me ? (DEFAULT_REL + %w(me)) : DEFAULT_REL + + prefix = url.match(URL_PREFIX_REGEX).to_s + display_url = url[prefix.length, 30] + suffix = url[prefix.length + 30..-1] + cutoff = url[prefix.length..-1].length > 30 + + <<~HTML.squish + #{h(display_url)} + HTML + rescue Addressable::URI::InvalidURIError, IDN::Idna::IdnaError + h(url) + end + end + private def rewrite @@ -70,19 +90,7 @@ class TextFormatter end def link_to_url(entity) - url = Addressable::URI.parse(entity[:url]).to_s - rel = with_rel_me? ? (DEFAULT_REL + %w(me)) : DEFAULT_REL - - prefix = url.match(URL_PREFIX_REGEX).to_s - display_url = url[prefix.length, 30] - suffix = url[prefix.length + 30..-1] - cutoff = url[prefix.length..-1].length > 30 - - <<~HTML.squish - #{h(display_url)} - HTML - rescue Addressable::URI::InvalidURIError, IDN::Idna::IdnaError - h(entity[:url]) + TextFormatter.shortened_link(entity[:url], rel_me: with_rel_me?) end def link_to_hashtag(entity) diff --git a/app/lib/vacuum/access_tokens_vacuum.rb b/app/lib/vacuum/access_tokens_vacuum.rb index 7b91f74a5..a224f6d63 100644 --- a/app/lib/vacuum/access_tokens_vacuum.rb +++ b/app/lib/vacuum/access_tokens_vacuum.rb @@ -9,10 +9,12 @@ class Vacuum::AccessTokensVacuum private def vacuum_revoked_access_tokens! - Doorkeeper::AccessToken.where.not(revoked_at: nil).where('revoked_at < NOW()').delete_all + Doorkeeper::AccessToken.where.not(expires_in: nil).where('created_at + make_interval(secs => expires_in) < NOW()').in_batches.delete_all + Doorkeeper::AccessToken.where.not(revoked_at: nil).where('revoked_at < NOW()').in_batches.delete_all end def vacuum_revoked_access_grants! - Doorkeeper::AccessGrant.where.not(revoked_at: nil).where('revoked_at < NOW()').delete_all + Doorkeeper::AccessGrant.where.not(expires_in: nil).where('created_at + make_interval(secs => expires_in) < NOW()').in_batches.delete_all + Doorkeeper::AccessGrant.where.not(revoked_at: nil).where('revoked_at < NOW()').in_batches.delete_all end end diff --git a/app/models/account_conversation.rb b/app/models/account_conversation.rb index 45e74bbeb..38ee247cf 100644 --- a/app/models/account_conversation.rb +++ b/app/models/account_conversation.rb @@ -16,34 +16,44 @@ class AccountConversation < ApplicationRecord include Redisable + attr_writer :participant_accounts + + before_validation :set_last_status after_commit :push_to_streaming_api belongs_to :account belongs_to :conversation belongs_to :last_status, class_name: 'Status' - before_validation :set_last_status - def participant_account_ids=(arr) self[:participant_account_ids] = arr.sort + @participant_accounts = nil end def participant_accounts - if participant_account_ids.empty? - [account] - else - participants = Account.where(id: participant_account_ids) - participants.empty? ? [account] : participants - end + @participant_accounts ||= Account.where(id: participant_account_ids).to_a + @participant_accounts.presence || [account] end class << self def to_a_paginated_by_id(limit, options = {}) - if options[:min_id] - paginate_by_min_id(limit, options[:min_id], options[:max_id]).reverse - else - paginate_by_max_id(limit, options[:max_id], options[:since_id]).to_a + array = begin + if options[:min_id] + paginate_by_min_id(limit, options[:min_id], options[:max_id]).reverse + else + paginate_by_max_id(limit, options[:max_id], options[:since_id]).to_a + end end + + # Preload participants + participant_ids = array.flat_map(&:participant_account_ids) + accounts_by_id = Account.where(id: participant_ids).index_by(&:id) + + array.each do |conversation| + conversation.participant_accounts = conversation.participant_account_ids.filter_map { |id| accounts_by_id[id] } + end + + array end def paginate_by_min_id(limit, min_id = nil, max_id = nil) diff --git a/app/models/concerns/attachmentable.rb b/app/models/concerns/attachmentable.rb index 01fae4236..35819003e 100644 --- a/app/models/concerns/attachmentable.rb +++ b/app/models/concerns/attachmentable.rb @@ -22,15 +22,14 @@ module Attachmentable included do def self.has_attached_file(name, options = {}) # rubocop:disable Naming/PredicateName - options = { validate_media_type: false }.merge(options) super(name, options) - send(:"before_#{name}_post_process") do + + send(:"before_#{name}_validate") do attachment = send(name) check_image_dimension(attachment) set_file_content_type(attachment) obfuscate_file_name(attachment) set_file_extension(attachment) - Paperclip::Validators::MediaTypeSpoofDetectionValidator.new(attributes: [name]).validate(self) end end end diff --git a/app/models/form/account_batch.rb b/app/models/form/account_batch.rb index 6a05f8163..4665a5867 100644 --- a/app/models/form/account_batch.rb +++ b/app/models/form/account_batch.rb @@ -123,7 +123,18 @@ class Form::AccountBatch account: current_account, action: :suspend ) + Admin::SuspensionWorker.perform_async(account.id) + + # Suspending a single account closes their associated reports, so + # mass-suspending would be consistent. + Report.where(target_account: account).unresolved.find_each do |report| + authorize(report, :update?) + log_action(:resolve, report) + report.resolve!(current_account) + rescue Mastodon::NotPermittedError + # This should not happen, but just in case, do not fail early + end end def approve_account(account) diff --git a/app/models/identity.rb b/app/models/identity.rb index 8cc65aef4..869f06372 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -12,7 +12,7 @@ # class Identity < ApplicationRecord - belongs_to :user, dependent: :destroy + belongs_to :user validates :uid, presence: true, uniqueness: { scope: :provider } validates :provider, presence: true diff --git a/app/models/webhook.rb b/app/models/webhook.rb index 4aafb1257..bc1c027d5 100644 --- a/app/models/webhook.rb +++ b/app/models/webhook.rb @@ -20,6 +20,8 @@ class Webhook < ApplicationRecord report.created ).freeze + attr_writer :current_account + scope :enabled, -> { where(enabled: true) } validates :url, presence: true, url: true @@ -27,6 +29,7 @@ class Webhook < ApplicationRecord validates :events, presence: true validate :validate_events + validate :validate_permissions before_validation :strip_events before_validation :generate_secret @@ -43,12 +46,29 @@ class Webhook < ApplicationRecord update!(enabled: false) end + def required_permissions + events.map { |event| Webhook.permission_for_event(event) } + end + + def self.permission_for_event(event) + case event + when 'account.approved', 'account.created', 'account.updated' + :manage_users + when 'report.created' + :manage_reports + end + end + private def validate_events errors.add(:events, :invalid) if events.any? { |e| !EVENTS.include?(e) } end + def validate_permissions + errors.add(:events, :invalid_permissions) if defined?(@current_account) && required_permissions.any? { |permission| !@current_account.user_role.can?(permission) } + end + def strip_events self.events = events.map { |str| str.strip.presence }.compact if events.present? end diff --git a/app/policies/webhook_policy.rb b/app/policies/webhook_policy.rb index a2199a333..577e891b6 100644 --- a/app/policies/webhook_policy.rb +++ b/app/policies/webhook_policy.rb @@ -14,7 +14,7 @@ class WebhookPolicy < ApplicationPolicy end def update? - role.can?(:manage_webhooks) + role.can?(:manage_webhooks) && record.required_permissions.all? { |permission| role.can?(permission) } end def enable? @@ -30,6 +30,6 @@ class WebhookPolicy < ApplicationPolicy end def destroy? - role.can?(:manage_webhooks) + role.can?(:manage_webhooks) && record.required_permissions.all? { |permission| role.can?(permission) } end end diff --git a/app/serializers/rest/preview_card_serializer.rb b/app/serializers/rest/preview_card_serializer.rb index 66ff47d22..e6d204fec 100644 --- a/app/serializers/rest/preview_card_serializer.rb +++ b/app/serializers/rest/preview_card_serializer.rb @@ -11,4 +11,8 @@ class REST::PreviewCardSerializer < ActiveModel::Serializer def image object.image? ? full_asset_url(object.image.url(:original)) : nil end + + def html + Sanitize.fragment(object.html, Sanitize::Config::MASTODON_OEMBED) + end end diff --git a/app/services/remove_status_service.rb b/app/services/remove_status_service.rb index 45cfb75f4..c84f90726 100644 --- a/app/services/remove_status_service.rb +++ b/app/services/remove_status_service.rb @@ -12,6 +12,7 @@ class RemoveStatusService < BaseService # @option [Boolean] :immediate # @option [Boolean] :preserve # @option [Boolean] :original_removed + # @option [Boolean] :skip_streaming def call(status, **options) @payload = Oj.dump(event: :delete, payload: status.id.to_s) @status = status @@ -52,6 +53,9 @@ class RemoveStatusService < BaseService private + # The following FeedManager calls all do not result in redis publishes for + # streaming, as the `:update` option is false + def remove_from_self FeedManager.instance.unpush_from_home(@account, @status) end @@ -75,6 +79,8 @@ class RemoveStatusService < BaseService # followers. Here we send a delete to actively mentioned accounts # that may not follow the account + return if skip_streaming? + @status.active_mentions.find_each do |mention| redis.publish("timeline:#{mention.account_id}", @payload) end @@ -103,7 +109,7 @@ class RemoveStatusService < BaseService # without us being able to do all the fancy stuff @status.reblogs.rewhere(deleted_at: [nil, @status.deleted_at]).includes(:account).reorder(nil).find_each do |reblog| - RemoveStatusService.new.call(reblog, original_removed: true) + RemoveStatusService.new.call(reblog, original_removed: true, skip_streaming: skip_streaming?) end end @@ -114,6 +120,8 @@ class RemoveStatusService < BaseService return unless @status.public_visibility? + return if skip_streaming? + @status.tags.map(&:name).each do |hashtag| redis.publish("timeline:hashtag:#{hashtag.mb_chars.downcase}", @payload) redis.publish("timeline:hashtag:#{hashtag.mb_chars.downcase}:local", @payload) if @status.local? @@ -123,6 +131,8 @@ class RemoveStatusService < BaseService def remove_from_public return unless @status.public_visibility? + return if skip_streaming? + redis.publish('timeline:public', @payload) redis.publish(@status.local? ? 'timeline:public:local' : 'timeline:public:remote', @payload) end @@ -130,6 +140,8 @@ class RemoveStatusService < BaseService def remove_from_media return unless @status.public_visibility? + return if skip_streaming? + redis.publish('timeline:public:media', @payload) redis.publish(@status.local? ? 'timeline:public:local:media' : 'timeline:public:remote:media', @payload) end @@ -143,4 +155,8 @@ class RemoveStatusService < BaseService def permanently? @options[:immediate] || !(@options[:preserve] || @status.reported?) end + + def skip_streaming? + !!@options[:skip_streaming] + end end diff --git a/app/services/resolve_url_service.rb b/app/services/resolve_url_service.rb index d8e795f3b..d6e528654 100644 --- a/app/services/resolve_url_service.rb +++ b/app/services/resolve_url_service.rb @@ -89,13 +89,28 @@ class ResolveURLService < BaseService def process_local_url recognized_params = Rails.application.routes.recognize_path(@url) - return unless recognized_params[:action] == 'show' + case recognized_params[:controller] + when 'statuses' + return unless recognized_params[:action] == 'show' - if recognized_params[:controller] == 'statuses' status = Status.find_by(id: recognized_params[:id]) check_local_status(status) - elsif recognized_params[:controller] == 'accounts' + when 'accounts' + return unless recognized_params[:action] == 'show' + Account.find_local(recognized_params[:username]) + when 'home' + return unless recognized_params[:action] == 'index' && recognized_params[:username_with_domain].present? + + if recognized_params[:any]&.match?(/\A[0-9]+\Z/) + status = Status.find_by(id: recognized_params[:any]) + check_local_status(status) + elsif recognized_params[:any].blank? + username, domain = recognized_params[:username_with_domain].gsub(/\A@/, '').split('@') + return unless username.present? && domain.present? + + Account.find_remote(username, domain) + end end end diff --git a/app/validators/vote_validator.rb b/app/validators/vote_validator.rb index b1692562d..4316c59ef 100644 --- a/app/validators/vote_validator.rb +++ b/app/validators/vote_validator.rb @@ -3,8 +3,8 @@ class VoteValidator < ActiveModel::Validator def validate(vote) vote.errors.add(:base, I18n.t('polls.errors.expired')) if vote.poll.expired? - vote.errors.add(:base, I18n.t('polls.errors.invalid_choice')) if invalid_choice?(vote) + vote.errors.add(:base, I18n.t('polls.errors.self_vote')) if self_vote?(vote) if vote.poll.multiple? && vote.poll.votes.where(account: vote.account, choice: vote.choice).exists? vote.errors.add(:base, I18n.t('polls.errors.already_voted')) @@ -18,4 +18,8 @@ class VoteValidator < ActiveModel::Validator def invalid_choice?(vote) vote.choice.negative? || vote.choice >= vote.poll.options.size end + + def self_vote?(vote) + vote.account_id == vote.poll.account_id + end end diff --git a/app/views/admin/webhooks/_form.html.haml b/app/views/admin/webhooks/_form.html.haml index c1e8f8979..6c5ff03dd 100644 --- a/app/views/admin/webhooks/_form.html.haml +++ b/app/views/admin/webhooks/_form.html.haml @@ -5,7 +5,7 @@ = f.input :url, wrapper: :with_block_label, input_html: { placeholder: 'https://' } .fields-group - = f.input :events, collection: Webhook::EVENTS, wrapper: :with_block_label, include_blank: false, as: :check_boxes, collection_wrapper_tag: 'ul', item_wrapper_tag: 'li' + = f.input :events, collection: Webhook::EVENTS, wrapper: :with_block_label, include_blank: false, as: :check_boxes, collection_wrapper_tag: 'ul', item_wrapper_tag: 'li', disabled: Webhook::EVENTS.filter { |event| !current_user.role.can?(Webhook.permission_for_event(event)) } .actions = f.button :button, @webhook.new_record? ? t('admin.webhooks.add_new') : t('generic.save_changes'), type: :submit diff --git a/app/views/oauth/authorized_applications/index.html.haml b/app/views/oauth/authorized_applications/index.html.haml index 0280d8aef..55d8524db 100644 --- a/app/views/oauth/authorized_applications/index.html.haml +++ b/app/views/oauth/authorized_applications/index.html.haml @@ -18,8 +18,8 @@ .announcements-list__item__action-bar .announcements-list__item__meta - - if application.most_recently_used_access_token - = t('doorkeeper.authorized_applications.index.last_used_at', date: l(application.most_recently_used_access_token.last_used_at.to_date)) + - if @last_used_at_by_app[application.id] + = t('doorkeeper.authorized_applications.index.last_used_at', date: l(@last_used_at_by_app[application.id].to_date)) - else = t('doorkeeper.authorized_applications.index.never_used') diff --git a/app/workers/scheduler/accounts_statuses_cleanup_scheduler.rb b/app/workers/scheduler/accounts_statuses_cleanup_scheduler.rb index d245f6bbd..a2ab31cc5 100644 --- a/app/workers/scheduler/accounts_statuses_cleanup_scheduler.rb +++ b/app/workers/scheduler/accounts_statuses_cleanup_scheduler.rb @@ -7,28 +7,30 @@ class Scheduler::AccountsStatusesCleanupScheduler # This limit is mostly to be nice to the fediverse at large and not # generate too much traffic. # This also helps limiting the running time of the scheduler itself. - MAX_BUDGET = 150 + MAX_BUDGET = 300 - # This is an attempt to spread the load across instances, as various - # accounts are likely to have various followers. + # This is an attempt to spread the load across remote servers, as + # spreading deletions across diverse accounts is likely to spread + # the deletion across diverse followers. It also helps each individual + # user see some effect sooner. PER_ACCOUNT_BUDGET = 5 # This is an attempt to limit the workload generated by status removal - # jobs to something the particular instance can handle. - PER_THREAD_BUDGET = 6 + # jobs to something the particular server can handle. + PER_THREAD_BUDGET = 5 - # Those avoid loading an instance that is already under load - MAX_DEFAULT_SIZE = 200 - MAX_DEFAULT_LATENCY = 5 - MAX_PUSH_SIZE = 500 - MAX_PUSH_LATENCY = 10 - - # 'pull' queue has lower priority jobs, and it's unlikely that pushing - # deletes would cause much issues with this queue if it didn't cause issues - # with default and push. Yet, do not enqueue deletes if the instance is - # lagging behind too much. - MAX_PULL_SIZE = 10_000 - MAX_PULL_LATENCY = 5.minutes.to_i + # These are latency limits on various queues above which a server is + # considered to be under load, causing the auto-deletion to be entirely + # skipped for that run. + LOAD_LATENCY_THRESHOLDS = { + default: 5, + push: 10, + # The `pull` queue has lower priority jobs, and it's unlikely that + # pushing deletes would cause much issues with this queue if it didn't + # cause issues with `default` and `push`. Yet, do not enqueue deletes + # if the instance is lagging behind too much. + pull: 5.minutes.to_i, + }.freeze sidekiq_options retry: 0, lock: :until_executed, lock_ttl: 1.day.to_i @@ -36,17 +38,37 @@ class Scheduler::AccountsStatusesCleanupScheduler return if under_load? budget = compute_budget - first_policy_id = last_processed_id + + # If the budget allows it, we want to consider all accounts with enabled + # auto cleanup at least once. + # + # We start from `first_policy_id` (the last processed id in the previous + # run) and process each policy until we loop to `first_policy_id`, + # recording into `affected_policies` any policy that caused posts to be + # deleted. + # + # After that, we set `full_iteration` to `false` and continue looping on + # policies from `affected_policies`. + first_policy_id = last_processed_id || 0 + first_iteration = true + full_iteration = true + affected_policies = [] loop do num_processed_accounts = 0 - scope = AccountStatusesCleanupPolicy.where(enabled: true) - scope.where(Account.arel_table[:id].gt(first_policy_id)) if first_policy_id.present? + scope = cleanup_policies(first_policy_id, affected_policies, first_iteration, full_iteration) scope.find_each(order: :asc) do |policy| num_deleted = AccountStatusesCleanupService.new.call(policy, [budget, PER_ACCOUNT_BUDGET].min) - num_processed_accounts += 1 unless num_deleted.zero? budget -= num_deleted + + unless num_deleted.zero? + num_processed_accounts += 1 + affected_policies << policy.id if full_iteration + end + + full_iteration = false if !first_iteration && policy.id >= first_policy_id + if budget.zero? save_last_processed_id(policy.id) break @@ -55,36 +77,55 @@ class Scheduler::AccountsStatusesCleanupScheduler # The idea here is to loop through all policies at least once until the budget is exhausted # and start back after the last processed account otherwise - break if budget.zero? || (num_processed_accounts.zero? && first_policy_id.nil?) - first_policy_id = nil + break if budget.zero? || (num_processed_accounts.zero? && !full_iteration) + + full_iteration = false unless first_iteration + first_iteration = false end end def compute_budget - threads = Sidekiq::ProcessSet.new.select { |x| x['queues'].include?('push') }.map { |x| x['concurrency'] }.sum + # Each post deletion is a `RemovalWorker` job (on `default` queue), each + # potentially spawning many `ActivityPub::DeliveryWorker` jobs (on the `push` queue). + threads = Sidekiq::ProcessSet.new.select { |x| x['queues'].include?('push') }.pluck('concurrency').sum [PER_THREAD_BUDGET * threads, MAX_BUDGET].min end def under_load? - queue_under_load?('default', MAX_DEFAULT_SIZE, MAX_DEFAULT_LATENCY) || queue_under_load?('push', MAX_PUSH_SIZE, MAX_PUSH_LATENCY) || queue_under_load?('pull', MAX_PULL_SIZE, MAX_PULL_LATENCY) + LOAD_LATENCY_THRESHOLDS.any? { |queue, max_latency| queue_under_load?(queue, max_latency) } end private - def queue_under_load?(name, max_size, max_latency) - queue = Sidekiq::Queue.new(name) - queue.size > max_size || queue.latency > max_latency + def cleanup_policies(first_policy_id, affected_policies, first_iteration, full_iteration) + scope = AccountStatusesCleanupPolicy.where(enabled: true) + + if full_iteration + # If we are doing a full iteration, examine all policies we have not examined yet + if first_iteration + scope.where(id: first_policy_id...) + else + scope.where(id: ..first_policy_id).or(scope.where(id: affected_policies)) + end + else + # Otherwise, examine only policies that previously yielded posts to delete + scope.where(id: affected_policies) + end + end + + def queue_under_load?(name, max_latency) + Sidekiq::Queue.new(name).latency > max_latency end def last_processed_id - redis.get('account_statuses_cleanup_scheduler:last_account_id') + redis.get('account_statuses_cleanup_scheduler:last_policy_id')&.to_i end def save_last_processed_id(id) if id.nil? - redis.del('account_statuses_cleanup_scheduler:last_account_id') + redis.del('account_statuses_cleanup_scheduler:last_policy_id') else - redis.set('account_statuses_cleanup_scheduler:last_account_id', id, ex: 1.hour.seconds) + redis.set('account_statuses_cleanup_scheduler:last_policy_id', id, ex: 1.hour.seconds) end end end diff --git a/app/workers/scheduler/indexing_scheduler.rb b/app/workers/scheduler/indexing_scheduler.rb index c42396629..d622f5586 100644 --- a/app/workers/scheduler/indexing_scheduler.rb +++ b/app/workers/scheduler/indexing_scheduler.rb @@ -6,17 +6,19 @@ class Scheduler::IndexingScheduler sidekiq_options retry: 0 + IMPORT_BATCH_SIZE = 1000 + SCAN_BATCH_SIZE = 10 * IMPORT_BATCH_SIZE + def perform return unless Chewy.enabled? indexes.each do |type| with_redis do |redis| - ids = redis.smembers("chewy:queue:#{type.name}") - - type.import!(ids) - - redis.pipelined do |pipeline| - ids.each { |id| pipeline.srem("chewy:queue:#{type.name}", id) } + redis.sscan_each("chewy:queue:#{type.name}", count: SCAN_BATCH_SIZE).each_slice(IMPORT_BATCH_SIZE) do |ids| + type.import!(ids) + redis.pipelined do |pipeline| + pipeline.srem("chewy:queue:#{type.name}", ids) + end end end end diff --git a/app/workers/scheduler/user_cleanup_scheduler.rb b/app/workers/scheduler/user_cleanup_scheduler.rb index 45cfbc62e..4aee7935a 100644 --- a/app/workers/scheduler/user_cleanup_scheduler.rb +++ b/app/workers/scheduler/user_cleanup_scheduler.rb @@ -24,7 +24,7 @@ class Scheduler::UserCleanupScheduler def clean_discarded_statuses! Status.unscoped.discarded.where('deleted_at <= ?', 30.days.ago).find_in_batches do |statuses| RemovalWorker.push_bulk(statuses) do |status| - [status.id, { 'immediate' => true }] + [status.id, { 'immediate' => true, 'skip_streaming' => true }] end end end diff --git a/config/application.rb b/config/application.rb index f0e65f443..55d98ca99 100644 --- a/config/application.rb +++ b/config/application.rb @@ -28,6 +28,7 @@ require_relative '../lib/paperclip/url_generator_extensions' require_relative '../lib/paperclip/attachment_extensions' require_relative '../lib/paperclip/lazy_thumbnail' require_relative '../lib/paperclip/gif_transcoder' +require_relative '../lib/paperclip/media_type_spoof_detector_extensions' require_relative '../lib/paperclip/transcoder' require_relative '../lib/paperclip/type_corrector' require_relative '../lib/paperclip/response_with_limit_adapter' diff --git a/config/imagemagick/policy.xml b/config/imagemagick/policy.xml new file mode 100644 index 000000000..1052476b3 --- /dev/null +++ b/config/imagemagick/policy.xml @@ -0,0 +1,27 @@ + + + + + + + + + + + + + + + + diff --git a/config/initializers/content_security_policy.rb b/config/initializers/content_security_policy.rb index cb5629337..1f66155c4 100644 --- a/config/initializers/content_security_policy.rb +++ b/config/initializers/content_security_policy.rb @@ -3,7 +3,7 @@ # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy def host_to_url(str) - "http#{Rails.configuration.x.use_https ? 's' : ''}://#{str}" unless str.blank? + "http#{Rails.configuration.x.use_https ? 's' : ''}://#{str}".split('/').first if str.present? end base_host = Rails.configuration.x.web_domain diff --git a/config/initializers/paperclip.rb b/config/initializers/paperclip.rb index 63f0d9240..7ed244e0c 100644 --- a/config/initializers/paperclip.rb +++ b/config/initializers/paperclip.rb @@ -155,3 +155,10 @@ unless defined?(Seahorse) end end end + +# Set our ImageMagick security policy, but allow admins to override it +ENV['MAGICK_CONFIGURE_PATH'] = begin + imagemagick_config_paths = ENV.fetch('MAGICK_CONFIGURE_PATH', '').split(File::PATH_SEPARATOR) + imagemagick_config_paths << Rails.root.join('config', 'imagemagick').expand_path.to_s + imagemagick_config_paths.join(File::PATH_SEPARATOR) +end diff --git a/config/initializers/twitter_regex.rb b/config/initializers/twitter_regex.rb index 6a7723fd2..e65b05dfd 100644 --- a/config/initializers/twitter_regex.rb +++ b/config/initializers/twitter_regex.rb @@ -25,7 +25,7 @@ module Twitter::TwitterText \) /iox UCHARS = '\u{A0}-\u{D7FF}\u{F900}-\u{FDCF}\u{FDF0}-\u{FFEF}\u{10000}-\u{1FFFD}\u{20000}-\u{2FFFD}\u{30000}-\u{3FFFD}\u{40000}-\u{4FFFD}\u{50000}-\u{5FFFD}\u{60000}-\u{6FFFD}\u{70000}-\u{7FFFD}\u{80000}-\u{8FFFD}\u{90000}-\u{9FFFD}\u{A0000}-\u{AFFFD}\u{B0000}-\u{BFFFD}\u{C0000}-\u{CFFFD}\u{D0000}-\u{DFFFD}\u{E1000}-\u{EFFFD}\u{E000}-\u{F8FF}\u{F0000}-\u{FFFFD}\u{100000}-\u{10FFFD}' - REGEXEN[:valid_url_query_chars] = /[a-z0-9!?\*'\(\);:&=\+\$\/%#\[\]\-_\.,~|@#{UCHARS}]/iou + REGEXEN[:valid_url_query_chars] = /[a-z0-9!?\*'\(\);:&=\+\$\/%#\[\]\-_\.,~|@\^#{UCHARS}]/iou REGEXEN[:valid_url_query_ending_chars] = /[a-z0-9_&=#\/\-#{UCHARS}]/iou REGEXEN[:valid_url_path] = /(?: (?: diff --git a/config/locales/activerecord.en.yml b/config/locales/activerecord.en.yml index 8aee15659..a53c7c6e9 100644 --- a/config/locales/activerecord.en.yml +++ b/config/locales/activerecord.en.yml @@ -53,3 +53,7 @@ en: position: elevated: cannot be higher than your current role own_role: cannot be changed with your current role + webhook: + attributes: + events: + invalid_permissions: cannot include events you don't have the rights to diff --git a/config/locales/en.yml b/config/locales/en.yml index b1e2a8568..ce20d5d1d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1387,6 +1387,7 @@ en: expired: The poll has already ended invalid_choice: The chosen vote option does not exist over_character_limit: cannot be longer than %{max} characters each + self_vote: You cannot vote in your own polls too_few_options: must have more than one item too_many_options: can't contain more than %{max} items preferences: diff --git a/dist/nginx.conf b/dist/nginx.conf index 4608a3a16..416700370 100644 --- a/dist/nginx.conf +++ b/dist/nginx.conf @@ -90,6 +90,8 @@ server { location ~ ^/system/ { add_header Cache-Control "public, max-age=2419200, immutable"; add_header Strict-Transport-Security "max-age=63072000; includeSubDomains"; + add_header X-Content-Type-Options nosniff; + add_header Content-Security-Policy "default-src 'none'; form-action 'none'"; try_files $uri =404; } diff --git a/lib/mastodon/accounts_cli.rb b/lib/mastodon/accounts_cli.rb index be93bba59..d4a6bbc12 100644 --- a/lib/mastodon/accounts_cli.rb +++ b/lib/mastodon/accounts_cli.rb @@ -542,7 +542,7 @@ module Mastodon User.pending.find_each(&:approve!) say('OK', :green) elsif options[:number] - User.pending.limit(options[:number]).each(&:approve!) + User.pending.order(created_at: :asc).limit(options[:number]).each(&:approve!) say('OK', :green) elsif username.present? account = Account.find_local(username) diff --git a/lib/mastodon/version.rb b/lib/mastodon/version.rb index 71bcfb4e1..9a8d5ec88 100644 --- a/lib/mastodon/version.rb +++ b/lib/mastodon/version.rb @@ -13,7 +13,7 @@ module Mastodon end def patch - 2 + 3 end def flags diff --git a/lib/paperclip/media_type_spoof_detector_extensions.rb b/lib/paperclip/media_type_spoof_detector_extensions.rb new file mode 100644 index 000000000..a406ef312 --- /dev/null +++ b/lib/paperclip/media_type_spoof_detector_extensions.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module Paperclip + module MediaTypeSpoofDetectorExtensions + def calculated_content_type + return @calculated_content_type if defined?(@calculated_content_type) + + @calculated_content_type = type_from_file_command.chomp + + # The `file` command fails to recognize some MP3 files as such + @calculated_content_type = type_from_marcel if @calculated_content_type == 'application/octet-stream' && type_from_marcel == 'audio/mpeg' + @calculated_content_type + end + + def type_from_marcel + @type_from_marcel ||= Marcel::MimeType.for Pathname.new(@file.path), + name: @file.path + end + end +end + +Paperclip::MediaTypeSpoofDetector.prepend(Paperclip::MediaTypeSpoofDetectorExtensions) diff --git a/lib/paperclip/transcoder.rb b/lib/paperclip/transcoder.rb index afd9f58ff..be40b4924 100644 --- a/lib/paperclip/transcoder.rb +++ b/lib/paperclip/transcoder.rb @@ -19,10 +19,7 @@ module Paperclip def make metadata = VideoMetadataExtractor.new(@file.path) - unless metadata.valid? - Paperclip.log("Unsupported file #{@file.path}") - return File.open(@file.path) - end + raise Paperclip::Error, "Error while transcoding #{@file.path}: unsupported file" unless metadata.valid? update_attachment_type(metadata) update_options_from_metadata(metadata) diff --git a/lib/public_file_server_middleware.rb b/lib/public_file_server_middleware.rb index 3799230a2..7e02e37a0 100644 --- a/lib/public_file_server_middleware.rb +++ b/lib/public_file_server_middleware.rb @@ -32,6 +32,11 @@ class PublicFileServerMiddleware end end + # Override the default CSP header set by the CSP middleware + headers['Content-Security-Policy'] = "default-src 'none'; form-action 'none'" if request_path.start_with?(paperclip_root_url) + + headers['X-Content-Type-Options'] = 'nosniff' + [status, headers, response] end diff --git a/lib/sanitize_ext/sanitize_config.rb b/lib/sanitize_ext/sanitize_config.rb index baf652662..703ba8b05 100644 --- a/lib/sanitize_ext/sanitize_config.rb +++ b/lib/sanitize_ext/sanitize_config.rb @@ -94,26 +94,26 @@ class Sanitize ] ) - MASTODON_OEMBED ||= freeze_config merge( - RELAXED, - elements: RELAXED[:elements] + %w(audio embed iframe source video), + MASTODON_OEMBED ||= freeze_config( + elements: %w(audio embed iframe source video), - attributes: merge( - RELAXED[:attributes], + attributes: { 'audio' => %w(controls), 'embed' => %w(height src type width), 'iframe' => %w(allowfullscreen frameborder height scrolling src width), 'source' => %w(src type), 'video' => %w(controls height loop width), - 'div' => [:data] - ), + }, - protocols: merge( - RELAXED[:protocols], + protocols: { 'embed' => { 'src' => HTTP_PROTOCOLS }, 'iframe' => { 'src' => HTTP_PROTOCOLS }, - 'source' => { 'src' => HTTP_PROTOCOLS } - ) + 'source' => { 'src' => HTTP_PROTOCOLS }, + }, + + add_attributes: { + 'iframe' => { 'sandbox' => 'allow-scripts allow-same-origin allow-popups allow-popups-to-escape-sandbox allow-forms' }, + } ) end end diff --git a/spec/controllers/api/v1/conversations_controller_spec.rb b/spec/controllers/api/v1/conversations_controller_spec.rb index 5add7cf1d..1ec26d520 100644 --- a/spec/controllers/api/v1/conversations_controller_spec.rb +++ b/spec/controllers/api/v1/conversations_controller_spec.rb @@ -16,6 +16,7 @@ RSpec.describe Api::V1::ConversationsController, type: :controller do before do PostStatusService.new.call(other.account, text: 'Hey @alice', visibility: 'direct') + PostStatusService.new.call(user.account, text: 'Hey, nobody here', visibility: 'direct') end it 'returns http success' do @@ -31,7 +32,26 @@ RSpec.describe Api::V1::ConversationsController, type: :controller do it 'returns conversations' do get :index json = body_as_json - expect(json.size).to eq 1 + expect(json.size).to eq 2 + expect(json[0][:accounts].size).to eq 1 + end + + context 'with since_id' do + context 'when requesting old posts' do + it 'returns conversations' do + get :index, params: { since_id: Mastodon::Snowflake.id_at(1.hour.ago, with_random: false) } + json = body_as_json + expect(json.size).to eq 2 + end + end + + context 'when requesting posts in the future' do + it 'returns no conversation' do + get :index, params: { since_id: Mastodon::Snowflake.id_at(1.hour.from_now, with_random: false) } + json = body_as_json + expect(json.size).to eq 0 + end + end end end end diff --git a/spec/controllers/api/v1/statuses/histories_controller_spec.rb b/spec/controllers/api/v1/statuses/histories_controller_spec.rb index 00677f1d2..99384c8ed 100644 --- a/spec/controllers/api/v1/statuses/histories_controller_spec.rb +++ b/spec/controllers/api/v1/statuses/histories_controller_spec.rb @@ -23,6 +23,7 @@ describe Api::V1::Statuses::HistoriesController do it 'returns http success' do expect(response).to have_http_status(200) + expect(body_as_json.size).to_not be 0 end end end diff --git a/spec/controllers/api/v2/admin/accounts_controller_spec.rb b/spec/controllers/api/v2/admin/accounts_controller_spec.rb index 2508a9e05..ebec4a13e 100644 --- a/spec/controllers/api/v2/admin/accounts_controller_spec.rb +++ b/spec/controllers/api/v2/admin/accounts_controller_spec.rb @@ -69,5 +69,13 @@ RSpec.describe Api::V2::Admin::AccountsController, type: :controller do end end end + + context 'with limit param' do + let(:params) { { limit: 1 } } + + it 'sets the correct pagination headers' do + expect(response.headers['Link'].find_link(%w(rel next)).href).to eq api_v2_admin_accounts_url(limit: 1, max_id: admin_account.id) + end + end end end diff --git a/spec/controllers/well_known/webfinger_controller_spec.rb b/spec/controllers/well_known/webfinger_controller_spec.rb index 8574d369d..0e7b34f47 100644 --- a/spec/controllers/well_known/webfinger_controller_spec.rb +++ b/spec/controllers/well_known/webfinger_controller_spec.rb @@ -4,6 +4,10 @@ describe WellKnown::WebfingerController, type: :controller do render_views describe 'GET #show' do + subject(:perform_show!) do + get :show, params: { resource: resource }, format: :json + end + let(:alternate_domains) { [] } let(:alice) { Fabricate(:account, username: 'alice') } let(:resource) { nil } @@ -15,10 +19,6 @@ describe WellKnown::WebfingerController, type: :controller do Rails.configuration.x.alternate_domains = tmp end - subject do - get :show, params: { resource: resource }, format: :json - end - shared_examples 'a successful response' do it 'returns http success' do expect(response).to have_http_status(200) @@ -43,7 +43,7 @@ describe WellKnown::WebfingerController, type: :controller do let(:resource) { alice.to_webfinger_s } before do - subject + perform_show! end it_behaves_like 'a successful response' @@ -54,7 +54,7 @@ describe WellKnown::WebfingerController, type: :controller do before do alice.suspend! - subject + perform_show! end it_behaves_like 'a successful response' @@ -66,7 +66,7 @@ describe WellKnown::WebfingerController, type: :controller do before do alice.suspend! alice.deletion_request.destroy - subject + perform_show! end it 'returns http gone' do @@ -78,7 +78,7 @@ describe WellKnown::WebfingerController, type: :controller do let(:resource) { 'acct:not@existing.com' } before do - subject + perform_show! end it 'returns http not found' do @@ -90,7 +90,7 @@ describe WellKnown::WebfingerController, type: :controller do let(:alternate_domains) { ['foo.org'] } before do - subject + perform_show! end context 'when an account exists' do @@ -114,11 +114,39 @@ describe WellKnown::WebfingerController, type: :controller do end end + context 'when the old name scheme is used to query the instance actor' do + let(:resource) do + "#{Rails.configuration.x.local_domain}@#{Rails.configuration.x.local_domain}" + end + + before do + perform_show! + end + + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'does not set a Vary header' do + expect(response.headers['Vary']).to be_nil + end + + it 'returns application/jrd+json' do + expect(response.media_type).to eq 'application/jrd+json' + end + + it 'returns links for the internal account' do + json = body_as_json + expect(json[:subject]).to eq 'acct:mastodon.internal@cb6e6126.ngrok.io' + expect(json[:aliases]).to eq ['https://cb6e6126.ngrok.io/actor'] + end + end + context 'with no resource parameter' do let(:resource) { nil } before do - subject + perform_show! end it 'returns http bad request' do @@ -130,7 +158,7 @@ describe WellKnown::WebfingerController, type: :controller do let(:resource) { 'df/:dfkj' } before do - subject + perform_show! end it 'returns http bad request' do diff --git a/spec/fixtures/files/boop.mp3 b/spec/fixtures/files/boop.mp3 new file mode 100644 index 000000000..ba106a3a3 Binary files /dev/null and b/spec/fixtures/files/boop.mp3 differ diff --git a/spec/lib/account_reach_finder_spec.rb b/spec/lib/account_reach_finder_spec.rb new file mode 100644 index 000000000..1da95ba6b --- /dev/null +++ b/spec/lib/account_reach_finder_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe AccountReachFinder do + let(:account) { Fabricate(:account) } + + let(:follower1) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://example.com/inbox-1') } + let(:follower2) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://example.com/inbox-2') } + let(:follower3) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://foo.bar/users/a/inbox', shared_inbox_url: 'https://foo.bar/inbox') } + + let(:mentioned1) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://foo.bar/users/b/inbox', shared_inbox_url: 'https://foo.bar/inbox') } + let(:mentioned2) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://example.com/inbox-3') } + let(:mentioned3) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://example.com/inbox-4') } + + let(:unrelated_account) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://example.com/unrelated-inbox') } + + before do + follower1.follow!(account) + follower2.follow!(account) + follower3.follow!(account) + + Fabricate(:status, account: account).tap do |status| + status.mentions << Mention.new(account: follower1) + status.mentions << Mention.new(account: mentioned1) + end + + Fabricate(:status, account: account) + + Fabricate(:status, account: account).tap do |status| + status.mentions << Mention.new(account: mentioned2) + status.mentions << Mention.new(account: mentioned3) + end + + Fabricate(:status).tap do |status| + status.mentions << Mention.new(account: unrelated_account) + end + end + + describe '#inboxes' do + it 'includes the preferred inbox URL of followers' do + expect(described_class.new(account).inboxes).to include(*[follower1, follower2, follower3].map(&:preferred_inbox_url)) + end + + it 'includes the preferred inbox URL of recently-mentioned accounts' do + expect(described_class.new(account).inboxes).to include(*[mentioned1, mentioned2, mentioned3].map(&:preferred_inbox_url)) + end + + it 'does not include the inbox of unrelated users' do + expect(described_class.new(account).inboxes).to_not include(unrelated_account.preferred_inbox_url) + end + end +end diff --git a/spec/lib/vacuum/access_tokens_vacuum_spec.rb b/spec/lib/vacuum/access_tokens_vacuum_spec.rb index 0244c3449..39c8cdb39 100644 --- a/spec/lib/vacuum/access_tokens_vacuum_spec.rb +++ b/spec/lib/vacuum/access_tokens_vacuum_spec.rb @@ -5,9 +5,11 @@ RSpec.describe Vacuum::AccessTokensVacuum do describe '#perform' do let!(:revoked_access_token) { Fabricate(:access_token, revoked_at: 1.minute.ago) } + let!(:expired_access_token) { Fabricate(:access_token, expires_in: 59.minutes.to_i, created_at: 1.hour.ago) } let!(:active_access_token) { Fabricate(:access_token) } let!(:revoked_access_grant) { Fabricate(:access_grant, revoked_at: 1.minute.ago) } + let!(:expired_access_grant) { Fabricate(:access_grant, expires_in: 59.minutes.to_i, created_at: 1.hour.ago) } let!(:active_access_grant) { Fabricate(:access_grant) } before do @@ -18,10 +20,18 @@ RSpec.describe Vacuum::AccessTokensVacuum do expect { revoked_access_token.reload }.to raise_error ActiveRecord::RecordNotFound end + it 'deletes expired access tokens' do + expect { expired_access_token.reload }.to raise_error ActiveRecord::RecordNotFound + end + it 'deletes revoked access grants' do expect { revoked_access_grant.reload }.to raise_error ActiveRecord::RecordNotFound end + it 'deletes expired access grants' do + expect { expired_access_grant.reload }.to raise_error ActiveRecord::RecordNotFound + end + it 'does not delete active access tokens' do expect { active_access_token.reload }.to_not raise_error end diff --git a/spec/models/form/account_batch_spec.rb b/spec/models/form/account_batch_spec.rb new file mode 100644 index 000000000..fd8e90901 --- /dev/null +++ b/spec/models/form/account_batch_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Form::AccountBatch do + let(:account_batch) { described_class.new } + + describe '#save' do + subject { account_batch.save } + + let(:account) { Fabricate(:user, role: UserRole.find_by(name: 'Admin')).account } + let(:account_ids) { [] } + let(:query) { Account.none } + + before do + account_batch.assign_attributes( + action: action, + current_account: account, + account_ids: account_ids, + query: query, + select_all_matching: select_all_matching + ) + end + + context 'when action is "suspend"' do + let(:action) { 'suspend' } + + let(:target_account) { Fabricate(:account) } + let(:target_account2) { Fabricate(:account) } + + before do + Fabricate(:report, target_account: target_account) + Fabricate(:report, target_account: target_account2) + end + + context 'when accounts are passed as account_ids' do + let(:select_all_matching) { '0' } + let(:account_ids) { [target_account.id, target_account2.id] } + + it 'suspends the expected users' do + expect { subject }.to change { [target_account.reload.suspended?, target_account2.reload.suspended?] }.from([false, false]).to([true, true]) + end + + it 'closes open reports targeting the suspended users' do + expect { subject }.to change { Report.unresolved.where(target_account: [target_account, target_account2]).count }.from(2).to(0) + end + end + + context 'when accounts are passed as a query' do + let(:select_all_matching) { '1' } + let(:query) { Account.where(id: [target_account.id, target_account2.id]) } + + it 'suspends the expected users' do + expect { subject }.to change { [target_account.reload.suspended?, target_account2.reload.suspended?] }.from([false, false]).to([true, true]) + end + + it 'closes open reports targeting the suspended users' do + expect { subject }.to change { Report.unresolved.where(target_account: [target_account, target_account2]).count }.from(2).to(0) + end + end + end + end +end diff --git a/spec/models/media_attachment_spec.rb b/spec/models/media_attachment_spec.rb index 29fd313ae..c3283ccb0 100644 --- a/spec/models/media_attachment_spec.rb +++ b/spec/models/media_attachment_spec.rb @@ -150,6 +150,26 @@ RSpec.describe MediaAttachment, type: :model do end end + describe 'mp3 with large cover art' do + let(:media) { described_class.create(account: Fabricate(:account), file: attachment_fixture('boop.mp3')) } + + it 'detects it as an audio file' do + expect(media.type).to eq 'audio' + end + + it 'sets meta for the duration' do + expect(media.file.meta['original']['duration']).to be_within(0.05).of(0.235102) + end + + it 'extracts thumbnail' do + expect(media.thumbnail.present?).to be true + end + + it 'gives the file a random name' do + expect(media.file_file_name).to_not eq 'boop.mp3' + end + end + describe 'jpeg' do let(:media) { MediaAttachment.create(account: Fabricate(:account), file: attachment_fixture('attachment.jpg')) } diff --git a/spec/services/fetch_link_card_service_spec.rb b/spec/services/fetch_link_card_service_spec.rb index 4914c2753..7a758f910 100644 --- a/spec/services/fetch_link_card_service_spec.rb +++ b/spec/services/fetch_link_card_service_spec.rb @@ -10,6 +10,7 @@ RSpec.describe FetchLinkCardService, type: :service do stub_request(:get, 'http://example.com/koi8-r').to_return(request_fixture('koi8-r.txt')) stub_request(:get, 'http://example.com/日本語').to_return(request_fixture('sjis.txt')) stub_request(:get, 'https://github.com/qbi/WannaCry').to_return(status: 404) + stub_request(:get, 'http://example.com/test?data=file.gpx%5E1').to_return(status: 200) stub_request(:get, 'http://example.com/test-').to_return(request_fixture('idn.txt')) stub_request(:get, 'http://example.com/windows-1251').to_return(request_fixture('windows-1251.txt')) @@ -85,6 +86,15 @@ RSpec.describe FetchLinkCardService, type: :service do expect(a_request(:get, 'http://example.com/sjis')).to_not have_been_made end end + + context do + let(:status) { Fabricate(:status, text: 'test http://example.com/test?data=file.gpx^1') } + + it 'does fetch URLs with a caret in search params' do + expect(a_request(:get, 'http://example.com/test?data=file.gpx')).to_not have_been_made + expect(a_request(:get, 'http://example.com/test?data=file.gpx%5E1')).to have_been_made.once + end + end end context 'in a remote status' do diff --git a/spec/services/resolve_url_service_spec.rb b/spec/services/resolve_url_service_spec.rb index b3e3defbf..ab5b50b76 100644 --- a/spec/services/resolve_url_service_spec.rb +++ b/spec/services/resolve_url_service_spec.rb @@ -145,5 +145,35 @@ describe ResolveURLService, type: :service do expect(subject.call(url, on_behalf_of: account)).to eq(status) end end + + context 'when searching for a local link of a remote private status' do + let(:account) { Fabricate(:account) } + let(:poster) { Fabricate(:account, username: 'foo', domain: 'example.com') } + let(:url) { 'https://example.com/@foo/42' } + let(:uri) { 'https://example.com/users/foo/statuses/42' } + let!(:status) { Fabricate(:status, url: url, uri: uri, account: poster, visibility: :private) } + let(:search_url) { "https://#{Rails.configuration.x.local_domain}/@foo@example.com/#{status.id}" } + + before do + stub_request(:get, url).to_return(status: 404) if url.present? + stub_request(:get, uri).to_return(status: 404) + end + + context 'when the account follows the poster' do + before do + account.follow!(poster) + end + + it 'returns the status' do + expect(subject.call(search_url, on_behalf_of: account)).to eq(status) + end + end + + context 'when the account does not follow the poster' do + it 'does not return the status' do + expect(subject.call(search_url, on_behalf_of: account)).to be_nil + end + end + end end end diff --git a/spec/workers/scheduler/accounts_statuses_cleanup_scheduler_spec.rb b/spec/workers/scheduler/accounts_statuses_cleanup_scheduler_spec.rb index d953cc39d..0b0c4dd48 100644 --- a/spec/workers/scheduler/accounts_statuses_cleanup_scheduler_spec.rb +++ b/spec/workers/scheduler/accounts_statuses_cleanup_scheduler_spec.rb @@ -7,11 +7,13 @@ describe Scheduler::AccountsStatusesCleanupScheduler do let!(:account2) { Fabricate(:account, domain: nil) } let!(:account3) { Fabricate(:account, domain: nil) } let!(:account4) { Fabricate(:account, domain: nil) } + let!(:account5) { Fabricate(:account, domain: nil) } let!(:remote) { Fabricate(:account) } let!(:policy1) { Fabricate(:account_statuses_cleanup_policy, account: account1) } let!(:policy2) { Fabricate(:account_statuses_cleanup_policy, account: account3) } let!(:policy3) { Fabricate(:account_statuses_cleanup_policy, account: account4, enabled: false) } + let!(:policy4) { Fabricate(:account_statuses_cleanup_policy, account: account5) } let(:queue_size) { 0 } let(:queue_latency) { 0 } @@ -40,6 +42,7 @@ describe Scheduler::AccountsStatusesCleanupScheduler do Fabricate(:status, account: account2, created_at: 3.years.ago) Fabricate(:status, account: account3, created_at: 3.years.ago) Fabricate(:status, account: account4, created_at: 3.years.ago) + Fabricate(:status, account: account5, created_at: 3.years.ago) Fabricate(:status, account: remote, created_at: 3.years.ago) end @@ -70,7 +73,7 @@ describe Scheduler::AccountsStatusesCleanupScheduler do end end - describe '#get_budget' do + describe '#compute_budget' do context 'on a single thread' do let(:process_set_stub) { [ { 'concurrency' => 1, 'queues' => ['push', 'default'] } ] } @@ -109,8 +112,48 @@ describe Scheduler::AccountsStatusesCleanupScheduler do expect { subject.perform }.to_not change { account4.statuses.count } end - it 'eventually deletes every deletable toot' do - expect { subject.perform; subject.perform; subject.perform; subject.perform }.to change { Status.count }.by(-20) + it 'eventually deletes every deletable toot given enough runs' do + stub_const 'Scheduler::AccountsStatusesCleanupScheduler::MAX_BUDGET', 4 + + expect { 10.times { subject.perform } }.to change { Status.count }.by(-30) + end + + it 'correctly round-trips between users across several runs' do + stub_const 'Scheduler::AccountsStatusesCleanupScheduler::MAX_BUDGET', 3 + stub_const 'Scheduler::AccountsStatusesCleanupScheduler::PER_ACCOUNT_BUDGET', 2 + + expect { 3.times { subject.perform } } + .to change { Status.count }.by(-3 * 3) + .and change { account1.statuses.count } + .and change { account3.statuses.count } + .and change { account5.statuses.count } + end + + context 'when given a big budget' do + let(:process_set_stub) { [{ 'concurrency' => 400, 'queues' => %w(push default) }] } + + before do + stub_const 'Scheduler::AccountsStatusesCleanupScheduler::MAX_BUDGET', 400 + end + + it 'correctly handles looping in a single run' do + expect(subject.compute_budget).to eq(400) + expect { subject.perform }.to change { Status.count }.by(-30) + end + end + + context 'when there is no work to be done' do + let(:process_set_stub) { [{ 'concurrency' => 400, 'queues' => %w(push default) }] } + + before do + stub_const 'Scheduler::AccountsStatusesCleanupScheduler::MAX_BUDGET', 400 + subject.perform + end + + it 'does not get stuck' do + expect(subject.compute_budget).to eq(400) + expect { subject.perform }.to_not change { Status.count } + end end end end diff --git a/streaming/index.js b/streaming/index.js index 32e3babaa..ffb570c5b 100644 --- a/streaming/index.js +++ b/streaming/index.js @@ -92,18 +92,31 @@ const redisUrlToClient = async (defaultConfig, redisUrl) => { const numWorkers = +process.env.STREAMING_CLUSTER_NUM || (env === 'development' ? 1 : Math.max(os.cpus().length - 1, 1)); /** + * Attempts to safely parse a string as JSON, used when both receiving a message + * from redis and when receiving a message from a client over a websocket + * connection, this is why it accepts a `req` argument. * @param {string} json - * @param {any} req - * @return {Object.|null} + * @param {any?} req + * @returns {Object.|null} */ const parseJSON = (json, req) => { try { return JSON.parse(json); } catch (err) { - if (req.accountId) { - log.warn(req.requestId, `Error parsing message from user ${req.accountId}: ${err}`); + /* FIXME: This logging isn't great, and should probably be done at the + * call-site of parseJSON, not in the method, but this would require changing + * the signature of parseJSON to return something akin to a Result type: + * [Error|null, null|Object { const redisPrefix = redisNamespace ? `${redisNamespace}:` : ''; /** - * @type {Object.>} + * @type {Object.): void>>} */ const subs = {}; @@ -207,7 +220,10 @@ const startWorker = async (workerId) => { return; } - callbacks.forEach(callback => callback(message)); + const json = parseJSON(message, null); + if (!json) return; + + callbacks.forEach(callback => callback(json)); }; /** @@ -229,6 +245,7 @@ const startWorker = async (workerId) => { /** * @param {string} channel + * @param {function(Object): void} callback */ const unsubscribe = (channel, callback) => { log.silly(`Removing listener for ${channel}`); @@ -378,7 +395,7 @@ const startWorker = async (workerId) => { /** * @param {any} req - * @return {string} + * @returns {string|undefined} */ const channelNameFromPath = req => { const { path, query } = req; @@ -487,15 +504,11 @@ const startWorker = async (workerId) => { /** * @param {any} req * @param {SystemMessageHandlers} eventHandlers - * @return {function(string): void} + * @returns {function(object): void} */ const createSystemMessageListener = (req, eventHandlers) => { return message => { - const json = parseJSON(message, req); - - if (!json) return; - - const { event } = json; + const { event } = message; log.silly(req.requestId, `System message for ${req.accountId}: ${event}`); @@ -612,19 +625,16 @@ const startWorker = async (workerId) => { * @param {function(string, string): void} output * @param {function(string[], function(string): void): void} attachCloseHandler * @param {boolean=} needsFiltering - * @return {function(string): void} + * @returns {function(object): void} */ const streamFrom = (ids, req, output, attachCloseHandler, needsFiltering = false) => { const accountId = req.accountId || req.remoteAddress; log.verbose(req.requestId, `Starting stream from ${ids.join(', ')} for ${accountId}`); + // Currently message is of type string, soon it'll be Record const listener = message => { - const json = parseJSON(message, req); - - if (!json) return; - - const { event, payload, queued_at } = json; + const { event, payload, queued_at } = message; const transmit = () => { const now = new Date().getTime(); @@ -1207,8 +1217,15 @@ const startWorker = async (workerId) => { ws.on('close', onEnd); ws.on('error', onEnd); - ws.on('message', data => { - const json = parseJSON(data, session.request); + ws.on('message', (data, isBinary) => { + if (isBinary) { + log.warn('socket', 'Received binary data, closing connection'); + ws.close(1003, 'The mastodon streaming server does not support binary messages'); + return; + } + const message = data.toString('utf8'); + + const json = parseJSON(message, session.request); if (!json) return;