Merge branch '323618-markdown-only-escape-what-required' into 'master'

User mentions not working for usernames ending in `_`

See merge request gitlab-org/gitlab!56039

doc/user/markdown.md

@@ -462,7 +462,8 @@ GitLab Flavored Markdown recognizes the following:
 
 For example, referencing an issue by using `#123` formats the output as a link
 to issue number 123 with text `#123`. Likewise, a link to issue number 123 is
-recognized and formatted with text `#123`.
+recognized and formatted with text `#123`. If you don't want `#123` to link to an issue,
+add a leading backslash `\#123`.
 
 In addition to this, links to some objects are also recognized and formatted. Some examples of these are:
 

lib/banzai/filter/markdown_pre_escape_filter.rb

@@ -24,7 +24,9 @@ module Banzai
     # This filter does the initial surrounding, and MarkdownPostEscapeFilter
     # does the conversion into span tags.
     class MarkdownPreEscapeFilter < HTML::Pipeline::TextFilter
-      ASCII_PUNCTUATION   = %r{([\\][!"#$%&'()*+,-./:;<=>?@\[\\\]^_`{|}~])}.freeze
+      # We just need to target those that are special GitLab references
+      REFERENCE_CHARACTERS = '@#!$&~%^'
+      ASCII_PUNCTUATION    = %r{([\\][#{REFERENCE_CHARACTERS}])}.freeze
       LITERAL_KEYWORD      = 'cmliteral'
 
       def call

spec/lib/banzai/pipeline/full_pipeline_spec.rb

@@ -142,5 +142,12 @@ RSpec.describe Banzai::Pipeline::FullPipeline do
 
       expect(output).to include("<span>#</span>#{issue.iid}")
     end
+
+    it 'converts user reference with escaped underscore because of italics' do
+      markdown = '_@test\__'
+      output = described_class.to_html(markdown, project: project)
+
+      expect(output).to include('<em>@test_</em>')
+    end
   end
 end

spec/lib/banzai/pipeline/plain_markdown_pipeline_spec.rb

@@ -31,11 +31,13 @@ RSpec.describe Banzai::Pipeline::PlainMarkdownPipeline do
       end
     end
 
-    # Test strings taken from https://spec.commonmark.org/0.29/#backslash-escapes
     describe 'CommonMark tests', :aggregate_failures do
-      it 'converts all ASCII punctuation to literals' do
-        markdown = %q(\!\"\#\$\%\&\'\*\+\,\-\.\/\:\;\<\=\>\?\@\[\]\^\_\`\{\|\}\~) + %q[\(\)\\\\]
-        punctuation = %w(! " # $ % &amp; ' * + , - . / : ; &lt; = &gt; ? @ [ \\ ] ^ _ ` { | } ~) + %w[( )]
+      it 'converts all reference punctuation to literals' do
+        reference_chars = Banzai::Filter::MarkdownPreEscapeFilter::REFERENCE_CHARACTERS
+        markdown = reference_chars.split('').map {|char| char.prepend("\\") }.join
+        punctuation = Banzai::Filter::MarkdownPreEscapeFilter::REFERENCE_CHARACTERS.split('')
+        punctuation = punctuation.delete_if {|char| char == '&' }
+        punctuation << '&amp;'
 
         result = described_class.call(markdown, project: project)
         output = result[:output].to_html
@@ -44,57 +46,45 @@ RSpec.describe Banzai::Pipeline::PlainMarkdownPipeline do
         expect(result[:escaped_literals]).to be_truthy
       end
 
-      it 'does not convert other characters to literals' do
-        markdown = %q(\→\A\a\ \3\φ\«)
-        expected = '\→\A\a\ \3\φ\«'
-
-        result = correct_html_included(markdown, expected)
-        expect(result[:escaped_literals]).to be_falsey
-      end
+      it 'ensure we handle all the GitLab reference characters' do
+        reference_chars = ObjectSpace.each_object(Class).map do |klass|
+          next unless klass.included_modules.include?(Referable)
+          next unless klass.respond_to?(:reference_prefix)
+          next unless klass.reference_prefix.length == 1
 
-      describe 'escaped characters are treated as regular characters and do not have their usual Markdown meanings' do
-        where(:markdown, :expected) do
-          %q(\*not emphasized*)              | %q(<span>*</span>not emphasized*)
-          %q(\<br/> not a tag)               | %q(<span>&lt;</span>br/&gt; not a tag)
-          %q!\[not a link](/foo)!            | %q!<span>[</span>not a link](/foo)!
-          %q(\`not code`)                    | %q(<span>`</span>not code`)
-          %q(1\. not a list)                 | %q(1<span>.</span> not a list)
-          %q(\# not a heading)               | %q(<span>#</span> not a heading)
-          %q(\[foo]: /url "not a reference") | %q(<span>[</span>foo]: /url "not a reference")
-          %q(\&ouml; not a character entity) | %q(<span>&amp;</span>ouml; not a character entity)
-        end
+          klass.reference_prefix
+        end.compact
 
-        with_them do
-          it 'keeps them as literals' do
-            correct_html_included(markdown, expected)
-          end
+        reference_chars.all? do |char|
+          Banzai::Filter::MarkdownPreEscapeFilter::REFERENCE_CHARACTERS.include?(char)
         end
       end
 
-      it 'backslash is itself escaped, the following character is not' do
-        markdown = %q(\\\\*emphasis*)
-        expected = %q(<span>\</span><em>emphasis</em>)
+      it 'does not convert non-reference punctuation to spans' do
+        markdown = %q(\"\'\*\+\,\-\.\/\:\;\<\=\>\?\[\]\_\`\{\|\}) + %q[\(\)\\\\]
 
-        correct_html_included(markdown, expected)
+        result = described_class.call(markdown, project: project)
+        output = result[:output].to_html
+
+        expect(output).not_to include('<span>')
+        expect(result[:escaped_literals]).to be_falsey
       end
 
-      it 'backslash at the end of the line is a hard line break' do
-        markdown = <<~MARKDOWN
-          foo\\
-          bar
-        MARKDOWN
-        expected = "foo<br>\nbar"
+      it 'does not convert other characters to literals' do
+        markdown = %q(\→\A\a\ \3\φ\«)
+        expected = '\→\A\a\ \3\φ\«'
 
-        correct_html_included(markdown, expected)
+        result = correct_html_included(markdown, expected)
+        expect(result[:escaped_literals]).to be_falsey
       end
 
       describe 'backslash escapes do not work in code blocks, code spans, autolinks, or raw HTML' do
         where(:markdown, :expected) do
-          %q(`` \[\` ``)       | %q(<code>\[\`</code>)
-          %q(    \[\])         | %Q(<code>\\[\\]\n</code>)
-          %Q(~~~\n\\[\\]\n~~~) | %Q(<code>\\[\\]\n</code>)
-          %q(<http://example.com?find=\*>) | %q(<a href="http://example.com?find=%5C*">http://example.com?find=\*</a>)
-          %q[<a href="/bar\/)">]           | %q[<a href="/bar%5C/)">]
+          %q(`` \@\! ``)       | %q(<code>\@\!</code>)
+          %q(    \@\!)         | %Q(<code>\\@\\!\n</code>)
+          %Q(~~~\n\\@\\!\n~~~) | %Q(<code>\\@\\!\n</code>)
+          %q(<http://example.com?find=\@>) | %q(<a href="http://example.com?find=%5C@">http://example.com?find=\@</a>)
+          %q[<a href="/bar\@)">]           | %q[<a href="/bar%5C@)">]
         end
 
         with_them do
@@ -104,9 +94,9 @@ RSpec.describe Banzai::Pipeline::PlainMarkdownPipeline do
 
       describe 'work in all other contexts, including URLs and link titles, link references, and info strings in fenced code blocks' do
         where(:markdown, :expected) do
-          %q![foo](/bar\* "ti\*tle")!            | %q(<a href="/bar*" title="ti*tle">foo</a>)
-          %Q![foo]\n\n[foo]: /bar\\* "ti\\*tle"! | %q(<a href="/bar*" title="ti*tle">foo</a>)
-          %Q(``` foo\\+bar\nfoo\n```)            | %Q(<code lang="foo+bar">foo\n</code>)
+          %q![foo](/bar\@ "\@title")!            | %q(<a href="/bar@" title="@title">foo</a>)
+          %Q![foo]\n\n[foo]: /bar\\@ "\\@title"! | %q(<a href="/bar@" title="@title">foo</a>)
+          %Q(``` foo\\@bar\nfoo\n```)            | %Q(<code lang="foo@bar">foo\n</code>)
         end
 
         with_them do