Merge branch 'osw-multi-line-suggestions-parsing' into 'master'

Prepare multi-line suggestions for rendering in Markdown

See merge request gitlab-org/gitlab-ce!26107

app/models/diff_note.rb

@@ -88,7 +88,7 @@ class DiffNote < Note
   end
 
   def banzai_render_context(field)
-    super.merge(suggestions_filter_enabled: supports_suggestion?)
+    super.merge(project: project, suggestions_filter_enabled: supports_suggestion?)
   end
 
   private

changelogs/unreleased/osw-multi-line-suggestions-parsing.yml

@@ -0,0 +1,5 @@
+---
+title: Prepare multi-line suggestions for rendering in Markdown
+merge_request: 26107
+author:
+type: other

lib/banzai/filter/output_safety.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+module Banzai
+  module Filter
+    module OutputSafety
+      def escape_once(html)
+        html.html_safe? ? html : ERB::Util.html_escape_once(html)
+      end
+    end
+  end
+end

lib/banzai/filter/reference_filter.rb

@@ -12,6 +12,7 @@ module Banzai
     #   :only_path          - Generate path-only links.
     class ReferenceFilter < HTML::Pipeline::Filter
       include RequestStoreReferenceCache
+      include OutputSafety
 
       class << self
         attr_accessor :reference_type
@@ -43,10 +44,6 @@ module Banzai
         end.join(' ')
       end
 
-      def escape_once(html)
-        html.html_safe? ? html : ERB::Util.html_escape_once(html)
-      end
-
       def ignore_ancestor_query
         @ignore_ancestor_query ||= begin
           parents = %w(pre code a style)

lib/banzai/filter/suggestion_filter.rb

@@ -6,11 +6,15 @@ module Banzai
     class SuggestionFilter < HTML::Pipeline::Filter
       # Class used for tagging elements that should be rendered
       TAG_CLASS = 'js-render-suggestion'.freeze
+      SUGGESTION_REGEX = Gitlab::Diff::SuggestionsParser::SUGGESTION_CONTEXT
 
       def call
         return doc unless suggestions_filter_enabled?
 
         doc.search('pre.suggestion > code').each do |node|
+          # TODO: Remove once multi-line suggestions FF get removed (#59178).
+          remove_multi_line_params(node.parent)
+
           node.add_class(TAG_CLASS)
         end
 
@@ -20,6 +24,20 @@ module Banzai
       def suggestions_filter_enabled?
         context[:suggestions_filter_enabled]
       end
+
+      private
+
+      def project
+        context[:project]
+      end
+
+      def remove_multi_line_params(node)
+        return if Feature.enabled?(:multi_line_suggestions, project)
+
+        if node[SyntaxHighlightFilter::LANG_PARAMS_ATTR]&.match?(SUGGESTION_REGEX)
+          node.remove_attribute(SyntaxHighlightFilter::LANG_PARAMS_ATTR)
+        end
+      end
     end
   end
 end

lib/banzai/filter/syntax_highlight_filter.rb

@@ -8,6 +8,11 @@ module Banzai
     # HTML Filter to highlight fenced code blocks
     #
     class SyntaxHighlightFilter < HTML::Pipeline::Filter
+      include OutputSafety
+
+      PARAMS_DELIMITER = ':'.freeze
+      LANG_PARAMS_ATTR = 'data-lang-params'.freeze
+
       def call
         doc.search('pre > code').each do |node|
           highlight_node(node)
@@ -18,7 +23,7 @@ module Banzai
 
       def highlight_node(node)
         css_classes = +'code highlight js-syntax-highlight'
-        lang = node.attr('lang')
+        lang, lang_params = parse_lang_params(node.attr('lang'))
         retried = false
 
         if use_rouge?(lang)
@@ -46,7 +51,10 @@ module Banzai
           retry
         end
 
-        highlighted = %(<pre class="#{css_classes}" lang="#{language}" v-pre="true"><code>#{code}</code></pre>)
+        highlighted = %(<pre class="#{css_classes}"
+                             lang="#{language}"
+                             #{lang_params}
+                             v-pre="true"><code>#{code}</code></pre>)
 
         # Extracted to a method to measure it
         replace_parent_pre_element(node, highlighted)
@@ -54,6 +62,15 @@ module Banzai
 
       private
 
+      def parse_lang_params(language)
+        return unless language
+
+        lang, params = language.split(PARAMS_DELIMITER, 2)
+        formatted_params = %(#{LANG_PARAMS_ATTR}="#{escape_once(params)}") if params
+
+        [lang, formatted_params]
+      end
+
       # Separate method so it can be instrumented.
       def lex(lexer, code)
         lexer.lex(code)

lib/gitlab/diff/suggestions_parser.rb

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Diff
+    class SuggestionsParser
+      # Matches for instance "-1", "+1" or "-1+2".
+      SUGGESTION_CONTEXT = /^(\-(?<above>\d+))?(\+(?<below>\d+))?$/.freeze
+    end
+  end
+end

spec/lib/banzai/filter/output_safety_spec.rb

@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Banzai::Filter::OutputSafety do
+  subject do
+    Class.new do
+      include Banzai::Filter::OutputSafety
+    end.new
+  end
+
+  let(:content) { '<pre><code>foo</code></pre>' }
+
+  context 'when given HTML is safe' do
+    let(:html) { content.html_safe }
+
+    it 'returns safe HTML' do
+      expect(subject.escape_once(html)).to eq(html)
+    end
+  end
+
+  context 'when given HTML is not safe' do
+    let(:html) { content }
+
+    it 'returns escaped HTML' do
+      expect(subject.escape_once(html)).to eq(ERB::Util.html_escape_once(html))
+    end
+  end
+end

spec/lib/banzai/filter/suggestion_filter_spec.rb

@@ -5,7 +5,7 @@ require 'spec_helper'
 describe Banzai::Filter::SuggestionFilter do
   include FilterSpecHelper
 
-  let(:input) { "<pre class='code highlight js-syntax-highlight suggestion'><code>foo\n</code></pre>" }
+  let(:input) { %(<pre class="code highlight js-syntax-highlight suggestion"><code>foo\n</code></pre>) }
   let(:default_context) do
     { suggestions_filter_enabled: true }
   end
@@ -23,4 +23,35 @@ describe Banzai::Filter::SuggestionFilter do
 
     expect(result[:class]).to be_nil
   end
+
+  context 'multi-line suggestions' do
+    let(:data_attr) { Banzai::Filter::SyntaxHighlightFilter::LANG_PARAMS_ATTR }
+    let(:input) { %(<pre class="code highlight js-syntax-highlight suggestion" #{data_attr}="-3+2"><code>foo\n</code></pre>) }
+
+    context 'feature disabled' do
+      before do
+        stub_feature_flags(multi_line_suggestions: false)
+      end
+
+      it 'removes data-lang-params if it matches a multi-line suggestion param' do
+        doc = filter(input, default_context)
+        pre = doc.css('pre').first
+
+        expect(pre[data_attr]).to be_nil
+      end
+    end
+
+    context 'feature enabled' do
+      before do
+        stub_feature_flags(multi_line_suggestions: true)
+      end
+
+      it 'keeps data-lang-params' do
+        doc = filter(input, default_context)
+        pre = doc.css('pre').first
+
+        expect(pre[data_attr]).to eq('-3+2')
+      end
+    end
+  end
 end

spec/lib/banzai/filter/syntax_highlight_filter_spec.rb

@@ -45,7 +45,10 @@ describe Banzai::Filter::SyntaxHighlightFilter do
   end
 
   context "languages that should be passed through" do
-    %w(math mermaid plantuml).each do |lang|
+    let(:delimiter) { described_class::PARAMS_DELIMITER }
+    let(:data_attr) { described_class::LANG_PARAMS_ATTR }
+
+    %w(math mermaid plantuml suggestion).each do |lang|
       context "when #{lang} is specified" do
         it "highlights as plaintext but with the correct language attribute and class" do
           result = filter(%{<pre><code lang="#{lang}">This is a test</code></pre>})
@@ -55,6 +58,33 @@ describe Banzai::Filter::SyntaxHighlightFilter do
 
         include_examples "XSS prevention", lang
       end
+
+      context "when #{lang} has extra params" do
+        let(:lang_params) { 'foo-bar-kux' }
+
+        it "includes data-lang-params tag with extra information" do
+          result = filter(%{<pre><code lang="#{lang}#{delimiter}#{lang_params}">This is a test</code></pre>})
+
+          expect(result.to_html).to eq(%{<pre class="code highlight js-syntax-highlight #{lang}" lang="#{lang}" #{data_attr}="#{lang_params}" v-pre="true"><code><span id="LC1" class="line" lang="#{lang}">This is a test</span></code></pre>})
+        end
+
+        include_examples "XSS prevention", lang
+        include_examples "XSS prevention",
+          "#{lang}#{described_class::PARAMS_DELIMITER}&lt;script&gt;alert(1)&lt;/script&gt;"
+        include_examples "XSS prevention",
+          "#{lang}#{described_class::PARAMS_DELIMITER}<script>alert(1)</script>"
+      end
+    end
+
+    context 'when multiple param delimiters are used' do
+      let(:lang) { 'suggestion' }
+      let(:lang_params) { '-1+10' }
+
+      it "delimits on the first appearence" do
+        result = filter(%{<pre><code lang="#{lang}#{delimiter}#{lang_params}#{delimiter}more-things">This is a test</code></pre>})
+
+        expect(result.to_html).to eq(%{<pre class="code highlight js-syntax-highlight #{lang}" lang="#{lang}" #{data_attr}="#{lang_params}#{delimiter}more-things" v-pre="true"><code><span id="LC1" class="line" lang="#{lang}">This is a test</span></code></pre>})
+      end
     end
   end
 

spec/models/diff_note_spec.rb

@@ -336,6 +336,16 @@ describe DiffNote do
     end
   end
 
+  describe '#banzai_render_context' do
+    let(:note) { create(:diff_note_on_merge_request) }
+
+    it 'includes expected context' do
+      context = note.banzai_render_context(:note)
+
+      expect(context).to include(suggestions_filter_enabled: true, noteable: note.noteable, project: note.project)
+    end
+  end
+
   describe "image diff notes" do
     subject { build(:image_diff_note_on_merge_request, project: project, noteable: merge_request) }