From 7aaf2b44ec698fd4f20b927fcac7edc0394a2647 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 30 Jun 2020 23:58:02 +0200 Subject: [PATCH 1/6] Fix remote files not using Content-Type header, streaming (#14184) --- app/lib/response_with_limit.rb | 10 ++++ app/models/concerns/attachmentable.rb | 17 ++++-- app/models/concerns/remotable.rb | 40 +------------- config/application.rb | 2 + config/initializers/paperclip.rb | 1 + lib/paperclip/image_extractor.rb | 45 ++++++++------- .../media_type_spoof_detector_extensions.rb | 27 +++++++++ lib/paperclip/response_with_limit_adapter.rb | 55 +++++++++++++++++++ spec/models/concerns/remotable_spec.rb | 10 +--- 9 files changed, 139 insertions(+), 68 deletions(-) create mode 100644 app/lib/response_with_limit.rb create mode 100644 lib/paperclip/media_type_spoof_detector_extensions.rb create mode 100644 lib/paperclip/response_with_limit_adapter.rb diff --git a/app/lib/response_with_limit.rb b/app/lib/response_with_limit.rb new file mode 100644 index 00000000000..2cc17bc5fe4 --- /dev/null +++ b/app/lib/response_with_limit.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class ResponseWithLimit + def initialize(response, limit) + @response = response + @limit = limit + end + + attr_reader :response, :limit +end diff --git a/app/models/concerns/attachmentable.rb b/app/models/concerns/attachmentable.rb index 18b872c1e61..c5febb82868 100644 --- a/app/models/concerns/attachmentable.rb +++ b/app/models/concerns/attachmentable.rb @@ -8,6 +8,17 @@ module Attachmentable MAX_MATRIX_LIMIT = 16_777_216 # 4096x4096px or approx. 16MB GIF_MATRIX_LIMIT = 921_600 # 1280x720px + # For some file extensions, there exist different content + # type variants, and browsers often send the wrong one, + # for example, sending an audio .ogg file as video/ogg, + # likewise, MimeMagic also misreports them as such. For + # those files, it is necessary to use the output of the + # `file` utility instead + INCORRECT_CONTENT_TYPES = %w( + video/ogg + video/webm + ).freeze + included do before_post_process :obfuscate_file_name before_post_process :set_file_extensions @@ -21,7 +32,7 @@ module Attachmentable self.class.attachment_definitions.each_key do |attachment_name| attachment = send(attachment_name) - next if attachment.blank? || attachment.queued_for_write[:original].blank? + next if attachment.blank? || attachment.queued_for_write[:original].blank? || !INCORRECT_CONTENT_TYPES.include?(attachment.instance_read(:content_type)) attachment.instance_write :content_type, calculated_content_type(attachment) end @@ -63,9 +74,7 @@ module Attachmentable end def calculated_content_type(attachment) - content_type = Paperclip.run('file', '-b --mime :file', file: attachment.queued_for_write[:original].path).split(/[:;\s]+/).first.chomp - content_type = 'video/mp4' if content_type == 'video/x-m4v' - content_type + Paperclip.run('file', '-b --mime :file', file: attachment.queued_for_write[:original].path).split(/[:;\s]+/).first.chomp rescue Terrapin::CommandLineError '' end diff --git a/app/models/concerns/remotable.rb b/app/models/concerns/remotable.rb index 53ebc08357b..c6d0c7f1f44 100644 --- a/app/models/concerns/remotable.rb +++ b/app/models/concerns/remotable.rb @@ -24,28 +24,16 @@ module Remotable Request.new(:get, url).perform do |response| raise Mastodon::UnexpectedResponseError, response unless (200...300).cover?(response.code) - content_type = parse_content_type(response.headers.get('content-type').last) - extname = detect_extname_from_content_type(content_type) - - if extname.nil? - disposition = response.headers.get('content-disposition').last - matches = disposition&.match(/filename="([^"]*)"/) - filename = matches.nil? ? parsed_url.path.split('/').last : matches[1] - extname = filename.nil? ? '' : File.extname(filename) - end - - basename = SecureRandom.hex(8) - - public_send("#{attachment_name}_file_name=", basename + extname) - public_send("#{attachment_name}=", StringIO.new(response.body_with_limit(limit))) + public_send("#{attachment_name}=", ResponseWithLimit.new(response, limit)) end rescue Mastodon::UnexpectedResponseError, HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError => e Rails.logger.debug "Error fetching remote #{attachment_name}: #{e}" raise e unless suppress_errors rescue Paperclip::Errors::NotIdentifiedByImageMagickError, Addressable::URI::InvalidURIError, Mastodon::HostValidationError, Mastodon::LengthValidationError, Paperclip::Error, Mastodon::DimensionsValidationError => e Rails.logger.debug "Error fetching remote #{attachment_name}: #{e}" - nil end + + nil end define_method("#{attribute_name}=") do |url| @@ -59,26 +47,4 @@ module Remotable alias_method("reset_#{attachment_name}!", "download_#{attachment_name}!") end end - - private - - def detect_extname_from_content_type(content_type) - return if content_type.nil? - - type = MIME::Types[content_type].first - - return if type.nil? - - extname = type.extensions.first - - return if extname.nil? - - ".#{extname}" - end - - def parse_content_type(content_type) - return if content_type.nil? - - content_type.split(/\s*;\s*/).first - end end diff --git a/config/application.rb b/config/application.rb index 8348649df4c..a1da9d61f60 100644 --- a/config/application.rb +++ b/config/application.rb @@ -9,10 +9,12 @@ Bundler.require(*Rails.groups) require_relative '../app/lib/exceptions' require_relative '../lib/paperclip/url_generator_extensions' require_relative '../lib/paperclip/attachment_extensions' +require_relative '../lib/paperclip/media_type_spoof_detector_extensions' require_relative '../lib/paperclip/lazy_thumbnail' require_relative '../lib/paperclip/gif_transcoder' require_relative '../lib/paperclip/video_transcoder' require_relative '../lib/paperclip/type_corrector' +require_relative '../lib/paperclip/response_with_limit_adapter' require_relative '../lib/mastodon/snowflake' require_relative '../lib/mastodon/version' require_relative '../lib/devise/two_factor_ldap_authenticatable' diff --git a/config/initializers/paperclip.rb b/config/initializers/paperclip.rb index ebb009065bd..b4849370dbd 100644 --- a/config/initializers/paperclip.rb +++ b/config/initializers/paperclip.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true Paperclip::DataUriAdapter.register +Paperclip::ResponseWithLimitAdapter.register Paperclip.interpolates :filename do |attachment, style| if style == :original diff --git a/lib/paperclip/image_extractor.rb b/lib/paperclip/image_extractor.rb index 114852e8b97..f5a54d1a5cd 100644 --- a/lib/paperclip/image_extractor.rb +++ b/lib/paperclip/image_extractor.rb @@ -4,28 +4,10 @@ require 'mime/types/columnar' module Paperclip class ImageExtractor < Paperclip::Processor - IMAGE_EXTRACTION_OPTIONS = { - convert_options: { - output: { - 'loglevel' => 'fatal', - vf: 'scale=\'min(400\, iw):min(400\, ih)\':force_original_aspect_ratio=decrease', - }.freeze, - }.freeze, - format: 'png', - time: -1, - file_geometry_parser: FastGeometryParser, - }.freeze - def make return @file unless options[:style] == :original - image = begin - begin - Paperclip::Transcoder.make(file, IMAGE_EXTRACTION_OPTIONS.dup, attachment) - rescue Paperclip::Error, ::Av::CommandError - nil - end - end + image = extract_image_from_file! unless image.nil? begin @@ -36,7 +18,7 @@ module Paperclip # to make sure it's cleaned up begin - FileUtils.rm(image) + image.close(true) rescue Errno::ENOENT nil end @@ -45,5 +27,28 @@ module Paperclip @file end + + private + + def extract_image_from_file! + ::Av.logger = Paperclip.logger + + cli = ::Av.cli + dst = Tempfile.new([File.basename(@file.path, '.*'), '.png']) + dst.binmode + + cli.add_source(@file.path) + cli.add_destination(dst.path) + cli.add_output_param loglevel: 'fatal' + + begin + cli.run + rescue Cocaine::ExitStatusError + dst.close(true) + return nil + end + + dst + end end end 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 00000000000..9c05573564f --- /dev/null +++ b/lib/paperclip/media_type_spoof_detector_extensions.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Paperclip + module MediaTypeSpoofDetectorExtensions + def calculated_content_type + @calculated_content_type ||= type_from_mime_magic || type_from_file_command + end + + def type_from_mime_magic + @type_from_mime_magic ||= begin + begin + File.open(@file.path) do |file| + MimeMagic.by_magic(file)&.type + end + rescue Errno::ENOENT + '' + end + end + end + + def type_from_file_command + @type_from_file_command ||= FileCommandContentTypeDetector.new(@file.path).detect + end + end +end + +Paperclip::MediaTypeSpoofDetector.prepend(Paperclip::MediaTypeSpoofDetectorExtensions) diff --git a/lib/paperclip/response_with_limit_adapter.rb b/lib/paperclip/response_with_limit_adapter.rb new file mode 100644 index 00000000000..7d897b8d672 --- /dev/null +++ b/lib/paperclip/response_with_limit_adapter.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +module Paperclip + class ResponseWithLimitAdapter < AbstractAdapter + def self.register + Paperclip.io_adapters.register self do |target| + target.is_a?(ResponseWithLimit) + end + end + + def initialize(target, options = {}) + super + cache_current_values + end + + private + + def cache_current_values + @original_filename = filename_from_content_disposition || filename_from_path || 'data' + @size = @target.response.content_length + @tempfile = copy_to_tempfile(@target) + @content_type = @target.response.mime_type || ContentTypeDetector.new(@tempfile.path).detect + end + + def copy_to_tempfile(source) + bytes_read = 0 + + source.response.body.each do |chunk| + bytes_read += chunk.bytesize + + destination.write(chunk) + chunk.clear + + raise Mastodon::LengthValidationError if bytes_read > source.limit + end + + destination.rewind + destination + rescue Mastodon::LengthValidationError + destination.close(true) + raise + ensure + source.response.connection.close + end + + def filename_from_content_disposition + disposition = @target.response.headers['content-disposition'] + disposition&.match(/filename="([^"]*)"/)&.captures&.first + end + + def filename_from_path + @target.response.uri.path.split('/').last + end + end +end diff --git a/spec/models/concerns/remotable_spec.rb b/spec/models/concerns/remotable_spec.rb index 2e6c8a9c6d2..9cc849dedb0 100644 --- a/spec/models/concerns/remotable_spec.rb +++ b/spec/models/concerns/remotable_spec.rb @@ -162,19 +162,15 @@ RSpec.describe Remotable do let(:headers) { { 'content-disposition' => file } } it 'assigns file' do - string_io = StringIO.new('') - extname = '.txt' - basename = '0123456789abcdef' + response_with_limit = ResponseWithLimit.new(nil, 0) - allow(SecureRandom).to receive(:hex).and_return(basename) - allow(StringIO).to receive(:new).with(anything).and_return(string_io) + allow(ResponseWithLimit).to receive(:new).with(anything, anything).and_return(response_with_limit) expect(foo).to receive(:public_send).with("download_#{hoge}!", url) foo.hoge_remote_url = url - expect(foo).to receive(:public_send).with("#{hoge}=", string_io) - expect(foo).to receive(:public_send).with("#{hoge}_file_name=", basename + extname) + expect(foo).to receive(:public_send).with("#{hoge}=", response_with_limit) foo.download_hoge!(url) end From 411bf188bbb9acb48b7bb1a14066f4975e036940 Mon Sep 17 00:00:00 2001 From: mayaeh Date: Wed, 1 Jul 2020 18:34:19 +0900 Subject: [PATCH 2/6] follow-up #14149 (#14192) ran `yarn manage:translations en` --- .../mastodon/locales/defaultMessages.json | 70 +++++++++++++++++-- app/javascript/mastodon/locales/en.json | 6 ++ config/locales/simple_form.en.yml | 2 +- 3 files changed, 73 insertions(+), 5 deletions(-) diff --git a/app/javascript/mastodon/locales/defaultMessages.json b/app/javascript/mastodon/locales/defaultMessages.json index 96c08f4d87f..fc624dcc035 100644 --- a/app/javascript/mastodon/locales/defaultMessages.json +++ b/app/javascript/mastodon/locales/defaultMessages.json @@ -492,6 +492,22 @@ }, { "descriptors": [ + { + "defaultMessage": "Public", + "id": "privacy.public.short" + }, + { + "defaultMessage": "Unlisted", + "id": "privacy.unlisted.short" + }, + { + "defaultMessage": "Followers-only", + "id": "privacy.private.short" + }, + { + "defaultMessage": "Direct", + "id": "privacy.direct.short" + }, { "defaultMessage": "Filtered", "id": "status.filtered" @@ -647,6 +663,31 @@ ], "path": "app/javascript/mastodon/features/account_timeline/index.json" }, + { + "descriptors": [ + { + "defaultMessage": "No comment provided", + "id": "account_note.placeholder" + }, + { + "defaultMessage": "Cancel", + "id": "account_note.cancel" + }, + { + "defaultMessage": "Save", + "id": "account_note.save" + }, + { + "defaultMessage": "Your note for @{name}", + "id": "account.account_note_header" + }, + { + "defaultMessage": "Edit", + "id": "account_note.edit" + } + ], + "path": "app/javascript/mastodon/features/account/components/account_note.json" + }, { "descriptors": [ { @@ -777,6 +818,10 @@ "defaultMessage": "Open moderation interface for @{name}", "id": "status.admin_account" }, + { + "defaultMessage": "Add note for @{name}", + "id": "account.add_account_note" + }, { "defaultMessage": "Follows you", "id": "account.follows_you" @@ -2465,6 +2510,27 @@ ], "path": "app/javascript/mastodon/features/status/components/card.json" }, + { + "descriptors": [ + { + "defaultMessage": "Public", + "id": "privacy.public.short" + }, + { + "defaultMessage": "Unlisted", + "id": "privacy.unlisted.short" + }, + { + "defaultMessage": "Followers-only", + "id": "privacy.private.short" + }, + { + "defaultMessage": "Direct", + "id": "privacy.direct.short" + } + ], + "path": "app/javascript/mastodon/features/status/components/detailed_status.json" + }, { "descriptors": [ { @@ -2992,10 +3058,6 @@ "defaultMessage": "Exit full screen", "id": "video.exit_fullscreen" }, - { - "defaultMessage": "Download file", - "id": "video.download" - }, { "defaultMessage": "Sensitive content", "id": "status.sensitive_warning" diff --git a/app/javascript/mastodon/locales/en.json b/app/javascript/mastodon/locales/en.json index e051c59bb0a..1212f2b366a 100644 --- a/app/javascript/mastodon/locales/en.json +++ b/app/javascript/mastodon/locales/en.json @@ -1,4 +1,6 @@ { + "account.account_note_header": "Your note for @{name}", + "account.add_account_note": "Add note for @{name}", "account.add_or_remove_from_list": "Add or Remove from lists", "account.badges.bot": "Bot", "account.badges.group": "Group", @@ -40,6 +42,10 @@ "account.unfollow": "Unfollow", "account.unmute": "Unmute @{name}", "account.unmute_notifications": "Unmute notifications from @{name}", + "account_note.cancel": "Cancel", + "account_note.edit": "Edit", + "account_note.placeholder": "No comment provided", + "account_note.save": "Save", "alert.rate_limited.message": "Please retry after {retry_time, time, medium}.", "alert.rate_limited.title": "Rate limited", "alert.unexpected.message": "An unexpected error occurred.", diff --git a/config/locales/simple_form.en.yml b/config/locales/simple_form.en.yml index f84b6c884c4..0a8a6fd6288 100644 --- a/config/locales/simple_form.en.yml +++ b/config/locales/simple_form.en.yml @@ -56,7 +56,7 @@ en: domain: This domain will be able to fetch data from this server and incoming data from it will be processed and stored email_domain_block: domain: This can be the domain name that shows up in the e-mail address, the MX record that domain resolves to, or IP of the server that MX record resolves to. Those will be checked upon user sign-up and the sign-up will be rejected. - with_dns_records: An attempt to resolve the given domain's DNS records will be made and the results will also be blacklisted + with_dns_records: An attempt to resolve the given domain's DNS records will be made and the results will also be blocked featured_tag: name: 'You might want to use one of these:' form_challenge: From 35cedc922ced6502f18b11353df661cf61c8ae2f Mon Sep 17 00:00:00 2001 From: ThibG Date: Wed, 1 Jul 2020 13:51:15 +0200 Subject: [PATCH 3/6] Change move handler to carry blocks over (#14144) * Change move handler to carry blocks and mutes over When user A blocks user B and B moves to a new account C, make A block C accordingly. Note that it only works if A's instance is aware of the Move, that is, if B is on A's instance or has followers there. * Also notify instances with known people blocking you when moving * Add automatic account notes when blocking/muting an account that had no note --- .../activitypub/move_distribution_worker.rb | 2 +- app/workers/move_worker.rb | 27 ++++++++++++++++ config/i18n-tasks.yml | 1 + config/locales/en.yml | 2 ++ .../account_migration_fabricator.rb | 4 +-- .../move_distribution_worker_spec.rb | 22 +++++++++++++ spec/workers/move_worker_spec.rb | 31 ++++++++++++++++--- 7 files changed, 82 insertions(+), 7 deletions(-) create mode 100644 spec/workers/activitypub/move_distribution_worker_spec.rb diff --git a/app/workers/activitypub/move_distribution_worker.rb b/app/workers/activitypub/move_distribution_worker.rb index bf1c0e7ae46..65c5c0d1c39 100644 --- a/app/workers/activitypub/move_distribution_worker.rb +++ b/app/workers/activitypub/move_distribution_worker.rb @@ -24,7 +24,7 @@ class ActivityPub::MoveDistributionWorker private def inboxes - @inboxes ||= @migration.account.followers.inboxes + @inboxes ||= (@migration.account.followers.inboxes + @migration.account.blocked_by.inboxes).uniq end def signed_payload diff --git a/app/workers/move_worker.rb b/app/workers/move_worker.rb index 4d76461b072..39e32131666 100644 --- a/app/workers/move_worker.rb +++ b/app/workers/move_worker.rb @@ -14,6 +14,8 @@ class MoveWorker end copy_account_notes! + carry_blocks_over! + carry_mutes_over! rescue ActiveRecord::RecordNotFound true end @@ -51,4 +53,29 @@ class MoveWorker end end end + + def carry_blocks_over! + @source_account.blocked_by_relationships.where(account: Account.local).find_each do |block| + unless block.account.blocking?(@target_account) || block.account.following?(@target_account) + BlockService.new.call(block.account, @target_account) + add_account_note_if_needed!(block.account, 'move_handler.carry_blocks_over_text') + end + end + end + + def carry_mutes_over! + @source_account.muted_by_relationships.where(account: Account.local).find_each do |mute| + MuteService.new.call(mute.account, @target_account, notifications: mute.hide_notifications) unless mute.account.muting?(@target_account) || mute.account.following?(@target_account) + add_account_note_if_needed!(mute.account, 'move_handler.carry_mutes_over_text') + end + end + + def add_account_note_if_needed!(account, id) + unless AccountNote.where(account: account, target_account: @target_account).exists? + text = I18n.with_locale(account.user.locale || I18n.default_locale) do + I18n.t(id, acct: @source_account.acct) + end + AccountNote.create!(account: account, target_account: @target_account, comment: text) + end + end end diff --git a/config/i18n-tasks.yml b/config/i18n-tasks.yml index c2d6a783828..2dc6f880bfc 100644 --- a/config/i18n-tasks.yml +++ b/config/i18n-tasks.yml @@ -60,6 +60,7 @@ ignore_unused: - 'admin.accounts.roles.*' - 'admin.action_logs.actions.*' - 'statuses.attached.*' + - 'move_handler.carry_{mutes,blocks}_over_text' ignore_inconsistent_interpolations: - '*.one' diff --git a/config/locales/en.yml b/config/locales/en.yml index b3906a127a2..2cae0a3e3ba 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -941,6 +941,8 @@ en: moderation: title: Moderation move_handler: + carry_blocks_over_text: This user moved from %{acct}, which you had blocked. + carry_mutes_over_text: This user moved from %{acct}, which you had muted. copy_account_note_text: 'This user moved from %{acct}, here were your previous notes about them:' notification_mailer: digest: diff --git a/spec/fabricators/account_migration_fabricator.rb b/spec/fabricators/account_migration_fabricator.rb index 3b3fc20773a..2a8e747a81a 100644 --- a/spec/fabricators/account_migration_fabricator.rb +++ b/spec/fabricators/account_migration_fabricator.rb @@ -1,6 +1,6 @@ Fabricator(:account_migration) do account - target_account + target_account { |attrs| Fabricate(:account, also_known_as: [ActivityPub::TagManager.instance.uri_for(attrs[:account])]) } + acct { |attrs| attrs[:target_account].acct } followers_count 1234 - acct 'test@example.com' end diff --git a/spec/workers/activitypub/move_distribution_worker_spec.rb b/spec/workers/activitypub/move_distribution_worker_spec.rb new file mode 100644 index 00000000000..b52788e54e7 --- /dev/null +++ b/spec/workers/activitypub/move_distribution_worker_spec.rb @@ -0,0 +1,22 @@ +require 'rails_helper' + +describe ActivityPub::MoveDistributionWorker do + subject { described_class.new } + + let(:migration) { Fabricate(:account_migration) } + let(:follower) { Fabricate(:account, protocol: :activitypub, inbox_url: 'http://example.com') } + let(:blocker) { Fabricate(:account, protocol: :activitypub, inbox_url: 'http://example2.com') } + + describe '#perform' do + before do + allow(ActivityPub::DeliveryWorker).to receive(:push_bulk) + follower.follow!(migration.account) + blocker.block!(migration.account) + end + + it 'delivers to followers and known blockers' do + subject.perform(migration.id) + expect(ActivityPub::DeliveryWorker).to have_received(:push_bulk).with(['http://example.com', 'http://example2.com']) + end + end +end diff --git a/spec/workers/move_worker_spec.rb b/spec/workers/move_worker_spec.rb index 77b921eaff2..8ab4f182fd0 100644 --- a/spec/workers/move_worker_spec.rb +++ b/spec/workers/move_worker_spec.rb @@ -4,20 +4,29 @@ require 'rails_helper' describe MoveWorker do let(:local_follower) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account } + let(:blocking_account) { Fabricate(:user, email: 'bar@example.com', account: Fabricate(:account, username: 'bar')).account } + let(:muting_account) { Fabricate(:user, email: 'foo@example.com', account: Fabricate(:account, username: 'foo')).account } let(:source_account) { Fabricate(:account, protocol: :activitypub, domain: 'example.com') } let(:target_account) { Fabricate(:account, protocol: :activitypub, domain: 'example.com') } let(:local_user) { Fabricate(:user) } let!(:account_note) { Fabricate(:account_note, account: local_user.account, target_account: source_account) } + let(:block_service) { double } + subject { described_class.new } before do local_follower.follow!(source_account) + blocking_account.block!(source_account) + muting_account.mute!(source_account) + + allow(UnfollowFollowWorker).to receive(:push_bulk) + allow(BlockService).to receive(:new).and_return(block_service) + allow(block_service).to receive(:call) end shared_examples 'user note handling' do it 'copies user note' do - allow(UnfollowFollowWorker).to receive(:push_bulk) subject.perform(source_account.id, target_account.id) expect(AccountNote.find_by(account: account_note.account, target_account: target_account).comment).to include(source_account.acct) expect(AccountNote.find_by(account: account_note.account, target_account: target_account).comment).to include(account_note.comment) @@ -26,7 +35,6 @@ describe MoveWorker do it 'merges user notes when needed' do new_account_note = AccountNote.create!(account: account_note.account, target_account: target_account, comment: 'new note prior to move') - allow(UnfollowFollowWorker).to receive(:push_bulk) subject.perform(source_account.id, target_account.id) expect(AccountNote.find_by(account: account_note.account, target_account: target_account).comment).to include(source_account.acct) expect(AccountNote.find_by(account: account_note.account, target_account: target_account).comment).to include(account_note.comment) @@ -34,15 +42,29 @@ describe MoveWorker do end end + shared_examples 'block and mute handling' do + it 'makes blocks carry over and add a note' do + subject.perform(source_account.id, target_account.id) + expect(block_service).to have_received(:call).with(blocking_account, target_account) + expect(AccountNote.find_by(account: blocking_account, target_account: target_account).comment).to include(source_account.acct) + end + + it 'makes mutes carry over and add a note' do + subject.perform(source_account.id, target_account.id) + expect(muting_account.muting?(target_account)).to be true + expect(AccountNote.find_by(account: muting_account, target_account: target_account).comment).to include(source_account.acct) + end + end + context 'both accounts are distant' do describe 'perform' do it 'calls UnfollowFollowWorker' do - allow(UnfollowFollowWorker).to receive(:push_bulk) subject.perform(source_account.id, target_account.id) expect(UnfollowFollowWorker).to have_received(:push_bulk).with([local_follower.id]) end include_examples 'user note handling' + include_examples 'block and mute handling' end end @@ -51,12 +73,12 @@ describe MoveWorker do describe 'perform' do it 'calls UnfollowFollowWorker' do - allow(UnfollowFollowWorker).to receive(:push_bulk) subject.perform(source_account.id, target_account.id) expect(UnfollowFollowWorker).to have_received(:push_bulk).with([local_follower.id]) end include_examples 'user note handling' + include_examples 'block and mute handling' end end @@ -71,6 +93,7 @@ describe MoveWorker do end include_examples 'user note handling' + include_examples 'block and mute handling' it 'does not fail when a local user is already following both accounts' do double_follower = Fabricate(:user, email: 'eve@example.com', account: Fabricate(:account, username: 'eve')).account From 4babf5b8b5ba8eca5a14f3b2813775240db8f8f1 Mon Sep 17 00:00:00 2001 From: ThibG Date: Wed, 1 Jul 2020 13:51:50 +0200 Subject: [PATCH 4/6] Fix lock icon not being shown when locking account in profile settings (#14190) --- app/javascript/packs/public.js | 10 ++++++---- app/javascript/styles/mastodon/accounts.scss | 4 ++++ app/views/application/_card.html.haml | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/app/javascript/packs/public.js b/app/javascript/packs/public.js index 557823c96d8..08cc662e60b 100644 --- a/app/javascript/packs/public.js +++ b/app/javascript/packs/public.js @@ -207,10 +207,12 @@ function main() { delegate(document, '#account_locked', 'change', ({ target }) => { const lock = document.querySelector('.card .display-name i'); - if (target.checked) { - lock.style.display = 'inline'; - } else { - lock.style.display = 'none'; + if (lock) { + if (target.checked) { + delete lock.dataset.hidden; + } else { + lock.dataset.hidden = 'true'; + } } }); diff --git a/app/javascript/styles/mastodon/accounts.scss b/app/javascript/styles/mastodon/accounts.scss index 5dc067f0e8f..2c78e81be2f 100644 --- a/app/javascript/styles/mastodon/accounts.scss +++ b/app/javascript/styles/mastodon/accounts.scss @@ -76,6 +76,10 @@ margin-left: 15px; text-align: left; + i[data-hidden] { + display: none; + } + strong { font-size: 15px; color: $primary-text-color; diff --git a/app/views/application/_card.html.haml b/app/views/application/_card.html.haml index e7ecfecd9a1..909d9ff818c 100644 --- a/app/views/application/_card.html.haml +++ b/app/views/application/_card.html.haml @@ -13,4 +13,4 @@ %strong.emojify.p-name= display_name(account, custom_emojify: true) %span = acct(account) - = fa_icon('lock') if account.locked? + = fa_icon('lock', { :data => ({hidden: true} unless account.locked?)}) From e9ea960773ee6a1068623374b21d768918fbc93b Mon Sep 17 00:00:00 2001 From: Ariel Date: Wed, 1 Jul 2020 08:52:05 -0300 Subject: [PATCH 5/6] Fix cursor type in statuses (#14185) --- app/javascript/styles/mastodon/components.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/javascript/styles/mastodon/components.scss b/app/javascript/styles/mastodon/components.scss index b3f6d5749f3..58bc0ff8b07 100644 --- a/app/javascript/styles/mastodon/components.scss +++ b/app/javascript/styles/mastodon/components.scss @@ -992,7 +992,7 @@ position: relative; min-height: 54px; border-bottom: 1px solid lighten($ui-base-color, 8%); - cursor: default; + cursor: auto; @supports (-ms-overflow-style: -ms-autohiding-scrollbar) { // Add margin to avoid Edge auto-hiding scrollbar appearing over content. From 6d23d40420e4548778f3ca4ed9e8cb16e0eb0073 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 1 Jul 2020 19:05:21 +0200 Subject: [PATCH 6/6] Change Redis#exists calls to Redis#exists? to avoid deprecation warning (#14191) --- .gitignore | 28 +++++++++---------- Gemfile.lock | 7 ++--- app/lib/activitypub/activity.rb | 2 +- app/lib/activitypub/activity/move.rb | 2 +- app/lib/feed_manager.rb | 2 +- app/models/account_conversation.rb | 2 +- app/models/encrypted_message.rb | 5 +--- app/models/home_feed.rb | 2 +- .../publish_announcement_reaction_worker.rb | 2 +- .../publish_scheduled_announcement_worker.rb | 2 +- app/workers/unpublish_announcement_worker.rb | 2 +- config/application.rb | 1 + config/initializers/redis.rb | 2 -- lib/redis/namespace_extensions.rb | 12 ++++++++ 14 files changed, 38 insertions(+), 33 deletions(-) create mode 100644 lib/redis/namespace_extensions.rb diff --git a/.gitignore b/.gitignore index 9f6c4b4137c..4545270b30b 100644 --- a/.gitignore +++ b/.gitignore @@ -17,36 +17,36 @@ /log/* !/log/.keep /tmp -coverage -public/system -public/assets -public/packs -public/packs-test +/coverage +/public/system +/public/assets +/public/packs +/public/packs-test .env .env.production .env.development -node_modules/ -build/ +/node_modules/ +/build/ # Ignore Vagrant files .vagrant/ # Ignore Capistrano customizations -config/deploy/* +/config/deploy/* # Ignore IDE files .vscode/ .idea/ # Ignore postgres + redis + elasticsearch volume optionally created by docker-compose -postgres -redis -elasticsearch +/postgres +/redis +/elasticsearch # ignore Helm lockfile, dependency charts, and local values file -chart/Chart.lock -chart/charts/*.tgz -chart/values.yaml +/chart/Chart.lock +/chart/charts/*.tgz +/chart/values.yaml # Ignore Apple files .DS_Store diff --git a/Gemfile.lock b/Gemfile.lock index 96572bde567..fcea8100257 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -444,8 +444,6 @@ GEM rack (>= 1.0, < 3) rack-cors (1.1.1) rack (>= 2.0.0) - rack-protection (2.0.8.1) - rack rack-proxy (0.6.5) rack rack-test (1.1.0) @@ -570,11 +568,10 @@ GEM nokogiri (>= 1.8.0) nokogumbo (~> 2.0) semantic_range (2.3.0) - sidekiq (6.0.7) + sidekiq (6.1.0) connection_pool (>= 2.2.2) rack (~> 2.0) - rack-protection (>= 2.0.0) - redis (>= 4.1.0) + redis (>= 4.2.0) sidekiq-bulk (0.2.0) sidekiq sidekiq-scheduler (3.0.1) diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb index ee35e1e8da2..58cec7ac49f 100644 --- a/app/lib/activitypub/activity.rb +++ b/app/lib/activitypub/activity.rb @@ -132,7 +132,7 @@ class ActivityPub::Activity end def delete_arrived_first?(uri) - redis.exists("delete_upon_arrival:#{@account.id}:#{uri}") + redis.exists?("delete_upon_arrival:#{@account.id}:#{uri}") end def delete_later!(uri) diff --git a/app/lib/activitypub/activity/move.rb b/app/lib/activitypub/activity/move.rb index 12bb82d259a..2103f503f20 100644 --- a/app/lib/activitypub/activity/move.rb +++ b/app/lib/activitypub/activity/move.rb @@ -33,7 +33,7 @@ class ActivityPub::Activity::Move < ActivityPub::Activity end def processed? - redis.exists("move_in_progress:#{@account.id}") + redis.exists?("move_in_progress:#{@account.id}") end def mark_as_processing! diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index efb4f6e2c21..53ff31f5ecb 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -169,7 +169,7 @@ class FeedManager private def push_update_required?(timeline_id) - redis.exists("subscribed:#{timeline_id}") + redis.exists?("subscribed:#{timeline_id}") end def blocks_or_mutes?(receiver_id, account_ids, context) diff --git a/app/models/account_conversation.rb b/app/models/account_conversation.rb index 0c03747e2ae..b4381658852 100644 --- a/app/models/account_conversation.rb +++ b/app/models/account_conversation.rb @@ -108,7 +108,7 @@ class AccountConversation < ApplicationRecord end def subscribed_to_timeline? - Redis.current.exists("subscribed:#{streaming_channel}") + Redis.current.exists?("subscribed:#{streaming_channel}") end def streaming_channel diff --git a/app/models/encrypted_message.rb b/app/models/encrypted_message.rb index 5e0aba4342a..aa4182b4e12 100644 --- a/app/models/encrypted_message.rb +++ b/app/models/encrypted_message.rb @@ -32,16 +32,13 @@ class EncryptedMessage < ApplicationRecord private def push_to_streaming_api - Rails.logger.info(streaming_channel) - Rails.logger.info(subscribed_to_timeline?) - return if destroyed? || !subscribed_to_timeline? PushEncryptedMessageWorker.perform_async(id) end def subscribed_to_timeline? - Redis.current.exists("subscribed:#{streaming_channel}") + Redis.current.exists?("subscribed:#{streaming_channel}") end def streaming_channel diff --git a/app/models/home_feed.rb b/app/models/home_feed.rb index 1fd506138be..0fe9dae4641 100644 --- a/app/models/home_feed.rb +++ b/app/models/home_feed.rb @@ -8,6 +8,6 @@ class HomeFeed < Feed end def regenerating? - redis.exists("account:#{@id}:regeneration") + redis.exists?("account:#{@id}:regeneration") end end diff --git a/app/workers/publish_announcement_reaction_worker.rb b/app/workers/publish_announcement_reaction_worker.rb index 418dc71275c..03da56550aa 100644 --- a/app/workers/publish_announcement_reaction_worker.rb +++ b/app/workers/publish_announcement_reaction_worker.rb @@ -14,7 +14,7 @@ class PublishAnnouncementReactionWorker payload = Oj.dump(event: :'announcement.reaction', payload: payload) FeedManager.instance.with_active_accounts do |account| - redis.publish("timeline:#{account.id}", payload) if redis.exists("subscribed:timeline:#{account.id}") + redis.publish("timeline:#{account.id}", payload) if redis.exists?("subscribed:timeline:#{account.id}") end rescue ActiveRecord::RecordNotFound true diff --git a/app/workers/publish_scheduled_announcement_worker.rb b/app/workers/publish_scheduled_announcement_worker.rb index 1392efed06f..c23eae6af7c 100644 --- a/app/workers/publish_scheduled_announcement_worker.rb +++ b/app/workers/publish_scheduled_announcement_worker.rb @@ -15,7 +15,7 @@ class PublishScheduledAnnouncementWorker payload = Oj.dump(event: :announcement, payload: payload) FeedManager.instance.with_active_accounts do |account| - redis.publish("timeline:#{account.id}", payload) if redis.exists("subscribed:timeline:#{account.id}") + redis.publish("timeline:#{account.id}", payload) if redis.exists?("subscribed:timeline:#{account.id}") end end diff --git a/app/workers/unpublish_announcement_worker.rb b/app/workers/unpublish_announcement_worker.rb index e99d70cf87a..e58c07554a7 100644 --- a/app/workers/unpublish_announcement_worker.rb +++ b/app/workers/unpublish_announcement_worker.rb @@ -8,7 +8,7 @@ class UnpublishAnnouncementWorker payload = Oj.dump(event: :'announcement.delete', payload: announcement_id.to_s) FeedManager.instance.with_active_accounts do |account| - redis.publish("timeline:#{account.id}", payload) if redis.exists("subscribed:timeline:#{account.id}") + redis.publish("timeline:#{account.id}", payload) if redis.exists?("subscribed:timeline:#{account.id}") end end end diff --git a/config/application.rb b/config/application.rb index a1da9d61f60..a3c37b042af 100644 --- a/config/application.rb +++ b/config/application.rb @@ -7,6 +7,7 @@ require 'rails/all' Bundler.require(*Rails.groups) require_relative '../app/lib/exceptions' +require_relative '../lib/redis/namespace_extensions' require_relative '../lib/paperclip/url_generator_extensions' require_relative '../lib/paperclip/attachment_extensions' require_relative '../lib/paperclip/media_type_spoof_detector_extensions' diff --git a/config/initializers/redis.rb b/config/initializers/redis.rb index 510194044eb..7573fc9f775 100644 --- a/config/initializers/redis.rb +++ b/config/initializers/redis.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -Redis.exists_returns_integer = false - redis_connection = Redis.new( url: ENV['REDIS_URL'], driver: :hiredis diff --git a/lib/redis/namespace_extensions.rb b/lib/redis/namespace_extensions.rb new file mode 100644 index 00000000000..310a4f465bf --- /dev/null +++ b/lib/redis/namespace_extensions.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class Redis + module NamespaceExtensions + def exists?(*args, &block) + call_with_namespace('exists?', *args, &block) + end + end +end + +Redis::Namespace::COMMANDS['exists?'] = [:first] +Redis::Namespace.prepend(Redis::NamespaceExtensions)