diff options
| -rw-r--r-- | app/models/diff_note.rb | 2 | ||||
| -rw-r--r-- | changelogs/unreleased/osw-multi-line-suggestions-parsing.yml | 5 | ||||
| -rw-r--r-- | lib/banzai/filter/output_safety.rb | 11 | ||||
| -rw-r--r-- | lib/banzai/filter/reference_filter.rb | 5 | ||||
| -rw-r--r-- | lib/banzai/filter/suggestion_filter.rb | 18 | ||||
| -rw-r--r-- | lib/banzai/filter/syntax_highlight_filter.rb | 21 | ||||
| -rw-r--r-- | lib/gitlab/diff/suggestions_parser.rb | 10 | ||||
| -rw-r--r-- | spec/lib/banzai/filter/output_safety_spec.rb | 29 | ||||
| -rw-r--r-- | spec/lib/banzai/filter/suggestion_filter_spec.rb | 33 | ||||
| -rw-r--r-- | spec/lib/banzai/filter/syntax_highlight_filter_spec.rb | 32 | ||||
| -rw-r--r-- | spec/models/diff_note_spec.rb | 10 |
11 files changed, 167 insertions, 9 deletions
diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index 805092e527a..87755cf3f3d 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 00000000000..985b01e9254 --- /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 00000000000..d4ebce5d9c9 --- /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 42f9b3a689c..b3ce9200b49 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 9950db373d8..848aca10a20 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 9ffde52b5f2..fe56f9a1e33 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 = %(<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) diff --git a/lib/gitlab/diff/suggestions_parser.rb b/lib/gitlab/diff/suggestions_parser.rb new file mode 100644 index 00000000000..043bd9a4bcb --- /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 = /^(\-(?<above>\d+))?(\+(?<below>\d+))?$/.freeze + end + end +end diff --git a/spec/lib/banzai/filter/output_safety_spec.rb b/spec/lib/banzai/filter/output_safety_spec.rb new file mode 100644 index 00000000000..5ffe591c9a4 --- /dev/null +++ b/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 diff --git a/spec/lib/banzai/filter/suggestion_filter_spec.rb b/spec/lib/banzai/filter/suggestion_filter_spec.rb index b13c90b54bd..af6f002fa30 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) { "<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 diff --git a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb index ef52c572898..05057789cc1 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(%{<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}<script>alert(1)</script>" + 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 diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb index fda00a693f0..67e5f4f7e41 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) } |