From b58db8f12eb19787ee3bd1ec8abab21027b3d4ef Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 25 Mar 2022 19:31:35 +0100 Subject: Add workaround for YouTube Shorts links (#17869) * Add workaround for YouTube Shorts links * Update link_details_extractor_spec.rb --- spec/lib/link_details_extractor_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'spec') diff --git a/spec/lib/link_details_extractor_spec.rb b/spec/lib/link_details_extractor_spec.rb index 84bb4579c..7ea867c61 100644 --- a/spec/lib/link_details_extractor_spec.rb +++ b/spec/lib/link_details_extractor_spec.rb @@ -25,6 +25,14 @@ RSpec.describe LinkDetailsExtractor do expect(subject.canonical_url).to eq 'https://foo.com/article' end end + + context 'when canonical URL is set to "null"' do + let(:html) { '' } + + it 'ignores the canonical URLs' do + expect(subject.canonical_url).to eq original_url + end + end end context 'when structured data is present' do -- cgit From 71f2b95106b2e75d3efb40040b29c216c2d99ee6 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 26 Mar 2022 00:38:44 +0100 Subject: Fix edits with no actual changes being allowed (#17843) * Fix edits with no actual changes being allowed locally * Fix edits with no actual changes being allowed through ActivityPub * Fix false positive changes caused by description processing in model * Fix not recording poll expiration update * Fix test * Revert changes to ProcessStatusUpdateService * Various fixes and improvements * Fix code style issues * Various changes and improvements * Add guard clause --- .../activitypub/parser/media_attachment_parser.rb | 4 ++- app/models/concerns/status_snapshot_concern.rb | 35 ++++++++++++++++++++++ app/models/media_attachment.rb | 7 +---- app/models/status.rb | 21 +------------ .../activitypub/process_status_update_service.rb | 22 +++++++------- app/services/update_status_service.rb | 28 ++++++++++++++--- spec/models/media_attachment_spec.rb | 8 ----- .../process_status_update_service_spec.rb | 31 +++++++++++++++++-- spec/services/update_status_service_spec.rb | 17 +++++++++++ 9 files changed, 121 insertions(+), 52 deletions(-) create mode 100644 app/models/concerns/status_snapshot_concern.rb (limited to 'spec') diff --git a/app/lib/activitypub/parser/media_attachment_parser.rb b/app/lib/activitypub/parser/media_attachment_parser.rb index 1798e58a4..30bea1f0e 100644 --- a/app/lib/activitypub/parser/media_attachment_parser.rb +++ b/app/lib/activitypub/parser/media_attachment_parser.rb @@ -27,7 +27,9 @@ class ActivityPub::Parser::MediaAttachmentParser end def description - @json['summary'].presence || @json['name'].presence + str = @json['summary'].presence || @json['name'].presence + str = str.strip[0...MediaAttachment::MAX_DESCRIPTION_LENGTH] if str.present? + str end def focus diff --git a/app/models/concerns/status_snapshot_concern.rb b/app/models/concerns/status_snapshot_concern.rb new file mode 100644 index 000000000..9741b9aeb --- /dev/null +++ b/app/models/concerns/status_snapshot_concern.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module StatusSnapshotConcern + extend ActiveSupport::Concern + + included do + has_many :edits, class_name: 'StatusEdit', inverse_of: :status, dependent: :destroy + end + + def edited? + edited_at.present? + end + + def build_snapshot(account_id: nil, at_time: nil, rate_limit: true) + # We don't use `edits#new` here to avoid it having saved when the + # status is saved, since we want to control that manually + + StatusEdit.new( + status_id: id, + text: text, + spoiler_text: spoiler_text, + sensitive: sensitive, + ordered_media_attachment_ids: ordered_media_attachment_ids&.dup || media_attachments.pluck(:id), + media_descriptions: ordered_media_attachments.map(&:description), + poll_options: preloadable_poll&.options&.dup, + account_id: account_id || self.account_id, + created_at: at_time || edited_at, + rate_limit: rate_limit + ) + end + + def snapshot!(**options) + build_snapshot(**options).save! + end +end diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index a3115637e..21c663e47 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -185,7 +185,7 @@ class MediaAttachment < ApplicationRecord remotable_attachment :thumbnail, IMAGE_LIMIT, suppress_errors: true, download_on_assign: false validates :account, presence: true - validates :description, length: { maximum: MAX_DESCRIPTION_LENGTH }, if: :local? + validates :description, length: { maximum: MAX_DESCRIPTION_LENGTH } validates :file, presence: true, if: :local? validates :thumbnail, absence: true, if: -> { local? && !audio_or_video? } @@ -258,7 +258,6 @@ class MediaAttachment < ApplicationRecord after_commit :enqueue_processing, on: :create after_commit :reset_parent_cache, on: :update - before_create :prepare_description, unless: :local? before_create :set_unknown_type before_create :set_processing @@ -306,10 +305,6 @@ class MediaAttachment < ApplicationRecord self.type = :unknown if file.blank? && !type_changed? end - def prepare_description - self.description = description.strip[0...MAX_DESCRIPTION_LENGTH] unless description.nil? - end - def set_type_and_extension self.type = begin if VIDEO_MIME_TYPES.include?(file_content_type) diff --git a/app/models/status.rb b/app/models/status.rb index 5b984543e..bf5a49b7f 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -35,6 +35,7 @@ class Status < ApplicationRecord include Paginable include Cacheable include StatusThreadingConcern + include StatusSnapshotConcern include RateLimitable rate_limit by: :account, family: :statuses @@ -59,8 +60,6 @@ class Status < ApplicationRecord belongs_to :thread, foreign_key: 'in_reply_to_id', class_name: 'Status', inverse_of: :replies, optional: true belongs_to :reblog, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblogs, optional: true - has_many :edits, class_name: 'StatusEdit', inverse_of: :status, dependent: :destroy - has_many :favourites, inverse_of: :status, dependent: :destroy has_many :bookmarks, inverse_of: :status, dependent: :destroy has_many :reblogs, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblog, dependent: :destroy @@ -212,24 +211,6 @@ class Status < ApplicationRecord public_visibility? || unlisted_visibility? end - def snapshot!(account_id: nil, at_time: nil, rate_limit: true) - edits.create!( - text: text, - spoiler_text: spoiler_text, - sensitive: sensitive, - ordered_media_attachment_ids: ordered_media_attachment_ids || media_attachments.pluck(:id), - media_descriptions: ordered_media_attachments.map(&:description), - poll_options: preloadable_poll&.options, - account_id: account_id || self.account_id, - created_at: at_time || edited_at, - rate_limit: rate_limit - ) - end - - def edited? - edited_at.present? - end - alias sign? distributable? def with_media? diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index 47a788c30..6dc14d8c2 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -4,6 +4,8 @@ class ActivityPub::ProcessStatusUpdateService < BaseService include JsonLdHelper def call(status, json) + raise ArgumentError, 'Status has unsaved changes' if status.changed? + @json = json @status_parser = ActivityPub::Parser::StatusParser.new(@json) @uri = @status_parser.uri @@ -17,16 +19,19 @@ class ActivityPub::ProcessStatusUpdateService < BaseService last_edit_date = status.edited_at.presence || status.created_at + # Since we rely on tracking of previous changes, ensure clean slate + status.clear_changes_information + # Only allow processing one create/update per status at a time RedisLock.acquire(lock_options) do |lock| if lock.acquired? Status.transaction do - create_previous_edit! + record_previous_edit! update_media_attachments! update_poll! update_immediate_attributes! update_metadata! - create_edit! + create_edits! end queue_poll_notifications! @@ -216,19 +221,14 @@ class ActivityPub::ProcessStatusUpdateService < BaseService { redis: Redis.current, key: "create:#{@uri}", autorelease: 15.minutes.seconds } end - def create_previous_edit! - # We only need to create a previous edit when no previous edits exist, e.g. - # when the status has never been edited. For other cases, we always create - # an edit, so the step can be skipped - - return if @status.edits.any? - - @status.snapshot!(at_time: @status.created_at, rate_limit: false) + def record_previous_edit! + @previous_edit = @status.build_snapshot(at_time: @status.created_at, rate_limit: false) if @status.edits.empty? end - def create_edit! + def create_edits! return unless significant_changes? + @previous_edit&.save! @status.snapshot!(account_id: @account.id, rate_limit: false) end diff --git a/app/services/update_status_service.rb b/app/services/update_status_service.rb index 055e5968d..c4c934976 100644 --- a/app/services/update_status_service.rb +++ b/app/services/update_status_service.rb @@ -4,6 +4,8 @@ class UpdateStatusService < BaseService include Redisable include LanguagesHelper + class NoChangesSubmittedError < StandardError; end + # @param [Status] status # @param [Integer] account_id # @param [Hash] options @@ -17,6 +19,8 @@ class UpdateStatusService < BaseService @status = status @options = options @account_id = account_id + @media_attachments_changed = false + @poll_changed = false Status.transaction do create_previous_edit! @@ -32,18 +36,24 @@ class UpdateStatusService < BaseService broadcast_updates! @status + rescue NoChangesSubmittedError + # For calls that result in no changes, swallow the error + # but get back to the original state + + @status.reload end private def update_media_attachments! - previous_media_attachments = @status.media_attachments.to_a + previous_media_attachments = @status.ordered_media_attachments.to_a next_media_attachments = validate_media! added_media_attachments = next_media_attachments - previous_media_attachments MediaAttachment.where(id: added_media_attachments.map(&:id)).update_all(status_id: @status.id) @status.ordered_media_attachment_ids = (@options[:media_ids] || []).map(&:to_i) & next_media_attachments.map(&:id) + @media_attachments_changed = previous_media_attachments.map(&:id) != @status.ordered_media_attachment_ids @status.media_attachments.reload end @@ -69,20 +79,23 @@ class UpdateStatusService < BaseService # If for some reasons the options were changed, it invalidates all previous # votes, so we need to remove them - poll_changed = true if @options[:poll][:options] != poll.options || ActiveModel::Type::Boolean.new.cast(@options[:poll][:multiple]) != poll.multiple + @poll_changed = true if @options[:poll][:options] != poll.options || ActiveModel::Type::Boolean.new.cast(@options[:poll][:multiple]) != poll.multiple poll.options = @options[:poll][:options] poll.hide_totals = @options[:poll][:hide_totals] || false poll.multiple = @options[:poll][:multiple] || false poll.expires_in = @options[:poll][:expires_in] - poll.reset_votes! if poll_changed + poll.reset_votes! if @poll_changed poll.save! @status.poll_id = poll.id elsif previous_poll.present? previous_poll.destroy + @poll_changed = true @status.poll_id = nil end + + @poll_changed = true if @previous_expires_at != @status.preloadable_poll&.expires_at end def update_immediate_attributes! @@ -90,8 +103,11 @@ class UpdateStatusService < BaseService @status.spoiler_text = @options[:spoiler_text] || '' if @options.key?(:spoiler_text) @status.sensitive = @options[:sensitive] || @options[:spoiler_text].present? if @options.key?(:sensitive) || @options.key?(:spoiler_text) @status.language = valid_locale_cascade(@options[:language], @status.language, @status.account.user&.preferred_posting_language, I18n.default_locale) - @status.edited_at = Time.now.utc + # We raise here to rollback the entire transaction + raise NoChangesSubmittedError unless significant_changes? + + @status.edited_at = Time.now.utc @status.save! end @@ -137,4 +153,8 @@ class UpdateStatusService < BaseService def create_edit! @status.snapshot!(account_id: @account_id) end + + def significant_changes? + @status.changed? || @poll_changed || @media_attachments_changed + end end diff --git a/spec/models/media_attachment_spec.rb b/spec/models/media_attachment_spec.rb index 7360b23cf..cbd9a09c5 100644 --- a/spec/models/media_attachment_spec.rb +++ b/spec/models/media_attachment_spec.rb @@ -186,14 +186,6 @@ RSpec.describe MediaAttachment, type: :model do expect(media.valid?).to be false end - describe 'descriptions for remote attachments' do - it 'are cut off at 1500 characters' do - media = Fabricate(:media_attachment, description: 'foo' * 1000, remote_url: 'http://example.com/blah.jpg') - - expect(media.description.size).to be <= 1_500 - end - end - describe 'size limit validation' do it 'rejects video files that are too large' do stub_const 'MediaAttachment::IMAGE_LIMIT', 100.megabytes diff --git a/spec/services/activitypub/process_status_update_service_spec.rb b/spec/services/activitypub/process_status_update_service_spec.rb index 788c7c9d9..f87adcae1 100644 --- a/spec/services/activitypub/process_status_update_service_spec.rb +++ b/spec/services/activitypub/process_status_update_service_spec.rb @@ -46,6 +46,29 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService, type: :service do expect(status.reload.spoiler_text).to eq 'Show more' end + context 'with no changes at all' do + let(:payload) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + id: 'foo', + type: 'Note', + content: 'Hello world', + } + end + + before do + subject.call(status, json) + end + + it 'does not create any edits' do + expect(status.reload.edits).to be_empty + end + + it 'does not mark status as edited' do + expect(status.edited?).to be false + end + end + context 'with no changes and originally with no ordered_media_attachment_ids' do let(:payload) do { @@ -61,8 +84,12 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService, type: :service do subject.call(status, json) end - it 'does not record an update' do - expect(status.reload.edited?).to be false + it 'does not create any edits' do + expect(status.reload.edits).to be_empty + end + + it 'does not mark status as edited' do + expect(status.edited?).to be false end end diff --git a/spec/services/update_status_service_spec.rb b/spec/services/update_status_service_spec.rb index 78cc89cd4..71a73be5b 100644 --- a/spec/services/update_status_service_spec.rb +++ b/spec/services/update_status_service_spec.rb @@ -3,6 +3,23 @@ require 'rails_helper' RSpec.describe UpdateStatusService, type: :service do subject { described_class.new } + context 'when nothing changes' do + let!(:status) { Fabricate(:status, text: 'Foo', language: 'en') } + + before do + allow(ActivityPub::DistributionWorker).to receive(:perform_async) + subject.call(status, status.account_id, text: 'Foo') + end + + it 'does not create an edit' do + expect(status.reload.edits).to be_empty + end + + it 'does not notify anyone' do + expect(ActivityPub::DistributionWorker).to_not have_received(:perform_async) + end + end + context 'when text changes' do let!(:status) { Fabricate(:status, text: 'Foo') } let(:preview_card) { Fabricate(:preview_card) } -- cgit From cefa526c6d3a45df2d0fcb7643ced828e2e87dea Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 26 Mar 2022 02:53:34 +0100 Subject: Refactor formatter (#17828) * Refactor formatter * Move custom emoji pre-rendering logic to view helpers * Move more methods out of Formatter * Fix code style issues * Remove Formatter * Add inline poll options to RSS feeds * Remove unused helper method * Fix code style issues * Various fixes and improvements * Fix test --- app/chewy/statuses_index.rb | 2 +- app/controllers/api/web/embeds_controller.rb | 2 +- app/helpers/accounts_helper.rb | 6 +- app/helpers/admin/trends/statuses_helper.rb | 5 +- app/helpers/application_helper.rb | 4 + app/helpers/formatting_helper.rb | 19 + app/helpers/routing_helper.rb | 3 +- app/helpers/statuses_helper.rb | 14 - app/lib/activitypub/activity/create.rb | 4 +- app/lib/emoji_formatter.rb | 98 ++++ app/lib/extractor.rb | 82 +++- app/lib/feed_manager.rb | 3 +- app/lib/formatter.rb | 294 ----------- app/lib/html_aware_formatter.rb | 38 ++ app/lib/plain_text_formatter.rb | 30 ++ app/lib/rss/serializer.rb | 23 +- app/lib/text_formatter.rb | 158 ++++++ app/mailers/application_mailer.rb | 1 + app/serializers/activitypub/actor_serializer.rb | 7 +- app/serializers/activitypub/note_serializer.rb | 6 +- app/serializers/rest/account_serializer.rb | 7 +- app/serializers/rest/announcement_serializer.rb | 4 +- app/serializers/rest/status_edit_serializer.rb | 4 +- app/serializers/rest/status_serializer.rb | 4 +- app/services/fetch_link_card_service.rb | 2 +- app/views/accounts/_bio.html.haml | 6 +- app/views/admin/accounts/show.html.haml | 6 +- app/views/admin/reports/_status.html.haml | 6 +- app/views/admin/reports/show.html.haml | 2 +- app/views/directories/index.html.haml | 2 +- app/views/disputes/strikes/show.html.haml | 2 +- app/views/notification_mailer/_status.html.haml | 4 +- app/views/notification_mailer/_status.text.erb | 2 +- app/views/notification_mailer/digest.text.erb | 2 +- app/views/statuses/_detailed_status.html.haml | 5 +- app/views/statuses/_poll.html.haml | 4 +- app/views/statuses/_simple_status.html.haml | 5 +- app/views/user_mailer/warning.html.haml | 2 +- config/initializers/twitter_regex.rb | 26 - spec/lib/emoji_formatter_spec.rb | 55 +++ spec/lib/formatter_spec.rb | 626 ------------------------ spec/lib/html_aware_formatter.rb | 44 ++ spec/lib/plain_text_formatter_spec.rb | 24 + spec/lib/text_formatter_spec.rb | 313 ++++++++++++ 44 files changed, 932 insertions(+), 1024 deletions(-) create mode 100644 app/helpers/formatting_helper.rb create mode 100644 app/lib/emoji_formatter.rb delete mode 100644 app/lib/formatter.rb create mode 100644 app/lib/html_aware_formatter.rb create mode 100644 app/lib/plain_text_formatter.rb create mode 100644 app/lib/text_formatter.rb create mode 100644 spec/lib/emoji_formatter_spec.rb delete mode 100644 spec/lib/formatter_spec.rb create mode 100644 spec/lib/html_aware_formatter.rb create mode 100644 spec/lib/plain_text_formatter_spec.rb create mode 100644 spec/lib/text_formatter_spec.rb (limited to 'spec') diff --git a/app/chewy/statuses_index.rb b/app/chewy/statuses_index.rb index 65cbb6fcd..d119f7cac 100644 --- a/app/chewy/statuses_index.rb +++ b/app/chewy/statuses_index.rb @@ -57,7 +57,7 @@ class StatusesIndex < Chewy::Index field :id, type: 'long' field :account_id, type: 'long' - field :text, type: 'text', value: ->(status) { [status.spoiler_text, Formatter.instance.plaintext(status)].concat(status.ordered_media_attachments.map(&:description)).concat(status.preloadable_poll ? status.preloadable_poll.options : []).join("\n\n") } do + field :text, type: 'text', value: ->(status) { [status.spoiler_text, PlainTextFormatter.new(status.text, status.local?).to_s].concat(status.ordered_media_attachments.map(&:description)).concat(status.preloadable_poll ? status.preloadable_poll.options : []).join("\n\n") } do field :stemmed, type: 'text', analyzer: 'content' end diff --git a/app/controllers/api/web/embeds_controller.rb b/app/controllers/api/web/embeds_controller.rb index 741ba910f..58f6345e6 100644 --- a/app/controllers/api/web/embeds_controller.rb +++ b/app/controllers/api/web/embeds_controller.rb @@ -15,7 +15,7 @@ class Api::Web::EmbedsController < Api::Web::BaseController return not_found if oembed.nil? begin - oembed[:html] = Formatter.instance.sanitize(oembed[:html], Sanitize::Config::MASTODON_OEMBED) + oembed[:html] = Sanitize.fragment(oembed[:html], Sanitize::Config::MASTODON_OEMBED) rescue ArgumentError return not_found end diff --git a/app/helpers/accounts_helper.rb b/app/helpers/accounts_helper.rb index a33961724..557f60f26 100644 --- a/app/helpers/accounts_helper.rb +++ b/app/helpers/accounts_helper.rb @@ -2,10 +2,12 @@ module AccountsHelper def display_name(account, **options) + str = account.display_name.presence || account.username + if options[:custom_emojify] - Formatter.instance.format_display_name(account, **options) + prerender_custom_emojis(h(str), account.emojis) else - account.display_name.presence || account.username + str end end diff --git a/app/helpers/admin/trends/statuses_helper.rb b/app/helpers/admin/trends/statuses_helper.rb index d16e3dd12..214c1e2a6 100644 --- a/app/helpers/admin/trends/statuses_helper.rb +++ b/app/helpers/admin/trends/statuses_helper.rb @@ -12,9 +12,6 @@ module Admin::Trends::StatusesHelper return '' if text.blank? - html = Formatter.instance.send(:encode, text) - html = Formatter.instance.send(:encode_custom_emojis, html, status.emojis, prefers_autoplay?) - - html.html_safe # rubocop:disable Rails/OutputSafety + prerender_custom_emojis(h(text), status.emojis) end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index e997570b5..651a98a85 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -239,4 +239,8 @@ module ApplicationHelper end end.values end + + def prerender_custom_emojis(html, custom_emojis) + EmojiFormatter.new(html, custom_emojis, animate: prefers_autoplay?).to_s + end end diff --git a/app/helpers/formatting_helper.rb b/app/helpers/formatting_helper.rb new file mode 100644 index 000000000..66e9e1e91 --- /dev/null +++ b/app/helpers/formatting_helper.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module FormattingHelper + def html_aware_format(text, local, options = {}) + HtmlAwareFormatter.new(text, local, options).to_s + end + + def linkify(text, options = {}) + TextFormatter.new(text, options).to_s + end + + def extract_plain_text(text, local) + PlainTextFormatter.new(text, local).to_s + end + + def status_content_format(status) + html_aware_format(status.text, status.local?, preloaded_accounts: [status.account] + (status.respond_to?(:active_mentions) ? status.active_mentions.map(&:account) : [])) + end +end diff --git a/app/helpers/routing_helper.rb b/app/helpers/routing_helper.rb index fb24a1b28..f95f46a56 100644 --- a/app/helpers/routing_helper.rb +++ b/app/helpers/routing_helper.rb @@ -2,6 +2,7 @@ module RoutingHelper extend ActiveSupport::Concern + include Rails.application.routes.url_helpers include ActionView::Helpers::AssetTagHelper include Webpacker::Helper @@ -22,8 +23,6 @@ module RoutingHelper full_asset_url(asset_pack_path(source, **options)) end - private - def use_storage? Rails.configuration.x.use_s3 || Rails.configuration.x.use_swift end diff --git a/app/helpers/statuses_helper.rb b/app/helpers/statuses_helper.rb index d328f89b7..e92b4c839 100644 --- a/app/helpers/statuses_helper.rb +++ b/app/helpers/statuses_helper.rb @@ -113,20 +113,6 @@ module StatusesHelper end end - private - - def simplified_text(text) - text.dup.tap do |new_text| - URI.extract(new_text).each do |url| - new_text.gsub!(url, '') - end - - new_text.gsub!(Account::MENTION_RE, '') - new_text.gsub!(Tag::HASHTAG_RE, '') - new_text.gsub!(/\s+/, '') - end - end - def embedded_view? params[:controller] == EMBEDDED_CONTROLLER && params[:action] == EMBEDDED_ACTION end diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index ea8d146d4..f4f98e29c 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class ActivityPub::Activity::Create < ActivityPub::Activity + include FormattingHelper + def perform dereference_object! @@ -367,7 +369,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity end def converted_text - Formatter.instance.linkify([@status_parser.title.presence, @status_parser.spoiler_text.presence, @status_parser.url || @status_parser.uri].compact.join("\n\n")) + linkify([@status_parser.title.presence, @status_parser.spoiler_text.presence, @status_parser.url || @status_parser.uri].compact.join("\n\n")) end def unsupported_media_type?(mime_type) diff --git a/app/lib/emoji_formatter.rb b/app/lib/emoji_formatter.rb new file mode 100644 index 000000000..f808f3a22 --- /dev/null +++ b/app/lib/emoji_formatter.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +class EmojiFormatter + include RoutingHelper + + DISALLOWED_BOUNDING_REGEX = /[[:alnum:]:]/.freeze + + attr_reader :html, :custom_emojis, :options + + # @param [ActiveSupport::SafeBuffer] html + # @param [Array] custom_emojis + # @param [Hash] options + # @option options [Boolean] :animate + def initialize(html, custom_emojis, options = {}) + raise ArgumentError unless html.html_safe? + + @html = html + @custom_emojis = custom_emojis + @options = options + end + + def to_s + return html if custom_emojis.empty? || html.blank? + + i = -1 + tag_open_index = nil + inside_shortname = false + shortname_start_index = -1 + invisible_depth = 0 + last_index = 0 + result = ''.dup + + while i + 1 < html.size + i += 1 + + if invisible_depth.zero? && inside_shortname && html[i] == ':' + inside_shortname = false + shortcode = html[shortname_start_index + 1..i - 1] + char_after = html[i + 1] + + next unless (char_after.nil? || !DISALLOWED_BOUNDING_REGEX.match?(char_after)) && (emoji = emoji_map[shortcode]) + + result << html[last_index..shortname_start_index - 1] if shortname_start_index.positive? + result << image_for_emoji(shortcode, emoji) + last_index = i + 1 + elsif tag_open_index && html[i] == '>' + tag = html[tag_open_index..i] + tag_open_index = nil + + if invisible_depth.positive? + invisible_depth += count_tag_nesting(tag) + elsif tag == '' - end - end - - context 'given a post containing unlinkable mentions' do - let(:status) { Fabricate(:status, text: '@alice', uri: nil) } - - it 'does not create a mention link' do - is_expected.to include '@alice' - end - end - - context do - subject do - status = Fabricate(:status, text: text, uri: nil) - Formatter.instance.format(status) - end - - include_examples 'encode and link URLs' - end - - context 'given a post with custom_emojify option' do - let!(:emoji) { Fabricate(:custom_emoji) } - let(:status) { Fabricate(:status, account: local_account, text: text) } - - subject { Formatter.instance.format(status, custom_emojify: true) } - - context 'given a post with an emoji shortcode at the start' do - let(:text) { ':coolcat: Beep boop' } - - it 'converts the shortcode to an image tag' do - is_expected.to match(/

:coolcat::coolcat: Beep boop
' } - - it 'converts the shortcode to an image tag' do - is_expected.to match(/

:coolcat:Beep :coolcat: boop

' } - - it 'converts the shortcode to an image tag' do - is_expected.to match(/Beep :coolcat::coolcat::coolcat:

' } - - it 'does not touch the shortcodes' do - is_expected.to match(/

:coolcat::coolcat:<\/p>/) - end - end - - context 'given a post with an emoji shortcode at the end' do - let(:text) { '

Beep boop
:coolcat:

' } - - it 'converts the shortcode to an image tag' do - is_expected.to match(/
:coolcat:alert("Hello")' } - - it 'strips the scripts' do - is_expected.to_not include '' - end - end - - context 'given a post containing malicious classes' do - let(:text) { 'Show more' } - - it 'strips the malicious classes' do - is_expected.to_not include 'status__content__spoiler-link' - end - end - end - - describe '#plaintext' do - subject { Formatter.instance.plaintext(status) } - - context 'given a post with local status' do - let(:status) { Fabricate(:status, text: '

a text by a nerd who uses an HTML tag in text

', uri: nil) } - - it 'returns the raw text' do - is_expected.to eq '

a text by a nerd who uses an HTML tag in text

' - end - end - - context 'given a post with remote status' do - let(:status) { Fabricate(:status, account: remote_account, text: '') } - - it 'returns tag-stripped text' do - is_expected.to eq '' - end - end - end - - describe '#simplified_format' do - subject { Formatter.instance.simplified_format(account) } - - context 'given a post with local status' do - let(:account) { Fabricate(:account, domain: nil, note: text) } - - context 'given a post containing linkable mentions for local accounts' do - let(:text) { '@alice' } - - before { local_account } - - it 'creates a mention link' do - is_expected.to eq '

@alice

' - end - end - - context 'given a post containing linkable mentions for remote accounts' do - let(:text) { '@bob@remote.test' } - - before { remote_account } - - it 'creates a mention link' do - is_expected.to eq '

@bob

' - end - end - - context 'given a post containing unlinkable mentions' do - let(:text) { '@alice' } - - it 'does not create a mention link' do - is_expected.to eq '

@alice

' - end - end - - context 'given a post with custom_emojify option' do - let!(:emoji) { Fabricate(:custom_emoji) } - - before { account.note = text } - subject { Formatter.instance.simplified_format(account, custom_emojify: true) } - - context 'given a post with an emoji shortcode at the start' do - let(:text) { ':coolcat: Beep boop' } - - it 'converts the shortcode to an image tag' do - is_expected.to match(/

:coolcat:alert("Hello")' } - let(:account) { Fabricate(:account, domain: 'remote', note: text) } - - it 'reformats' do - is_expected.to_not include '' - end - - context 'with custom_emojify option' do - let!(:emoji) { Fabricate(:custom_emoji, domain: remote_account.domain) } - - before { remote_account.note = text } - - subject { Formatter.instance.simplified_format(remote_account, custom_emojify: true) } - - context 'given a post with an emoji shortcode at the start' do - let(:text) { '

:coolcat: Beep boop
' } - - it 'converts shortcode to image tag' do - is_expected.to match(/

:coolcat:Beep :coolcat: boop

' } - - it 'converts shortcode to image tag' do - is_expected.to match(/Beep :coolcat::coolcat::coolcat:

' } - - it 'does not touch the shortcodes' do - is_expected.to match(/

:coolcat::coolcat:<\/p>/) - end - end - - context 'given a post with an emoji shortcode at the end' do - let(:text) { '

Beep boop
:coolcat:

' } - - it 'converts shortcode to image tag' do - is_expected.to match(/
:coolcat:alert("Hello")' } - - subject { Formatter.instance.sanitize(html, Sanitize::Config::MASTODON_STRICT) } - - it 'sanitizes' do - is_expected.to eq '' - end - end -end diff --git a/spec/lib/html_aware_formatter.rb b/spec/lib/html_aware_formatter.rb new file mode 100644 index 000000000..18d23abf5 --- /dev/null +++ b/spec/lib/html_aware_formatter.rb @@ -0,0 +1,44 @@ +require 'rails_helper' + +RSpec.describe HtmlAwareFormatter do + describe '#to_s' do + subject { described_class.new(text, local).to_s } + + context 'when local' do + let(:local) { true } + let(:text) { 'Foo bar' } + + it 'returns formatted text' do + is_expected.to eq '

Foo bar

' + end + end + + context 'when remote' do + let(:local) { false } + + context 'given plain text' do + let(:text) { 'Beep boop' } + + it 'keeps the plain text' do + is_expected.to include 'Beep boop' + end + end + + context 'given text containing script tags' do + let(:text) { '' } + + it 'strips the scripts' do + is_expected.to_not include '' + end + end + + context 'given text containing malicious classes' do + let(:text) { 'Show more' } + + it 'strips the malicious classes' do + is_expected.to_not include 'status__content__spoiler-link' + end + end + end + end +end diff --git a/spec/lib/plain_text_formatter_spec.rb b/spec/lib/plain_text_formatter_spec.rb new file mode 100644 index 000000000..c3d0ee630 --- /dev/null +++ b/spec/lib/plain_text_formatter_spec.rb @@ -0,0 +1,24 @@ +require 'rails_helper' + +RSpec.describe PlainTextFormatter do + describe '#to_s' do + subject { described_class.new(status.text, status.local?).to_s } + + context 'given a post with local status' do + let(:status) { Fabricate(:status, text: '

a text by a nerd who uses an HTML tag in text

', uri: nil) } + + it 'returns the raw text' do + is_expected.to eq '

a text by a nerd who uses an HTML tag in text

' + end + end + + context 'given a post with remote status' do + let(:remote_account) { Fabricate(:account, domain: 'remote.test', username: 'bob', url: 'https://remote.test/') } + let(:status) { Fabricate(:status, account: remote_account, text: '

Hello

') } + + it 'returns tag-stripped text' do + is_expected.to eq 'Hello' + end + end + end +end diff --git a/spec/lib/text_formatter_spec.rb b/spec/lib/text_formatter_spec.rb new file mode 100644 index 000000000..52a9d2498 --- /dev/null +++ b/spec/lib/text_formatter_spec.rb @@ -0,0 +1,313 @@ +require 'rails_helper' + +RSpec.describe TextFormatter do + describe '#to_s' do + let(:preloaded_accounts) { nil } + + subject { described_class.new(text, preloaded_accounts: preloaded_accounts).to_s } + + context 'given text containing plain text' do + let(:text) { 'text' } + + it 'paragraphizes the text' do + is_expected.to eq '

text

' + end + end + + context 'given text containing line feeds' do + let(:text) { "line\nfeed" } + + it 'removes line feeds' do + is_expected.not_to include "\n" + end + end + + context 'given text containing linkable mentions' do + let(:preloaded_accounts) { [Fabricate(:account, username: 'alice')] } + let(:text) { '@alice' } + + it 'creates a mention link' do + is_expected.to include '@alice' + end + end + + context 'given text containing unlinkable mentions' do + let(:preloaded_accounts) { [] } + let(:text) { '@alice' } + + it 'does not create a mention link' do + is_expected.to include '@alice' + end + end + + context 'given a stand-alone medium URL' do + let(:text) { 'https://hackernoon.com/the-power-to-build-communities-a-response-to-mark-zuckerberg-3f2cac9148a4' } + + it 'matches the full URL' do + is_expected.to include 'href="https://hackernoon.com/the-power-to-build-communities-a-response-to-mark-zuckerberg-3f2cac9148a4"' + end + end + + context 'given a stand-alone google URL' do + let(:text) { 'http://google.com' } + + it 'matches the full URL' do + is_expected.to include 'href="http://google.com"' + end + end + + context 'given a stand-alone URL with a newer TLD' do + let(:text) { 'http://example.gay' } + + it 'matches the full URL' do + is_expected.to include 'href="http://example.gay"' + end + end + + context 'given a stand-alone IDN URL' do + let(:text) { 'https://nic.みんな/' } + + it 'matches the full URL' do + is_expected.to include 'href="https://nic.みんな/"' + end + + it 'has display URL' do + is_expected.to include 'nic.みんな/' + end + end + + context 'given a URL with a trailing period' do + let(:text) { 'http://www.mcmansionhell.com/post/156408871451/50-states-of-mcmansion-hell-scottsdale-arizona. ' } + + it 'matches the full URL but not the period' do + is_expected.to include 'href="http://www.mcmansionhell.com/post/156408871451/50-states-of-mcmansion-hell-scottsdale-arizona"' + end + end + + context 'given a URL enclosed with parentheses' do + let(:text) { '(http://google.com/)' } + + it 'matches the full URL but not the parentheses' do + is_expected.to include 'href="http://google.com/"' + end + end + + context 'given a URL with a trailing exclamation point' do + let(:text) { 'http://www.google.com!' } + + it 'matches the full URL but not the exclamation point' do + is_expected.to include 'href="http://www.google.com"' + end + end + + context 'given a URL with a trailing single quote' do + let(:text) { "http://www.google.com'" } + + it 'matches the full URL but not the single quote' do + is_expected.to include 'href="http://www.google.com"' + end + end + + context 'given a URL with a trailing angle bracket' do + let(:text) { 'http://www.google.com>' } + + it 'matches the full URL but not the angle bracket' do + is_expected.to include 'href="http://www.google.com"' + end + end + + context 'given a URL with a query string' do + context 'with escaped unicode character' do + let(:text) { 'https://www.ruby-toolbox.com/search?utf8=%E2%9C%93&q=autolink' } + + it 'matches the full URL' do + is_expected.to include 'href="https://www.ruby-toolbox.com/search?utf8=%E2%9C%93&q=autolink"' + end + end + + context 'with unicode character' do + let(:text) { 'https://www.ruby-toolbox.com/search?utf8=✓&q=autolink' } + + it 'matches the full URL' do + is_expected.to include 'href="https://www.ruby-toolbox.com/search?utf8=✓&q=autolink"' + end + end + + context 'with unicode character at the end' do + let(:text) { 'https://www.ruby-toolbox.com/search?utf8=✓' } + + it 'matches the full URL' do + is_expected.to include 'href="https://www.ruby-toolbox.com/search?utf8=✓"' + end + end + + context 'with escaped and not escaped unicode characters' do + let(:text) { 'https://www.ruby-toolbox.com/search?utf8=%E2%9C%93&utf81=✓&q=autolink' } + + it 'preserves escaped unicode characters' do + is_expected.to include 'href="https://www.ruby-toolbox.com/search?utf8=%E2%9C%93&utf81=✓&q=autolink"' + end + end + end + + context 'given a URL with parentheses in it' do + let(:text) { 'https://en.wikipedia.org/wiki/Diaspora_(software)' } + + it 'matches the full URL' do + is_expected.to include 'href="https://en.wikipedia.org/wiki/Diaspora_(software)"' + end + end + + context 'given a URL in quotation marks' do + let(:text) { '"https://example.com/"' } + + it 'does not match the quotation marks' do + is_expected.to include 'href="https://example.com/"' + end + end + + context 'given a URL in angle brackets' do + let(:text) { '' } + + it 'does not match the angle brackets' do + is_expected.to include 'href="https://example.com/"' + end + end + + context 'given a URL with Japanese path string' do + let(:text) { 'https://ja.wikipedia.org/wiki/日本' } + + it 'matches the full URL' do + is_expected.to include 'href="https://ja.wikipedia.org/wiki/日本"' + end + end + + context 'given a URL with Korean path string' do + let(:text) { 'https://ko.wikipedia.org/wiki/대한민국' } + + it 'matches the full URL' do + is_expected.to include 'href="https://ko.wikipedia.org/wiki/대한민국"' + end + end + + context 'given a URL with a full-width space' do + let(:text) { 'https://example.com/　abc123' } + + it 'does not match the full-width space' do + is_expected.to include 'href="https://example.com/"' + end + end + + context 'given a URL in Japanese quotation marks' do + let(:text) { '「[https://example.org/」' } + + it 'does not match the quotation marks' do + is_expected.to include 'href="https://example.org/"' + end + end + + context 'given a URL with Simplified Chinese path string' do + let(:text) { 'https://baike.baidu.com/item/中华人民共和国' } + + it 'matches the full URL' do + is_expected.to include 'href="https://baike.baidu.com/item/中华人民共和国"' + end + end + + context 'given a URL with Traditional Chinese path string' do + let(:text) { 'https://zh.wikipedia.org/wiki/臺灣' } + + it 'matches the full URL' do + is_expected.to include 'href="https://zh.wikipedia.org/wiki/臺灣"' + end + end + + context 'given a URL containing unsafe code (XSS attack, visible part)' do + let(:text) { %q{http://example.com/bb} } + + it 'does not include the HTML in the URL' do + is_expected.to include '"http://example.com/b"' + end + + it 'escapes the HTML' do + is_expected.to include '<del>b</del>' + end + end + + context 'given a URL containing unsafe code (XSS attack, invisible part)' do + let(:text) { %q{http://example.com/blahblahblahblah/a} } + + it 'does not include the HTML in the URL' do + is_expected.to include '"http://example.com/blahblahblahblah/a"' + end + + it 'escapes the HTML' do + is_expected.to include '<script>alert("Hello")</script>' + end + end + + context 'given text containing HTML code (script tag)' do + let(:text) { '' } + + it 'escapes the HTML' do + is_expected.to include '

<script>alert("Hello")</script>

' + end + end + + context 'given text containing HTML (XSS attack)' do + let(:text) { %q{} } + + it 'escapes the HTML' do + is_expected.to include '

<img src="javascript:alert('XSS');">

' + end + end + + context 'given an invalid URL' do + let(:text) { 'http://www\.google\.com' } + + it 'outputs the raw URL' do + is_expected.to eq '

http://www\.google\.com

' + end + end + + context 'given text containing a hashtag' do + let(:text) { '#hashtag' } + + it 'creates a hashtag link' do + is_expected.to include '/tags/hashtag" class="mention hashtag" rel="tag">#hashtag' + end + end + + context 'given text containing a hashtag with Unicode chars' do + let(:text) { '#hashtagタグ' } + + it 'creates a hashtag link' do + is_expected.to include '/tags/hashtag%E3%82%BF%E3%82%B0" class="mention hashtag" rel="tag">#hashtagタグ' + end + end + + context 'given text with a stand-alone xmpp: URI' do + let(:text) { 'xmpp:user@instance.com' } + + it 'matches the full URI' do + is_expected.to include 'href="xmpp:user@instance.com"' + end + end + + context 'given text with an xmpp: URI with a query-string' do + let(:text) { 'please join xmpp:muc@instance.com?join right now' } + + it 'matches the full URI' do + is_expected.to include 'href="xmpp:muc@instance.com?join"' + end + end + + context 'given text containing a magnet: URI' do + let(:text) { 'wikipedia gives this example of a magnet uri: magnet:?xt=urn:btih:c12fe1c06bba254a9dc9f519b335aa7c1367a88a' } + + it 'matches the full URI' do + is_expected.to include 'href="magnet:?xt=urn:btih:c12fe1c06bba254a9dc9f519b335aa7c1367a88a"' + end + end + end +end -- cgit From 30658924a80434e6a2bceb61267b911ea8d37898 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 28 Mar 2022 12:43:58 +0200 Subject: Fix test-related issues (#17888) * Remove obsolete RSS::Serializer test Since #17828, RSS::Serializer no longer has specific code for deleted statuses, but it is never called on deleted statuses anyway. * Rename erroneously-named test files * Fix failing test * Fix test deprecation warnings * Update CircleCI Ruby orb 1.4.0 has a bug that does not match all the test files due to incorrect globbing --- .circleci/config.yml | 2 +- spec/controllers/admin/accounts_controller_spec.rb | 10 ++--- .../settings/exports/bookmarks_controller_spec.rb | 22 +++++++++++ .../settings/exports/bookmarks_controller_specs.rb | 17 -------- spec/lib/html_aware_formatter.rb | 44 --------------------- spec/lib/html_aware_formatter_spec.rb | 44 +++++++++++++++++++++ spec/lib/rss/serializer_spec.rb | 7 ---- spec/services/after_block_service_spec.rb | 8 ++-- spec/services/delete_account_service_spec.rb | 14 +++---- spec/services/mute_service_spec.rb | 22 ++++------- spec/services/notify_service_spec.rb | 46 +++++++++++----------- spec/services/suspend_account_service_spec.rb | 12 +++--- spec/services/unsuspend_account_service_spec.rb | 26 ++++++------ 13 files changed, 127 insertions(+), 147 deletions(-) create mode 100644 spec/controllers/settings/exports/bookmarks_controller_spec.rb delete mode 100644 spec/controllers/settings/exports/bookmarks_controller_specs.rb delete mode 100644 spec/lib/html_aware_formatter.rb create mode 100644 spec/lib/html_aware_formatter_spec.rb (limited to 'spec') diff --git a/.circleci/config.yml b/.circleci/config.yml index 4fcc8c618..b9228f996 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,7 +1,7 @@ version: 2.1 orbs: - ruby: circleci/ruby@1.4.0 + ruby: circleci/ruby@1.4.1 node: circleci/node@5.0.1 executors: diff --git a/spec/controllers/admin/accounts_controller_spec.rb b/spec/controllers/admin/accounts_controller_spec.rb index 0f71d697c..1779fb7c0 100644 --- a/spec/controllers/admin/accounts_controller_spec.rb +++ b/spec/controllers/admin/accounts_controller_spec.rb @@ -194,9 +194,7 @@ RSpec.describe Admin::AccountsController, type: :controller do end describe 'POST #unblock_email' do - subject do - -> { post :unblock_email, params: { id: account.id } } - end + subject { post :unblock_email, params: { id: account.id } } let(:current_user) { Fabricate(:user, admin: admin) } let(:account) { Fabricate(:account, suspended: true) } @@ -206,11 +204,11 @@ RSpec.describe Admin::AccountsController, type: :controller do let(:admin) { true } it 'succeeds in removing email blocks' do - is_expected.to change { CanonicalEmailBlock.where(reference_account: account).count }.from(1).to(0) + expect { subject }.to change { CanonicalEmailBlock.where(reference_account: account).count }.from(1).to(0) end it 'redirects to admin account path' do - subject.call + subject expect(response).to redirect_to admin_account_path(account.id) end end @@ -219,7 +217,7 @@ RSpec.describe Admin::AccountsController, type: :controller do let(:admin) { false } it 'fails to remove avatar' do - subject.call + subject expect(response).to have_http_status :forbidden end end diff --git a/spec/controllers/settings/exports/bookmarks_controller_spec.rb b/spec/controllers/settings/exports/bookmarks_controller_spec.rb new file mode 100644 index 000000000..a06c02e0c --- /dev/null +++ b/spec/controllers/settings/exports/bookmarks_controller_spec.rb @@ -0,0 +1,22 @@ +require 'rails_helper' + +describe Settings::Exports::BookmarksController do + render_views + + let(:user) { Fabricate(:user) } + let(:account) { Fabricate(:account, domain: 'foo.bar') } + let(:status) { Fabricate(:status, account: account, uri: 'https://foo.bar/statuses/1312') } + + describe 'GET #index' do + before do + user.account.bookmarks.create!(status: status) + end + + it 'returns a csv of the bookmarked toots' do + sign_in user, scope: :user + get :index, format: :csv + + expect(response.body).to eq "https://foo.bar/statuses/1312\n" + end + end +end diff --git a/spec/controllers/settings/exports/bookmarks_controller_specs.rb b/spec/controllers/settings/exports/bookmarks_controller_specs.rb deleted file mode 100644 index 85761577b..000000000 --- a/spec/controllers/settings/exports/bookmarks_controller_specs.rb +++ /dev/null @@ -1,17 +0,0 @@ -require 'rails_helper' - -describe Settings::Exports::BookmarksController do - render_views - - describe 'GET #index' do - it 'returns a csv of the bookmarked toots' do - user = Fabricate(:user) - user.account.bookmarks.create!(status: Fabricate(:status, uri: 'https://foo.bar/statuses/1312')) - - sign_in user, scope: :user - get :index, format: :csv - - expect(response.body).to eq "https://foo.bar/statuses/1312\n" - end - end -end diff --git a/spec/lib/html_aware_formatter.rb b/spec/lib/html_aware_formatter.rb deleted file mode 100644 index 18d23abf5..000000000 --- a/spec/lib/html_aware_formatter.rb +++ /dev/null @@ -1,44 +0,0 @@ -require 'rails_helper' - -RSpec.describe HtmlAwareFormatter do - describe '#to_s' do - subject { described_class.new(text, local).to_s } - - context 'when local' do - let(:local) { true } - let(:text) { 'Foo bar' } - - it 'returns formatted text' do - is_expected.to eq '

Foo bar

' - end - end - - context 'when remote' do - let(:local) { false } - - context 'given plain text' do - let(:text) { 'Beep boop' } - - it 'keeps the plain text' do - is_expected.to include 'Beep boop' - end - end - - context 'given text containing script tags' do - let(:text) { '' } - - it 'strips the scripts' do - is_expected.to_not include '' - end - end - - context 'given text containing malicious classes' do - let(:text) { 'Show more' } - - it 'strips the malicious classes' do - is_expected.to_not include 'status__content__spoiler-link' - end - end - end - end -end diff --git a/spec/lib/html_aware_formatter_spec.rb b/spec/lib/html_aware_formatter_spec.rb new file mode 100644 index 000000000..18d23abf5 --- /dev/null +++ b/spec/lib/html_aware_formatter_spec.rb @@ -0,0 +1,44 @@ +require 'rails_helper' + +RSpec.describe HtmlAwareFormatter do + describe '#to_s' do + subject { described_class.new(text, local).to_s } + + context 'when local' do + let(:local) { true } + let(:text) { 'Foo bar' } + + it 'returns formatted text' do + is_expected.to eq '

Foo bar

' + end + end + + context 'when remote' do + let(:local) { false } + + context 'given plain text' do + let(:text) { 'Beep boop' } + + it 'keeps the plain text' do + is_expected.to include 'Beep boop' + end + end + + context 'given text containing script tags' do + let(:text) { '' } + + it 'strips the scripts' do + is_expected.to_not include '' + end + end + + context 'given text containing malicious classes' do + let(:text) { 'Show more' } + + it 'strips the malicious classes' do + is_expected.to_not include 'status__content__spoiler-link' + end + end + end + end +end diff --git a/spec/lib/rss/serializer_spec.rb b/spec/lib/rss/serializer_spec.rb index 0364d13de..1da45d302 100644 --- a/spec/lib/rss/serializer_spec.rb +++ b/spec/lib/rss/serializer_spec.rb @@ -13,13 +13,6 @@ describe RSS::Serializer do subject { RSS::Serializer.new.send(:status_title, status) } - context 'if destroyed?' do - it 'returns "#{account.acct} deleted status"' do - status.destroy! - expect(subject).to eq "#{account.acct} deleted status" - end - end - context 'on a toot with long text' do let(:text) { "This toot's text is longer than the allowed number of characters" } diff --git a/spec/services/after_block_service_spec.rb b/spec/services/after_block_service_spec.rb index fe5b26b2b..c09425d7c 100644 --- a/spec/services/after_block_service_spec.rb +++ b/spec/services/after_block_service_spec.rb @@ -1,9 +1,7 @@ require 'rails_helper' RSpec.describe AfterBlockService, type: :service do - subject do - -> { described_class.new.call(account, target_account) } - end + subject { described_class.new.call(account, target_account) } let(:account) { Fabricate(:account) } let(:target_account) { Fabricate(:account) } @@ -24,7 +22,7 @@ RSpec.describe AfterBlockService, type: :service do FeedManager.instance.push_to_home(account, other_account_status) FeedManager.instance.push_to_home(account, other_account_reblog) - is_expected.to change { + expect { subject }.to change { Redis.current.zrange(home_timeline_key, 0, -1) }.from([status.id.to_s, other_account_status.id.to_s, other_account_reblog.id.to_s]).to([other_account_status.id.to_s]) end @@ -43,7 +41,7 @@ RSpec.describe AfterBlockService, type: :service do FeedManager.instance.push_to_list(list, other_account_status) FeedManager.instance.push_to_list(list, other_account_reblog) - is_expected.to change { + expect { subject }.to change { Redis.current.zrange(list_timeline_key, 0, -1) }.from([status.id.to_s, other_account_status.id.to_s, other_account_reblog.id.to_s]).to([other_account_status.id.to_s]) end diff --git a/spec/services/delete_account_service_spec.rb b/spec/services/delete_account_service_spec.rb index 9c785fc17..1fbe4d07c 100644 --- a/spec/services/delete_account_service_spec.rb +++ b/spec/services/delete_account_service_spec.rb @@ -23,12 +23,10 @@ RSpec.describe DeleteAccountService, type: :service do let!(:account_note) { Fabricate(:account_note, account: account) } - subject do - -> { described_class.new.call(account) } - end + subject { described_class.new.call(account) } it 'deletes associated owned records' do - is_expected.to change { + expect { subject }.to change { [ account.statuses, account.media_attachments, @@ -43,7 +41,7 @@ RSpec.describe DeleteAccountService, type: :service do end it 'deletes associated target records' do - is_expected.to change { + expect { subject }.to change { [ AccountPin.where(target_account: account), ].map(&:count) @@ -51,7 +49,7 @@ RSpec.describe DeleteAccountService, type: :service do end it 'deletes associated target notifications' do - is_expected.to change { + expect { subject }.to change { [ 'poll', 'favourite', 'status', 'mention', 'follow' ].map { |type| Notification.where(type: type).count } @@ -73,7 +71,7 @@ RSpec.describe DeleteAccountService, type: :service do let!(:local_follower) { Fabricate(:account) } it 'sends a delete actor activity to all known inboxes' do - subject.call + subject expect(a_request(:post, "https://alice.com/inbox")).to have_been_made.once expect(a_request(:post, "https://bob.com/inbox")).to have_been_made.once end @@ -91,7 +89,7 @@ RSpec.describe DeleteAccountService, type: :service do let!(:local_follower) { Fabricate(:account) } it 'sends a reject follow to follower inboxes' do - subject.call + subject expect(a_request(:post, account.inbox_url)).to have_been_made.once end end diff --git a/spec/services/mute_service_spec.rb b/spec/services/mute_service_spec.rb index 4bb839b8d..bdec1c67b 100644 --- a/spec/services/mute_service_spec.rb +++ b/spec/services/mute_service_spec.rb @@ -1,9 +1,7 @@ require 'rails_helper' RSpec.describe MuteService, type: :service do - subject do - -> { described_class.new.call(account, target_account) } - end + subject { described_class.new.call(account, target_account) } let(:account) { Fabricate(:account) } let(:target_account) { Fabricate(:account) } @@ -21,45 +19,41 @@ RSpec.describe MuteService, type: :service do FeedManager.instance.push_to_home(account, status) FeedManager.instance.push_to_home(account, other_account_status) - is_expected.to change { + expect { subject }.to change { Redis.current.zrange(home_timeline_key, 0, -1) }.from([status.id.to_s, other_account_status.id.to_s]).to([other_account_status.id.to_s]) end end it 'mutes account' do - is_expected.to change { + expect { subject }.to change { account.muting?(target_account) }.from(false).to(true) end context 'without specifying a notifications parameter' do it 'mutes notifications from the account' do - is_expected.to change { + expect { subject }.to change { account.muting_notifications?(target_account) }.from(false).to(true) end end context 'with a true notifications parameter' do - subject do - -> { described_class.new.call(account, target_account, notifications: true) } - end + subject { described_class.new.call(account, target_account, notifications: true) } it 'mutes notifications from the account' do - is_expected.to change { + expect { subject }.to change { account.muting_notifications?(target_account) }.from(false).to(true) end end context 'with a false notifications parameter' do - subject do - -> { described_class.new.call(account, target_account, notifications: false) } - end + subject { described_class.new.call(account, target_account, notifications: false) } it 'does not mute notifications from the account' do - is_expected.to_not change { + expect { subject }.to_not change { account.muting_notifications?(target_account) }.from(false) end diff --git a/spec/services/notify_service_spec.rb b/spec/services/notify_service_spec.rb index 7433866b7..294c31b04 100644 --- a/spec/services/notify_service_spec.rb +++ b/spec/services/notify_service_spec.rb @@ -1,9 +1,7 @@ require 'rails_helper' RSpec.describe NotifyService, type: :service do - subject do - -> { described_class.new.call(recipient, type, activity) } - end + subject { described_class.new.call(recipient, type, activity) } let(:user) { Fabricate(:user) } let(:recipient) { user.account } @@ -11,42 +9,42 @@ RSpec.describe NotifyService, type: :service do let(:activity) { Fabricate(:follow, account: sender, target_account: recipient) } let(:type) { :follow } - it { is_expected.to change(Notification, :count).by(1) } + it { expect { subject }.to change(Notification, :count).by(1) } it 'does not notify when sender is blocked' do recipient.block!(sender) - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end it 'does not notify when sender is muted with hide_notifications' do recipient.mute!(sender, notifications: true) - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end it 'does notify when sender is muted without hide_notifications' do recipient.mute!(sender, notifications: false) - is_expected.to change(Notification, :count) + expect { subject }.to change(Notification, :count) end it 'does not notify when sender\'s domain is blocked' do recipient.block_domain!(sender.domain) - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end it 'does still notify when sender\'s domain is blocked but sender is followed' do recipient.block_domain!(sender.domain) recipient.follow!(sender) - is_expected.to change(Notification, :count) + expect { subject }.to change(Notification, :count) end it 'does not notify when sender is silenced and not followed' do sender.silence! - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end it 'does not notify when recipient is suspended' do recipient.suspend! - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end context 'for direct messages' do @@ -61,7 +59,7 @@ RSpec.describe NotifyService, type: :service do let(:enabled) { true } it 'does not notify' do - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end context 'if the message chain is initiated by recipient, but is not direct message' do @@ -70,7 +68,7 @@ RSpec.describe NotifyService, type: :service do let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, visibility: :direct, thread: reply_to)) } it 'does not notify' do - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end end @@ -81,7 +79,7 @@ RSpec.describe NotifyService, type: :service do let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, visibility: :direct, thread: dummy_reply)) } it 'does not notify' do - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end end @@ -91,7 +89,7 @@ RSpec.describe NotifyService, type: :service do let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, visibility: :direct, thread: reply_to)) } it 'does notify' do - is_expected.to change(Notification, :count) + expect { subject }.to change(Notification, :count) end end end @@ -100,7 +98,7 @@ RSpec.describe NotifyService, type: :service do let(:enabled) { false } it 'does notify' do - is_expected.to change(Notification, :count) + expect { subject }.to change(Notification, :count) end end end @@ -112,17 +110,17 @@ RSpec.describe NotifyService, type: :service do it 'shows reblogs by default' do recipient.follow!(sender) - is_expected.to change(Notification, :count) + expect { subject }.to change(Notification, :count) end it 'shows reblogs when explicitly enabled' do recipient.follow!(sender, reblogs: true) - is_expected.to change(Notification, :count) + expect { subject }.to change(Notification, :count) end it 'shows reblogs when disabled' do recipient.follow!(sender, reblogs: false) - is_expected.to change(Notification, :count) + expect { subject }.to change(Notification, :count) end end @@ -134,12 +132,12 @@ RSpec.describe NotifyService, type: :service do it 'does not notify when conversation is muted' do recipient.mute_conversation!(activity.status.conversation) - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end it 'does not notify when it is a reply to a blocked user' do recipient.block!(asshole) - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end end @@ -147,7 +145,7 @@ RSpec.describe NotifyService, type: :service do let(:sender) { recipient } it 'does not notify when recipient is the sender' do - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end end @@ -163,7 +161,7 @@ RSpec.describe NotifyService, type: :service do let(:enabled) { true } it 'sends email' do - is_expected.to change(ActionMailer::Base.deliveries, :count).by(1) + expect { subject }.to change(ActionMailer::Base.deliveries, :count).by(1) end end @@ -171,7 +169,7 @@ RSpec.describe NotifyService, type: :service do let(:enabled) { false } it "doesn't send email" do - is_expected.to_not change(ActionMailer::Base.deliveries, :count).from(0) + expect { subject }.to_not change(ActionMailer::Base.deliveries, :count).from(0) end end end diff --git a/spec/services/suspend_account_service_spec.rb b/spec/services/suspend_account_service_spec.rb index cf7eb257a..5d45e4ffd 100644 --- a/spec/services/suspend_account_service_spec.rb +++ b/spec/services/suspend_account_service_spec.rb @@ -5,9 +5,7 @@ RSpec.describe SuspendAccountService, type: :service do let!(:local_follower) { Fabricate(:user, current_sign_in_at: 1.hour.ago).account } let!(:list) { Fabricate(:list, account: local_follower) } - subject do - -> { described_class.new.call(account) } - end + subject { described_class.new.call(account) } before do allow(FeedManager.instance).to receive(:unmerge_from_home).and_return(nil) @@ -18,13 +16,13 @@ RSpec.describe SuspendAccountService, type: :service do end it "unmerges from local followers' feeds" do - subject.call + subject expect(FeedManager.instance).to have_received(:unmerge_from_home).with(account, local_follower) expect(FeedManager.instance).to have_received(:unmerge_from_list).with(account, list) end it 'marks account as suspended' do - is_expected.to change { account.suspended? }.from(false).to(true) + expect { subject }.to change { account.suspended? }.from(false).to(true) end end @@ -51,7 +49,7 @@ RSpec.describe SuspendAccountService, type: :service do end it 'sends an update actor to followers and reporters' do - subject.call + subject expect(a_request(:post, remote_follower.inbox_url).with { |req| match_update_actor_request(req, account) }).to have_been_made.once expect(a_request(:post, remote_reporter.inbox_url).with { |req| match_update_actor_request(req, account) }).to have_been_made.once end @@ -77,7 +75,7 @@ RSpec.describe SuspendAccountService, type: :service do end it 'sends a reject follow' do - subject.call + subject expect(a_request(:post, account.inbox_url).with { |req| match_reject_follow_request(req, account, local_followee) }).to have_been_made.once end end diff --git a/spec/services/unsuspend_account_service_spec.rb b/spec/services/unsuspend_account_service_spec.rb index 0593beb6f..3ac4cc085 100644 --- a/spec/services/unsuspend_account_service_spec.rb +++ b/spec/services/unsuspend_account_service_spec.rb @@ -5,9 +5,7 @@ RSpec.describe UnsuspendAccountService, type: :service do let!(:local_follower) { Fabricate(:user, current_sign_in_at: 1.hour.ago).account } let!(:list) { Fabricate(:list, account: local_follower) } - subject do - -> { described_class.new.call(account) } - end + subject { described_class.new.call(account) } before do allow(FeedManager.instance).to receive(:merge_into_home).and_return(nil) @@ -33,7 +31,7 @@ RSpec.describe UnsuspendAccountService, type: :service do end it 'marks account as unsuspended' do - is_expected.to change { account.suspended? }.from(true).to(false) + expect { subject }.to change { account.suspended? }.from(true).to(false) end include_examples 'common behavior' do @@ -47,13 +45,13 @@ RSpec.describe UnsuspendAccountService, type: :service do end it "merges back into local followers' feeds" do - subject.call + subject expect(FeedManager.instance).to have_received(:merge_into_home).with(account, local_follower) expect(FeedManager.instance).to have_received(:merge_into_list).with(account, list) end it 'sends an update actor to followers and reporters' do - subject.call + subject expect(a_request(:post, remote_follower.inbox_url).with { |req| match_update_actor_request(req, account) }).to have_been_made.once expect(a_request(:post, remote_reporter.inbox_url).with { |req| match_update_actor_request(req, account) }).to have_been_made.once end @@ -75,18 +73,18 @@ RSpec.describe UnsuspendAccountService, type: :service do end it 're-fetches the account' do - subject.call + subject expect(resolve_account_service).to have_received(:call).with(account) end it "merges back into local followers' feeds" do - subject.call + subject expect(FeedManager.instance).to have_received(:merge_into_home).with(account, local_follower) expect(FeedManager.instance).to have_received(:merge_into_list).with(account, list) end it 'marks account as unsuspended' do - is_expected.to change { account.suspended? }.from(true).to(false) + expect { subject }.to change { account.suspended? }.from(true).to(false) end end @@ -99,18 +97,18 @@ RSpec.describe UnsuspendAccountService, type: :service do end it 're-fetches the account' do - subject.call + subject expect(resolve_account_service).to have_received(:call).with(account) end it "does not merge back into local followers' feeds" do - subject.call + subject expect(FeedManager.instance).to_not have_received(:merge_into_home).with(account, local_follower) expect(FeedManager.instance).to_not have_received(:merge_into_list).with(account, list) end it 'does not mark the account as unsuspended' do - is_expected.not_to change { account.suspended? } + expect { subject }.not_to change { account.suspended? } end end @@ -120,12 +118,12 @@ RSpec.describe UnsuspendAccountService, type: :service do end it 're-fetches the account' do - subject.call + subject expect(resolve_account_service).to have_received(:call).with(account) end it "does not merge back into local followers' feeds" do - subject.call + subject expect(FeedManager.instance).to_not have_received(:merge_into_home).with(account, local_follower) expect(FeedManager.instance).to_not have_received(:merge_into_list).with(account, list) end -- cgit From 61cefbebf717326bd6ec3923e67e3702a24a0b24 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 28 Mar 2022 20:51:51 +0200 Subject: Add advanced text formatting back into glitch-soc --- app/helpers/formatting_helper.rb | 2 +- app/lib/advanced_text_formatter.rb | 131 +++++++++++++++ app/lib/html_aware_formatter.rb | 6 +- lib/sanitize_ext/sanitize_config.rb | 57 +++++-- spec/lib/advanced_text_formatter_spec.rb | 274 +++++++++++++++++++++++++++++++ spec/lib/sanitize_config_spec.rb | 18 +- 6 files changed, 459 insertions(+), 29 deletions(-) create mode 100644 app/lib/advanced_text_formatter.rb create mode 100644 spec/lib/advanced_text_formatter_spec.rb (limited to 'spec') diff --git a/app/helpers/formatting_helper.rb b/app/helpers/formatting_helper.rb index e11156999..2a622ae0b 100644 --- a/app/helpers/formatting_helper.rb +++ b/app/helpers/formatting_helper.rb @@ -14,7 +14,7 @@ module FormattingHelper end def status_content_format(status) - html_aware_format(status.text, status.local?, preloaded_accounts: [status.account] + (status.respond_to?(:active_mentions) ? status.active_mentions.map(&:account) : [])) + html_aware_format(status.text, status.local?, preloaded_accounts: [status.account] + (status.respond_to?(:active_mentions) ? status.active_mentions.map(&:account) : []), content_type: status.content_type) end def account_bio_format(account) diff --git a/app/lib/advanced_text_formatter.rb b/app/lib/advanced_text_formatter.rb new file mode 100644 index 000000000..5ce87d306 --- /dev/null +++ b/app/lib/advanced_text_formatter.rb @@ -0,0 +1,131 @@ +# frozen_string_literal: true + +class AdvancedTextFormatter < TextFormatter + class HTMLRenderer < Redcarpet::Render::HTML + def initialize(options, &block) + super(options) + @format_link = block + end + + def block_code(code, _language) + <<~HTML.squish + 
#{h(code).gsub("\n", '
')}
+ HTML + end + + def autolink(link, link_type) + return link if link_type == :email + @format_link.call(link) + end + end + + # @param [String] text + # @param [Hash] options + # @option options [Boolean] :multiline + # @option options [Boolean] :with_domains + # @option options [Boolean] :with_rel_me + # @option options [Array] :preloaded_accounts + # @option options [String] :content_type + def initialize(text, options = {}) + content_type = options.delete(:content_type) + super(text, options) + + @text = format_markdown(text) if content_type == 'text/markdown' + end + + # Differs from TextFormatter by not messing with newline after parsing + def to_s + return ''.html_safe if text.blank? + + html = rewrite do |entity| + if entity[:url] + link_to_url(entity) + elsif entity[:hashtag] + link_to_hashtag(entity) + elsif entity[:screen_name] + link_to_mention(entity) + end + end + + html.html_safe # rubocop:disable Rails/OutputSafety + end + + # Differs from `TextFormatter` by skipping HTML tags and entities + def entities + @entities ||= begin + gaps = [] + total_offset = 0 + + escaped = text.gsub(/<[^>]*>|&#[0-9]+;/) do |match| + total_offset += match.length - 1 + end_offset = Regexp.last_match.end(0) + gaps << [end_offset - total_offset, total_offset] + ' ' + end + + Extractor.extract_entities_with_indices(escaped, extract_url_without_protocol: false).map do |entity| + start_pos, end_pos = entity[:indices] + offset_idx = gaps.rindex { |gap| gap.first <= start_pos } + offset = offset_idx.nil? ? 0 : gaps[offset_idx].last + entity.merge(indices: [start_pos + offset, end_pos + offset]) + end + end + end + + private + + # Differs from `TextFormatter` in that it keeps HTML; but it sanitizes at the end to remain safe + def rewrite + entities.sort_by! do |entity| + entity[:indices].first + end + + result = ''.dup + + last_index = entities.reduce(0) do |index, entity| + indices = entity[:indices] + result << text[index...indices.first] + result << yield(entity) + indices.last + end + + result << text[last_index..-1] + + Sanitize.fragment(result, Sanitize::Config::MASTODON_OUTGOING) + end + + def format_markdown(html) + html = markdown_formatter.render(html) + html.delete("\r").delete("\n") + end + + def markdown_formatter + extensions = { + autolink: true, + no_intra_emphasis: true, + fenced_code_blocks: true, + disable_indented_code_blocks: true, + strikethrough: true, + lax_spacing: true, + space_after_headers: true, + superscript: true, + underline: true, + highlight: true, + footnotes: false, + } + + renderer = HTMLRenderer.new({ + filter_html: false, + escape_html: false, + no_images: true, + no_styles: true, + safe_links_only: true, + hard_wrap: true, + link_attributes: { target: '_blank', rel: 'nofollow noopener' }, + }) do |url| + link_to_url({ url: url }) + end + + Redcarpet::Markdown.new(renderer, extensions) + end +end diff --git a/app/lib/html_aware_formatter.rb b/app/lib/html_aware_formatter.rb index 64edba09b..7a1cd0340 100644 --- a/app/lib/html_aware_formatter.rb +++ b/app/lib/html_aware_formatter.rb @@ -33,6 +33,10 @@ class HtmlAwareFormatter end def linkify - TextFormatter.new(text, options).to_s + if %w(text/markdown text/html).include?(@options[:content_type]) + AdvancedTextFormatter.new(text, options).to_s + else + TextFormatter.new(text, options).to_s + end end end diff --git a/lib/sanitize_ext/sanitize_config.rb b/lib/sanitize_ext/sanitize_config.rb index ecaec2f84..935e1f4f6 100644 --- a/lib/sanitize_ext/sanitize_config.rb +++ b/lib/sanitize_ext/sanitize_config.rb @@ -55,18 +55,6 @@ class Sanitize end end - LINK_REL_TRANSFORMER = lambda do |env| - return unless env[:node_name] == 'a' and env[:node]['href'] - - node = env[:node] - - rel = (node['rel'] || '').split(' ') & ['tag'] - unless env[:config][:outgoing] && TagManager.instance.local_url?(node['href']) - rel += ['nofollow', 'noopener', 'noreferrer'] - end - node['rel'] = rel.join(' ') - end - UNSUPPORTED_HREF_TRANSFORMER = lambda do |env| return unless env[:node_name] == 'a' @@ -97,6 +85,7 @@ class Sanitize add_attributes: { 'a' => { + 'rel' => 'nofollow noopener noreferrer', 'target' => '_blank', }, }, @@ -110,7 +99,6 @@ class Sanitize CLASS_WHITELIST_TRANSFORMER, IMG_TAG_TRANSFORMER, UNSUPPORTED_HREF_TRANSFORMER, - LINK_REL_TRANSFORMER, ] ) @@ -135,5 +123,48 @@ class Sanitize 'source' => { 'src' => HTTP_PROTOCOLS } ) ) + + LINK_REL_TRANSFORMER = lambda do |env| + return unless env[:node_name] == 'a' && env[:node]['href'] + + node = env[:node] + + rel = (node['rel'] || '').split(' ') & ['tag'] + rel += ['nofollow', 'noopener', 'noreferrer'] unless TagManager.instance.local_url?(node['href']) + + if rel.empty? + node['rel']&.delete + else + node['rel'] = rel.join(' ') + end + end + + LINK_TARGET_TRANSFORMER = lambda do |env| + return unless env[:node_name] == 'a' && env[:node]['href'] + + node = env[:node] + if node['target'] != '_blank' && TagManager.instance.local_url?(node['href']) + node['target']&.delete + else + node['target'] = '_blank' + end + end + + MASTODON_OUTGOING ||= freeze_config MASTODON_STRICT.merge( + attributes: merge( + MASTODON_STRICT[:attributes], + 'a' => %w(href rel class title target) + ), + + add_attributes: {}, + + transformers: [ + CLASS_WHITELIST_TRANSFORMER, + IMG_TAG_TRANSFORMER, + UNSUPPORTED_HREF_TRANSFORMER, + LINK_REL_TRANSFORMER, + LINK_TARGET_TRANSFORMER, + ] + ) end end diff --git a/spec/lib/advanced_text_formatter_spec.rb b/spec/lib/advanced_text_formatter_spec.rb new file mode 100644 index 000000000..c097b86e1 --- /dev/null +++ b/spec/lib/advanced_text_formatter_spec.rb @@ -0,0 +1,274 @@ +require 'rails_helper' + +RSpec.describe AdvancedTextFormatter do + describe '#to_s' do + let(:preloaded_accounts) { nil } + let(:content_type) { 'text/markdown' } + + subject { described_class.new(text, preloaded_accounts: preloaded_accounts, content_type: content_type).to_s } + + context 'given a markdown source' do + let(:content_type) { 'text/markdown' } + + context 'given text containing plain text' do + let(:text) { 'text' } + + it 'paragraphizes the text' do + is_expected.to eq '

text

' + end + end + + context 'given text containing line feeds' do + let(:text) { "line\nfeed" } + + it 'removes line feeds' do + is_expected.not_to include "\n" + end + end + + context 'given some inline code using backticks' do + let(:text) { 'test `foo` bar' } + + it 'formats code using ' do + is_expected.to include 'test foo bar' + end + end + + context 'given some quote' do + let(:text) { "> foo\n\nbar" } + + it 'formats code using ' do + is_expected.to include '

foo

' + end + end + + context 'given text containing linkable mentions' do + let(:preloaded_accounts) { [Fabricate(:account, username: 'alice')] } + let(:text) { '@alice' } + + it 'creates a mention link' do + is_expected.to include '@alice' + end + end + + context 'given text containing unlinkable mentions' do + let(:preloaded_accounts) { [] } + let(:text) { '@alice' } + + it 'does not create a mention link' do + is_expected.to include '@alice' + end + end + + context 'given a stand-alone medium URL' do + let(:text) { 'https://hackernoon.com/the-power-to-build-communities-a-response-to-mark-zuckerberg-3f2cac9148a4' } + + it 'matches the full URL' do + is_expected.to include 'href="https://hackernoon.com/the-power-to-build-communities-a-response-to-mark-zuckerberg-3f2cac9148a4"' + end + end + + context 'given a stand-alone google URL' do + let(:text) { 'http://google.com' } + + it 'matches the full URL' do + is_expected.to include 'href="http://google.com"' + end + end + + context 'given a stand-alone URL with a newer TLD' do + let(:text) { 'http://example.gay' } + + it 'matches the full URL' do + is_expected.to include 'href="http://example.gay"' + end + end + + context 'given a stand-alone IDN URL' do + let(:text) { 'https://nic.みんな/' } + + it 'matches the full URL' do + is_expected.to include 'href="https://nic.みんな/"' + end + + it 'has display URL' do + is_expected.to include 'nic.みんな/' + end + end + + context 'given a URL with a trailing period' do + let(:text) { 'http://www.mcmansionhell.com/post/156408871451/50-states-of-mcmansion-hell-scottsdale-arizona. ' } + + it 'matches the full URL but not the period' do + is_expected.to include 'href="http://www.mcmansionhell.com/post/156408871451/50-states-of-mcmansion-hell-scottsdale-arizona"' + end + end + + context 'given a URL enclosed with parentheses' do + let(:text) { '(http://google.com/)' } + + it 'matches the full URL but not the parentheses' do + is_expected.to include 'href="http://google.com/"' + end + end + + context 'given a URL with a trailing exclamation point' do + let(:text) { 'http://www.google.com!' } + + it 'matches the full URL but not the exclamation point' do + is_expected.to include 'href="http://www.google.com"' + end + end + + context 'given a URL with a trailing single quote' do + let(:text) { "http://www.google.com'" } + + it 'matches the full URL but not the single quote' do + is_expected.to include 'href="http://www.google.com"' + end + end + end + + context 'given a URL with a trailing angle bracket' do + let(:text) { 'http://www.google.com>' } + + it 'matches the full URL but not the angle bracket' do + is_expected.to include 'href="http://www.google.com"' + end + end + + context 'given a URL with a query string' do + context 'with escaped unicode character' do + let(:text) { 'https://www.ruby-toolbox.com/search?utf8=%E2%9C%93&q=autolink' } + + it 'matches the full URL' do + is_expected.to include 'href="https://www.ruby-toolbox.com/search?utf8=%E2%9C%93&q=autolink"' + end + end + + context 'with unicode character' do + let(:text) { 'https://www.ruby-toolbox.com/search?utf8=✓&q=autolink' } + + it 'matches the full URL' do + is_expected.to include 'href="https://www.ruby-toolbox.com/search?utf8=✓&q=autolink"' + end + end + + context 'with unicode character at the end' do + let(:text) { 'https://www.ruby-toolbox.com/search?utf8=✓' } + + it 'matches the full URL' do + is_expected.to include 'href="https://www.ruby-toolbox.com/search?utf8=✓"' + end + end + + context 'with escaped and not escaped unicode characters' do + let(:text) { 'https://www.ruby-toolbox.com/search?utf8=%E2%9C%93&utf81=✓&q=autolink' } + + it 'preserves escaped unicode characters' do + is_expected.to include 'href="https://www.ruby-toolbox.com/search?utf8=%E2%9C%93&utf81=✓&q=autolink"' + end + end + + context 'given a URL with parentheses in it' do + let(:text) { 'https://en.wikipedia.org/wiki/Diaspora_(software)' } + + it 'matches the full URL' do + is_expected.to include 'href="https://en.wikipedia.org/wiki/Diaspora_(software)"' + end + end + + context 'given a URL in quotation marks' do + let(:text) { '"https://example.com/"' } + + it 'does not match the quotation marks' do + is_expected.to include 'href="https://example.com/"' + end + end + + context 'given a URL in angle brackets' do + let(:text) { '' } + + it 'does not match the angle brackets' do + is_expected.to include 'href="https://example.com/"' + end + end + + context 'given a URL containing unsafe code (XSS attack, invisible part)' do + let(:text) { %q{http://example.com/blahblahblahblah/a} } + + it 'does not include the HTML in the URL' do + is_expected.to include '"http://example.com/blahblahblahblah/a"' + end + + it 'does not include a script tag' do + is_expected.to_not include '' } + + it 'does not include a script tag' do + is_expected.to_not include '