diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index 805092e527a30d63a1a0bcb18f4da45c0f817e10..87755cf3f3de2f14628785692506a6d5c1db2c83 100644 --- a/app/models/diff_note.rb +++ b/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 diff --git a/changelogs/unreleased/osw-multi-line-suggestions-parsing.yml b/changelogs/unreleased/osw-multi-line-suggestions-parsing.yml new file mode 100644 index 0000000000000000000000000000000000000000..985b01e9254f63714c87574e142e2972cdb371b7 --- /dev/null +++ b/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 diff --git a/lib/banzai/filter/output_safety.rb b/lib/banzai/filter/output_safety.rb new file mode 100644 index 0000000000000000000000000000000000000000..d4ebce5d9c9fa95b5137a30191650c421de6d943 --- /dev/null +++ b/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 diff --git a/lib/banzai/filter/reference_filter.rb b/lib/banzai/filter/reference_filter.rb index 42f9b3a689c00969e3b9948de8cf9f4fb3309e3a..b3ce9200b49c07014a2f5dc72d8468d84fe54f34 100644 --- a/lib/banzai/filter/reference_filter.rb +++ b/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) diff --git a/lib/banzai/filter/suggestion_filter.rb b/lib/banzai/filter/suggestion_filter.rb index 9950db373d81d6f6da3b2d69a12e106dfd12ef1d..848aca10a20c349b8af93fd9625e9b4b45073bbd 100644 --- a/lib/banzai/filter/suggestion_filter.rb +++ b/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 diff --git a/lib/banzai/filter/syntax_highlight_filter.rb b/lib/banzai/filter/syntax_highlight_filter.rb index 9ffde52b5f2f7facf14915a21797679b715b18ec..fe56f9a1e33949da126669f7d3647a6ec01a5cde 100644 --- a/lib/banzai/filter/syntax_highlight_filter.rb +++ b/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 = %(
#{code})
+ highlighted = %(#{code})
# 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)
diff --git a/lib/gitlab/diff/suggestions_parser.rb b/lib/gitlab/diff/suggestions_parser.rb
new file mode 100644
index 0000000000000000000000000000000000000000..043bd9a4bcb455311c3a3835db25549a7e316e08
--- /dev/null
+++ b/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 = /^(\-(?foo' }
+
+ 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
diff --git a/spec/lib/banzai/filter/suggestion_filter_spec.rb b/spec/lib/banzai/filter/suggestion_filter_spec.rb
index b13c90b54bd68025ceea10c40bdffdfc85077769..af6f002fa303113b7203b8ef6c42103bdd6542c8 100644
--- a/spec/lib/banzai/filter/suggestion_filter_spec.rb
+++ b/spec/lib/banzai/filter/suggestion_filter_spec.rb
@@ -5,7 +5,7 @@ require 'spec_helper'
describe Banzai::Filter::SuggestionFilter do
include FilterSpecHelper
- let(:input) { "foo\n" }
+ let(:input) { %(foo\n) }
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) { %(foo\n) }
+
+ 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
diff --git a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb
index ef52c57289821cd52a1ed64214b3e5f79dc0ea99..05057789cc1eca1271e8482a8ae0861cf1f89651 100644
--- a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb
+++ b/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(%{This is a test})
@@ -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(%{This is a test})
+
+ expect(result.to_html).to eq(%{This is a test})
+ end
+
+ include_examples "XSS prevention", lang
+ include_examples "XSS prevention",
+ "#{lang}#{described_class::PARAMS_DELIMITER}<script>alert(1)</script>"
+ include_examples "XSS prevention",
+ "#{lang}#{described_class::PARAMS_DELIMITER}"
+ 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(%{This is a test})
+
+ expect(result.to_html).to eq(%{This is a test})
+ end
end
end
diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb
index fda00a693f05ac6999c843599161eb98f0f69139..67e5f4f7e4119d3b78d2f30072145eec50d2fabc 100644
--- a/spec/models/diff_note_spec.rb
+++ b/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) }