Implement multi-line suggestions filtering
Implements the filtering logic for `suggestion:-x+y` syntax.
This commit is contained in:
parent
74ebeebbcd
commit
53a5960496
|
@ -88,7 +88,7 @@ class DiffNote < Note
|
||||||
end
|
end
|
||||||
|
|
||||||
def banzai_render_context(field)
|
def banzai_render_context(field)
|
||||||
super.merge(suggestions_filter_enabled: supports_suggestion?)
|
super.merge(project: project, suggestions_filter_enabled: supports_suggestion?)
|
||||||
end
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Prepare multi-line suggestions for rendering in Markdown
|
||||||
|
merge_request: 26107
|
||||||
|
author:
|
||||||
|
type: other
|
|
@ -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
|
|
@ -12,6 +12,7 @@ module Banzai
|
||||||
# :only_path - Generate path-only links.
|
# :only_path - Generate path-only links.
|
||||||
class ReferenceFilter < HTML::Pipeline::Filter
|
class ReferenceFilter < HTML::Pipeline::Filter
|
||||||
include RequestStoreReferenceCache
|
include RequestStoreReferenceCache
|
||||||
|
include OutputSafety
|
||||||
|
|
||||||
class << self
|
class << self
|
||||||
attr_accessor :reference_type
|
attr_accessor :reference_type
|
||||||
|
@ -43,10 +44,6 @@ module Banzai
|
||||||
end.join(' ')
|
end.join(' ')
|
||||||
end
|
end
|
||||||
|
|
||||||
def escape_once(html)
|
|
||||||
html.html_safe? ? html : ERB::Util.html_escape_once(html)
|
|
||||||
end
|
|
||||||
|
|
||||||
def ignore_ancestor_query
|
def ignore_ancestor_query
|
||||||
@ignore_ancestor_query ||= begin
|
@ignore_ancestor_query ||= begin
|
||||||
parents = %w(pre code a style)
|
parents = %w(pre code a style)
|
||||||
|
|
|
@ -6,11 +6,15 @@ module Banzai
|
||||||
class SuggestionFilter < HTML::Pipeline::Filter
|
class SuggestionFilter < HTML::Pipeline::Filter
|
||||||
# Class used for tagging elements that should be rendered
|
# Class used for tagging elements that should be rendered
|
||||||
TAG_CLASS = 'js-render-suggestion'.freeze
|
TAG_CLASS = 'js-render-suggestion'.freeze
|
||||||
|
SUGGESTION_REGEX = Gitlab::Diff::SuggestionsParser::SUGGESTION_CONTEXT
|
||||||
|
|
||||||
def call
|
def call
|
||||||
return doc unless suggestions_filter_enabled?
|
return doc unless suggestions_filter_enabled?
|
||||||
|
|
||||||
doc.search('pre.suggestion > code').each do |node|
|
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)
|
node.add_class(TAG_CLASS)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -20,6 +24,20 @@ module Banzai
|
||||||
def suggestions_filter_enabled?
|
def suggestions_filter_enabled?
|
||||||
context[:suggestions_filter_enabled]
|
context[:suggestions_filter_enabled]
|
||||||
end
|
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
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -8,6 +8,11 @@ module Banzai
|
||||||
# HTML Filter to highlight fenced code blocks
|
# HTML Filter to highlight fenced code blocks
|
||||||
#
|
#
|
||||||
class SyntaxHighlightFilter < HTML::Pipeline::Filter
|
class SyntaxHighlightFilter < HTML::Pipeline::Filter
|
||||||
|
include OutputSafety
|
||||||
|
|
||||||
|
PARAMS_DELIMITER = ':'.freeze
|
||||||
|
LANG_PARAMS_ATTR = 'data-lang-params'.freeze
|
||||||
|
|
||||||
def call
|
def call
|
||||||
doc.search('pre > code').each do |node|
|
doc.search('pre > code').each do |node|
|
||||||
highlight_node(node)
|
highlight_node(node)
|
||||||
|
@ -18,7 +23,7 @@ module Banzai
|
||||||
|
|
||||||
def highlight_node(node)
|
def highlight_node(node)
|
||||||
css_classes = +'code highlight js-syntax-highlight'
|
css_classes = +'code highlight js-syntax-highlight'
|
||||||
lang = node.attr('lang')
|
lang, lang_params = parse_lang_params(node.attr('lang'))
|
||||||
retried = false
|
retried = false
|
||||||
|
|
||||||
if use_rouge?(lang)
|
if use_rouge?(lang)
|
||||||
|
@ -46,7 +51,10 @@ module Banzai
|
||||||
retry
|
retry
|
||||||
end
|
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
|
# Extracted to a method to measure it
|
||||||
replace_parent_pre_element(node, highlighted)
|
replace_parent_pre_element(node, highlighted)
|
||||||
|
@ -54,6 +62,15 @@ module Banzai
|
||||||
|
|
||||||
private
|
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.
|
# Separate method so it can be instrumented.
|
||||||
def lex(lexer, code)
|
def lex(lexer, code)
|
||||||
lexer.lex(code)
|
lexer.lex(code)
|
||||||
|
|
|
@ -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
|
|
@ -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
|
|
@ -5,7 +5,7 @@ require 'spec_helper'
|
||||||
describe Banzai::Filter::SuggestionFilter do
|
describe Banzai::Filter::SuggestionFilter do
|
||||||
include FilterSpecHelper
|
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
|
let(:default_context) do
|
||||||
{ suggestions_filter_enabled: true }
|
{ suggestions_filter_enabled: true }
|
||||||
end
|
end
|
||||||
|
@ -23,4 +23,35 @@ describe Banzai::Filter::SuggestionFilter do
|
||||||
|
|
||||||
expect(result[:class]).to be_nil
|
expect(result[:class]).to be_nil
|
||||||
end
|
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
|
end
|
||||||
|
|
|
@ -45,7 +45,10 @@ describe Banzai::Filter::SyntaxHighlightFilter do
|
||||||
end
|
end
|
||||||
|
|
||||||
context "languages that should be passed through" do
|
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
|
context "when #{lang} is specified" do
|
||||||
it "highlights as plaintext but with the correct language attribute and class" 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>})
|
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
|
include_examples "XSS prevention", lang
|
||||||
end
|
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
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -336,6 +336,16 @@ describe DiffNote do
|
||||||
end
|
end
|
||||||
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
|
describe "image diff notes" do
|
||||||
subject { build(:image_diff_note_on_merge_request, project: project, noteable: merge_request) }
|
subject { build(:image_diff_note_on_merge_request, project: project, noteable: merge_request) }
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue