#{encode(code).gsub("\n", "
")}|
|<\/p>)+/) { |match| "#{match}\n" } - strip_tags(text) - end - - def simplified_format(account, **options) - return '' if account.note.blank? - - html = account.local? ? linkify(account.note) : reformat(account.note) - html = encode_custom_emojis(html, account.emojis, options[:autoplay]) if options[:custom_emojify] - html.html_safe # rubocop:disable Rails/OutputSafety - end - - def sanitize(html, config) - Sanitize.fragment(html, config) - end - - def format_spoiler(status, **options) - html = encode(status.spoiler_text) - html = encode_custom_emojis(html, status.emojis, options[:autoplay]) - html.html_safe # rubocop:disable Rails/OutputSafety - end - - def format_poll_option(status, option, **options) - html = encode(option.title) - html = encode_custom_emojis(html, status.emojis, options[:autoplay]) - html.html_safe # rubocop:disable Rails/OutputSafety - end - - def format_display_name(account, **options) - html = encode(account.display_name.presence || account.username) - html = encode_custom_emojis(html, account.emojis, options[:autoplay]) if options[:custom_emojify] - html.html_safe # rubocop:disable Rails/OutputSafety - end - - def format_field(account, str, **options) - html = account.local? ? encode_and_link_urls(str, me: true, with_domain: true) : reformat(str) - html = encode_custom_emojis(html, account.emojis, options[:autoplay]) if options[:custom_emojify] - html.html_safe # rubocop:disable Rails/OutputSafety - end - - def linkify(text) - html = encode_and_link_urls(text) - html = simple_format(html, {}, sanitize: false) - html = html.delete("\n") - - html.html_safe # rubocop:disable Rails/OutputSafety - end - - def link_url(url) - "#{link_html(url)}" - end - - private - - 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' }, - }) - - Redcarpet::Markdown.new(renderer, extensions) - end - - def html_entities - @html_entities ||= HTMLEntities.new - end - - def encode(html) - html_entities.encode(html) - end - - def encode_and_link_urls(html, accounts = nil, options = {}) - if accounts.is_a?(Hash) - options = accounts - accounts = nil - end - - entities = options[:keep_html] ? html_friendly_extractor(html) : utf8_friendly_extractor(html, extract_url_without_protocol: false) - - rewrite(html.dup, entities, options[:keep_html]) do |entity| - if entity[:url] - link_to_url(entity, options) - elsif entity[:hashtag] - link_to_hashtag(entity) - elsif entity[:screen_name] - link_to_mention(entity, accounts, options) - end - end - end - - def count_tag_nesting(tag) - if tag[1] == '/' then -1 - elsif tag[-2] == '/' then 0 - else 1 - end - end - - # rubocop:disable Metrics/BlockNesting - def encode_custom_emojis(html, emojis, animate = false) - return html if emojis.empty? - - emoji_map = emojis.each_with_object({}) { |e, h| h[e.shortcode] = [full_asset_url(e.image.url), full_asset_url(e.image.url(:static))] } - - i = -1 - tag_open_index = nil - inside_shortname = false - shortname_start_index = -1 - invisible_depth = 0 - - while i + 1 < html.size - i += 1 - - if invisible_depth.zero? && inside_shortname && html[i] == ':' - shortcode = html[shortname_start_index + 1..i - 1] - emoji = emoji_map[shortcode] - - if emoji - original_url, static_url = emoji - replacement = begin - if animate - image_tag(original_url, draggable: false, class: 'emojione', alt: ":#{shortcode}:", title: ":#{shortcode}:") - else - image_tag(original_url, draggable: false, class: 'emojione custom-emoji', alt: ":#{shortcode}:", title: ":#{shortcode}:", data: { original: original_url, static: static_url }) - end - end - before_html = shortname_start_index.positive? ? html[0..shortname_start_index - 1] : '' - html = before_html + replacement + html[i + 1..-1] - i += replacement.size - (shortcode.size + 2) - 1 - else - i -= 1 - end - - inside_shortname = false - 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 == '' - invisible_depth = 1 - end - elsif html[i] == '<' - tag_open_index = i - inside_shortname = false - elsif !tag_open_index && html[i] == ':' - inside_shortname = true - shortname_start_index = i - end - end - - html - end - # rubocop:enable Metrics/BlockNesting - - def rewrite(text, entities, keep_html = false) - text = text.to_s - - # Sort by start index - entities = entities.sort_by do |entity| - indices = entity.respond_to?(:indices) ? entity.indices : entity[:indices] - indices.first - end - - result = [] - - last_index = entities.reduce(0) do |index, entity| - indices = entity.respond_to?(:indices) ? entity.indices : entity[:indices] - result << (keep_html ? text[index...indices.first] : encode(text[index...indices.first])) - result << yield(entity) - indices.last - end - - result << (keep_html ? text[last_index..-1] : encode(text[last_index..-1])) - - result.flatten.join - end - - def utf8_friendly_extractor(text, options = {}) - # Note: I couldn't obtain list_slug with @user/list-name format - # for mention so this requires additional check - special = Extractor.extract_urls_with_indices(text, options) - standard = Extractor.extract_entities_with_indices(text, options) - extra = Extractor.extract_extra_uris_with_indices(text, options) - - Extractor.remove_overlapping_entities(special + standard + extra) - end - - def html_friendly_extractor(html, options = {}) - gaps = [] - total_offset = 0 - - escaped = html.gsub(/<[^>]*>|&#[0-9]+;/) do |match| - total_offset += match.length - 1 - end_offset = Regexp.last_match.end(0) - gaps << [end_offset - total_offset, total_offset] - "\u200b" - end - - entities = Extractor.extract_hashtags_with_indices(escaped, :check_url_overlap => false) + - Extractor.extract_mentions_or_lists_with_indices(escaped) - Extractor.remove_overlapping_entities(entities).map do |extract| - pos = extract[:indices].first - offset_idx = gaps.rindex { |gap| gap.first <= pos } - offset = offset_idx.nil? ? 0 : gaps[offset_idx].last - next extract.merge( - :indices => [extract[:indices].first + offset, extract[:indices].last + offset] - ) - end - end - - def link_to_url(entity, options = {}) - url = Addressable::URI.parse(entity[:url]) - html_attrs = { target: '_blank', rel: 'nofollow noopener noreferrer' } - - html_attrs[:rel] = "me #{html_attrs[:rel]}" if options[:me] - - Twitter::TwitterText::Autolink.send(:link_to_text, entity, link_html(entity[:url]), url, html_attrs) - rescue Addressable::URI::InvalidURIError, IDN::Idna::IdnaError - encode(entity[:url]) - end - - def link_to_mention(entity, linkable_accounts, options = {}) - acct = entity[:screen_name] - - return link_to_account(acct, options) unless linkable_accounts - - same_username_hits = 0 - account = nil - username, domain = acct.split('@') - domain = nil if TagManager.instance.local_domain?(domain) - - linkable_accounts.each do |item| - same_username = item.username.casecmp(username).zero? - same_domain = item.domain.nil? ? domain.nil? : item.domain.casecmp(domain)&.zero? - - if same_username && !same_domain - same_username_hits += 1 - elsif same_username && same_domain - account = item - end - end - - account ? mention_html(account, with_domain: same_username_hits.positive? || options[:with_domain]) : "@#{encode(acct)}" - end - - def link_to_account(acct, options = {}) - username, domain = acct.split('@') - - domain = nil if TagManager.instance.local_domain?(domain) - account = EntityCache.instance.mention(username, domain) - - account ? mention_html(account, with_domain: options[:with_domain]) : "@#{encode(acct)}" - end - - def link_to_hashtag(entity) - hashtag_html(entity[:hashtag]) - end - - def link_html(url) - url = Addressable::URI.parse(url).to_s - prefix = url.match(/\A(https?:\/\/(www\.)?|xmpp:)/).to_s - text = url[prefix.length, 30] - suffix = url[prefix.length + 30..-1] - cutoff = url[prefix.length..-1].length > 30 - - "#{encode(prefix)}#{encode(text)}#{encode(suffix)}" - end - - def hashtag_html(tag) - "##{encode(tag)}" - end - - def mention_html(account, with_domain: false) - "@#{encode(with_domain ? account.pretty_acct : account.username)}" - end -end diff --git a/app/lib/html_aware_formatter.rb b/app/lib/html_aware_formatter.rb new file mode 100644 index 0000000000..7a1cd03409 --- /dev/null +++ b/app/lib/html_aware_formatter.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +class HtmlAwareFormatter + attr_reader :text, :local, :options + + alias local? local + + # @param [String] text + # @param [Boolean] local + # @param [Hash] options + def initialize(text, local, options = {}) + @text = text + @local = local + @options = options + end + + def to_s + return ''.html_safe if text.blank? + + if local? + linkify + else + reformat.html_safe # rubocop:disable Rails/OutputSafety + end + rescue ArgumentError + ''.html_safe + end + + private + + def reformat + Sanitize.fragment(text, Sanitize::Config::MASTODON_STRICT) + end + + def linkify + 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/app/lib/link_details_extractor.rb b/app/lib/link_details_extractor.rb index fabbd244df..b0c4e4f425 100644 --- a/app/lib/link_details_extractor.rb +++ b/app/lib/link_details_extractor.rb @@ -208,7 +208,7 @@ class LinkDetailsExtractor end def valid_url_or_nil(str, same_origin_only: false) - return if str.blank? + return if str.blank? || str == 'null' url = @original_url + Addressable::URI.parse(str) diff --git a/app/lib/plain_text_formatter.rb b/app/lib/plain_text_formatter.rb new file mode 100644 index 0000000000..08aa296964 --- /dev/null +++ b/app/lib/plain_text_formatter.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +class PlainTextFormatter + include ActionView::Helpers::TextHelper + + NEWLINE_TAGS_RE = /(
|
|<\/p>)+/.freeze + + attr_reader :text, :local + + alias local? local + + def initialize(text, local) + @text = text + @local = local + end + + def to_s + if local? + text + else + strip_tags(insert_newlines).chomp + end + end + + private + + def insert_newlines + text.gsub(NEWLINE_TAGS_RE) { |match| "#{match}\n" } + end +end diff --git a/app/lib/rss/serializer.rb b/app/lib/rss/serializer.rb index 7e3ed1f178..d44e94221b 100644 --- a/app/lib/rss/serializer.rb +++ b/app/lib/rss/serializer.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class RSS::Serializer + include FormattingHelper + private def render_statuses(builder, statuses) @@ -9,7 +11,7 @@ class RSS::Serializer item.title(status_title(status)) .link(ActivityPub::TagManager.instance.url_for(status)) .pub_date(status.created_at) - .description(status.spoiler_text.presence || Formatter.instance.format(status, inline_poll_options: true).to_str) + .description(status_description(status)) status.ordered_media_attachments.each do |media| item.enclosure(full_asset_url(media.file.url(:original, false)), media.file.content_type, media.file.size) @@ -19,9 +21,8 @@ class RSS::Serializer end def status_title(status) - return "#{status.account.acct} deleted status" if status.destroyed? - preview = status.proper.spoiler_text.presence || status.proper.text + if preview.length > 30 || preview[0, 30].include?("\n") preview = preview[0, 30] preview = preview[0, preview.index("\n").presence || 30] + '…' @@ -35,4 +36,20 @@ class RSS::Serializer "#{status.account.acct}: #{preview}" end end + + def status_description(status) + if status.proper.spoiler_text? + status.proper.spoiler_text + else + html = status_content_format(status.proper).to_str + after_html = '' + + if status.proper.preloadable_poll + poll_options_html = status.proper.preloadable_poll.options.map { |o| "[ ] #{o}" }.join('
') + after_html = "
#{poll_options_html}
" + end + + "#{html}#{after_html}" + end + end end diff --git a/app/lib/text_formatter.rb b/app/lib/text_formatter.rb new file mode 100644 index 0000000000..48e2fc2338 --- /dev/null +++ b/app/lib/text_formatter.rb @@ -0,0 +1,158 @@ +# frozen_string_literal: true + +class TextFormatter + include ActionView::Helpers::TextHelper + include ERB::Util + include RoutingHelper + + URL_PREFIX_REGEX = /\A(https?:\/\/(www\.)?|xmpp:)/.freeze + + DEFAULT_REL = %w(nofollow noopener noreferrer).freeze + + DEFAULT_OPTIONS = { + multiline: true, + }.freeze + + attr_reader :text, :options + + # @param [String] text + # @param [Hash] options + # @option options [Boolean] :multiline + # @option options [Boolean] :with_domains + # @option options [Boolean] :with_rel_me + # @option options [Array<script>alert("Hello")</script>
' - end - end - - context 'given text containing HTML (XSS attack)' do - let(:text) { %q{<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 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 a 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 - - describe '#format_spoiler' do - subject { Formatter.instance.format_spoiler(status) } - - context 'given a post containing plain text' do - let(:status) { Fabricate(:status, text: 'text', spoiler_text: 'Secret!', uri: nil) } - - it 'Returns the spoiler text' do - is_expected.to eq 'Secret!' - end - end - - context 'given a post with an emoji shortcode at the start' do - let!(:emoji) { Fabricate(:custom_emoji) } - let(:status) { Fabricate(:status, text: 'text', spoiler_text: ':coolcat: Secret!', uri: nil) } - let(:text) { ':coolcat: Beep boop' } - - it 'converts the shortcode to an image tag' do - is_expected.to match(/text
' - end - end - - context 'given a post containing line feeds' do - let(:status) { Fabricate(:status, text: "line\nfeed", uri: nil) } - - it 'removes line feeds' do - is_expected.not_to include "\n" - end - end - - context 'given a post containing linkable mentions' do - let(:status) { Fabricate(:status, mentions: [ Fabricate(:mention, account: local_account) ], text: '@alice') } - - it 'creates a mention link' do - is_expected.to include '@alice' - 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 - let(:content_type) { 'text/plain' } - - subject do - status = Fabricate(:status, text: text, content_type: content_type, uri: nil) - Formatter.instance.format(status) - end - - context 'given an invalid URL (invalid port)' do - let(:text) { 'https://foo.bar:X/' } - let(:content_type) { 'text/markdown' } - - it 'outputs the raw URL' do - is_expected.to eq 'https://foo.bar:X/
' - end - 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: Beep boop
' }
-
- it 'converts the shortcode to an image tag' do
- is_expected.to match(/
Beep :coolcat: boop
:coolcat::coolcat:<\/p>/) - end - end - - context 'given a post with an emoji shortcode at the end' do - let(:text) { '
Beep boop
:coolcat:
a text by a nerd who uses an HTML tag in text
', content_type: content_type, uri: nil) } - let(:content_type) { 'text/plain' } - - 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 '' - 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 '' - 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(/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(/
Beep :coolcat: boop
:coolcat::coolcat:<\/p>/) - end - end - - context 'given a post with an emoji shortcode at the end' do - let(:text) { '
Beep boop
:coolcat:
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/link_details_extractor_spec.rb b/spec/lib/link_details_extractor_spec.rb index 84bb4579ca..7ea867c614 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 diff --git a/spec/lib/plain_text_formatter_spec.rb b/spec/lib/plain_text_formatter_spec.rb new file mode 100644 index 0000000000..c3d0ee6301 --- /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/rss/serializer_spec.rb b/spec/lib/rss/serializer_spec.rb index 0364d13deb..1da45d302a 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/lib/sanitize_config_spec.rb b/spec/lib/sanitize_config_spec.rb index 8bcffb2e5a..dc6418e5bd 100644 --- a/spec/lib/sanitize_config_spec.rb +++ b/spec/lib/sanitize_config_spec.rb @@ -41,18 +41,8 @@ describe Sanitize::Config do end end - describe '::MASTODON_STRICT' do - subject { Sanitize::Config::MASTODON_STRICT } - - it_behaves_like 'common HTML sanitization' - - it 'keeps a with href and rel tag' do - expect(Sanitize.fragment('Test', subject)).to eq 'Test' - end - end - - describe '::MASTODON_STRICT with outgoing toots' do - subject { Sanitize::Config::MASTODON_STRICT.merge(outgoing: true) } + describe '::MASTODON_OUTGOING' do + subject { Sanitize::Config::MASTODON_OUTGOING } around do |example| original_web_domain = Rails.configuration.x.web_domain @@ -62,9 +52,9 @@ describe Sanitize::Config do it_behaves_like 'common HTML sanitization' - it 'keeps a with href and rel tag, not adding to rel if url is local' do + it 'keeps a with href and rel tag, not adding to rel or target if url is local' do Rails.configuration.x.web_domain = 'domain.test' - expect(Sanitize.fragment('Test', subject)).to eq 'Test' + expect(Sanitize.fragment('Test', subject)).to eq 'Test' end end end diff --git a/spec/lib/text_formatter_spec.rb b/spec/lib/text_formatter_spec.rb new file mode 100644 index 0000000000..52a9d24988 --- /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) { '<script>alert("Hello")</script>
' + end + end + + context 'given text containing HTML (XSS attack)' do + let(:text) { %q{<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 diff --git a/spec/models/media_attachment_spec.rb b/spec/models/media_attachment_spec.rb index 7360b23cf0..cbd9a09c55 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 788c7c9d9c..f87adcae13 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/after_block_service_spec.rb b/spec/services/after_block_service_spec.rb index fe5b26b2be..c09425d7c2 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 9c785fc17c..1fbe4d07c2 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 4bb839b8d4..bdec1c67b4 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 83e62ff361..67dd0483b4 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 cf7eb257a0..5d45e4ffdb 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 0593beb6f2..3ac4cc085e 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 diff --git a/spec/services/update_status_service_spec.rb b/spec/services/update_status_service_spec.rb index 78cc89cd46..71a73be5b0 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) }