Commit cc238077 authored by Brett Walker's avatar Brett Walker

Utilize C version of CommonMark renderer

Originally we used the Ruby version, which gave us
additional flexibility to handle a special case.
We've now refactored that out and can use the
C version, which is much more performant over
the Ruby version

Changelog: performance
parent c11f8b46
---
name: use_cmark_renderer
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61792
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/345744
milestone: '14.6'
type: development
group: group::project management
default_enabled: true
...@@ -7,13 +7,14 @@ module Banzai ...@@ -7,13 +7,14 @@ module Banzai
# Footnotes are supported in CommonMark. However we were stripping # Footnotes are supported in CommonMark. However we were stripping
# the ids during sanitization. Those are now allowed. # the ids during sanitization. Those are now allowed.
# #
# Footnotes are numbered the same - the first one has `id=fn1`, the # Footnotes are numbered as an increasing integer starting at `1`.
# second is `id=fn2`, etc. In order to allow footnotes when rendering # The `id` associated with a footnote is based on the footnote reference
# multiple markdown blocks on a page, we need to make each footnote # string. For example, `[^foot]` will generate `id="fn-foot"`.
# reference unique. # In order to allow footnotes when rendering multiple markdown blocks
# # on a page, we need to make each footnote reference unique.
# This filter adds a random number to each footnote (the same number # This filter adds a random number to each footnote (the same number
# can be used for a single render). So you get `id=fn1-4335` and `id=fn2-4335`. # can be used for a single render). So you get `id=fn-1-4335` and `id=fn-foot-4335`.
# #
class FootnoteFilter < HTML::Pipeline::Filter class FootnoteFilter < HTML::Pipeline::Filter
FOOTNOTE_ID_PREFIX = 'fn-' FOOTNOTE_ID_PREFIX = 'fn-'
...@@ -26,53 +27,24 @@ module Banzai ...@@ -26,53 +27,24 @@ module Banzai
CSS_FOOTNOTE = 'sup > a[data-footnote-ref]' CSS_FOOTNOTE = 'sup > a[data-footnote-ref]'
XPATH_FOOTNOTE = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_FOOTNOTE).freeze XPATH_FOOTNOTE = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_FOOTNOTE).freeze
# only needed when feature flag use_cmark_renderer is turned off
INTEGER_PATTERN = /\A\d+\z/.freeze
FOOTNOTE_ID_PREFIX_OLD = 'fn'
FOOTNOTE_LINK_ID_PREFIX_OLD = 'fnref'
FOOTNOTE_LI_REFERENCE_PATTERN_OLD = /\A#{FOOTNOTE_ID_PREFIX_OLD}\d+\z/.freeze
FOOTNOTE_LINK_REFERENCE_PATTERN_OLD = /\A#{FOOTNOTE_LINK_ID_PREFIX_OLD}\d+\z/.freeze
FOOTNOTE_START_NUMBER = 1
CSS_SECTION_OLD = "ol > li[id=#{FOOTNOTE_ID_PREFIX_OLD}#{FOOTNOTE_START_NUMBER}]"
XPATH_SECTION_OLD = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_SECTION_OLD).freeze
def call def call
if Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml) # Sanitization stripped off the section class - add it back in
# Sanitization stripped off the section class - add it back in return doc unless section_node = doc.at_xpath(XPATH_SECTION)
return doc unless section_node = doc.at_xpath(XPATH_SECTION)
section_node.append_class('footnotes') section_node.append_class('footnotes')
else
return doc unless first_footnote = doc.at_xpath(XPATH_SECTION_OLD)
return doc unless first_footnote.parent
first_footnote.parent.wrap('<section class="footnotes">')
end
rand_suffix = "-#{random_number}" rand_suffix = "-#{random_number}"
modified_footnotes = {} modified_footnotes = {}
xpath_footnote = if Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml) doc.xpath(XPATH_FOOTNOTE).each do |link_node|
XPATH_FOOTNOTE ref_num = link_node[:id].delete_prefix(FOOTNOTE_LINK_ID_PREFIX)
else ref_num.gsub!(/[[:punct:]]/, '\\\\\&')
Gitlab::Utils::Nokogiri.css_to_xpath('sup > a[id]')
end
doc.xpath(xpath_footnote).each do |link_node|
if Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml)
ref_num = link_node[:id].delete_prefix(FOOTNOTE_LINK_ID_PREFIX)
ref_num.gsub!(/[[:punct:]]/, '\\\\\&')
else
ref_num = link_node[:id].delete_prefix(FOOTNOTE_LINK_ID_PREFIX_OLD)
end
css = Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml) ? "section[data-footnotes] li[id=#{fn_id(ref_num)}]" : "li[id=#{fn_id(ref_num)}]" css = "section[data-footnotes] li[id=#{fn_id(ref_num)}]"
node_xpath = Gitlab::Utils::Nokogiri.css_to_xpath(css) node_xpath = Gitlab::Utils::Nokogiri.css_to_xpath(css)
footnote_node = doc.at_xpath(node_xpath) footnote_node = doc.at_xpath(node_xpath)
if footnote_node || modified_footnotes[ref_num] if footnote_node || modified_footnotes[ref_num]
next if Feature.disabled?(:use_cmark_renderer, default_enabled: :yaml) && !INTEGER_PATTERN.match?(ref_num)
link_node[:href] += rand_suffix link_node[:href] += rand_suffix
link_node[:id] += rand_suffix link_node[:id] += rand_suffix
...@@ -103,13 +75,11 @@ module Banzai ...@@ -103,13 +75,11 @@ module Banzai
end end
def fn_id(num) def fn_id(num)
prefix = Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml) ? FOOTNOTE_ID_PREFIX : FOOTNOTE_ID_PREFIX_OLD "#{FOOTNOTE_ID_PREFIX}#{num}"
"#{prefix}#{num}"
end end
def fnref_id(num) def fnref_id(num)
prefix = Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml) ? FOOTNOTE_LINK_ID_PREFIX : FOOTNOTE_LINK_ID_PREFIX_OLD "#{FOOTNOTE_LINK_ID_PREFIX}#{num}"
"#{prefix}#{num}"
end end
end end
end end
......
...@@ -4,8 +4,8 @@ ...@@ -4,8 +4,8 @@
# This module is used in Banzai::Filter::MarkdownFilter. # This module is used in Banzai::Filter::MarkdownFilter.
# Used gem is `commonmarker` which is a ruby wrapper for libcmark (CommonMark parser) # Used gem is `commonmarker` which is a ruby wrapper for libcmark (CommonMark parser)
# including GitHub's GFM extensions. # including GitHub's GFM extensions.
# We now utilize the renderer built in `C`, rather than the ruby based renderer.
# Homepage: https://github.com/gjtorikian/commonmarker # Homepage: https://github.com/gjtorikian/commonmarker
module Banzai module Banzai
module Filter module Filter
module MarkdownEngines module MarkdownEngines
...@@ -22,57 +22,29 @@ module Banzai ...@@ -22,57 +22,29 @@ module Banzai
:VALIDATE_UTF8 # replace illegal sequences with the replacement character U+FFFD. :VALIDATE_UTF8 # replace illegal sequences with the replacement character U+FFFD.
].freeze ].freeze
RENDER_OPTIONS_C = [ RENDER_OPTIONS = [
:GITHUB_PRE_LANG, # use GitHub-style <pre lang> for fenced code blocks. :GITHUB_PRE_LANG, # use GitHub-style <pre lang> for fenced code blocks.
:FOOTNOTES, # render footnotes. :FOOTNOTES, # render footnotes.
:FULL_INFO_STRING, # include full info strings of code blocks in separate attribute. :FULL_INFO_STRING, # include full info strings of code blocks in separate attribute.
:UNSAFE # allow raw/custom HTML and unsafe links. :UNSAFE # allow raw/custom HTML and unsafe links.
].freeze ].freeze
# The `:GITHUB_PRE_LANG` option is not used intentionally because
# it renders a fence block with language as `<pre lang="LANG"><code>some code\n</code></pre>`
# while GitLab's syntax is `<pre><code lang="LANG">some code\n</code></pre>`.
# If in the future the syntax is about to be made GitHub-compatible, please, add `:GITHUB_PRE_LANG` render option below
# and remove `code_block` method from `lib/banzai/renderer/common_mark/html.rb`.
RENDER_OPTIONS_RUBY = [
# as of commonmarker 0.18.0, we need to use :UNSAFE to get the same as the original :DEFAULT
# https://github.com/gjtorikian/commonmarker/pull/81
:UNSAFE # allow raw/custom HTML and unsafe links.
].freeze
def initialize(context) def initialize(context)
@context = context @context = context
@renderer = Banzai::Renderer::CommonMark::HTML.new(options: render_options) if Feature.disabled?(:use_cmark_renderer, default_enabled: :yaml)
end end
def render(text) def render(text)
if Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml) CommonMarker.render_html(text, render_options, EXTENSIONS)
CommonMarker.render_html(text, render_options, extensions)
else
doc = CommonMarker.render_doc(text, PARSE_OPTIONS, extensions)
@renderer.render(doc)
end
end end
private private
def extensions
if Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml)
EXTENSIONS
else
EXTENSIONS + [
:tagfilter # strips out several "unsafe" HTML tags from being used: https://github.github.com/gfm/#disallowed-raw-html-extension-
].freeze
end
end
def render_options def render_options
@context[:no_sourcepos] ? render_options_no_sourcepos : render_options_sourcepos @context[:no_sourcepos] ? render_options_no_sourcepos : render_options_sourcepos
end end
def render_options_no_sourcepos def render_options_no_sourcepos
Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml) ? RENDER_OPTIONS_C : RENDER_OPTIONS_RUBY RENDER_OPTIONS
end end
def render_options_sourcepos def render_options_sourcepos
......
...@@ -8,8 +8,10 @@ module Banzai ...@@ -8,8 +8,10 @@ module Banzai
NOT_LITERAL_REGEX = %r{#{LITERAL_KEYWORD}-((%5C|\\).+?)-#{LITERAL_KEYWORD}}.freeze NOT_LITERAL_REGEX = %r{#{LITERAL_KEYWORD}-((%5C|\\).+?)-#{LITERAL_KEYWORD}}.freeze
SPAN_REGEX = %r{<span>(.*?)</span>}.freeze SPAN_REGEX = %r{<span>(.*?)</span>}.freeze
CSS_A = 'a' CSS_A = 'a'
XPATH_A = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_A).freeze XPATH_A = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_A).freeze
CSS_LANG_TAG = 'pre'
XPATH_LANG_TAG = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_LANG_TAG).freeze
def call def call
return doc unless result[:escaped_literals] return doc unless result[:escaped_literals]
...@@ -32,22 +34,12 @@ module Banzai ...@@ -32,22 +34,12 @@ module Banzai
node.attributes['title'].value = node.attributes['title'].value.gsub(SPAN_REGEX, '\1') if node.attributes['title'] node.attributes['title'].value = node.attributes['title'].value.gsub(SPAN_REGEX, '\1') if node.attributes['title']
end end
doc.xpath(lang_tag).each do |node| doc.xpath(XPATH_LANG_TAG).each do |node|
node.attributes['lang'].value = node.attributes['lang'].value.gsub(SPAN_REGEX, '\1') if node.attributes['lang'] node.attributes['lang'].value = node.attributes['lang'].value.gsub(SPAN_REGEX, '\1') if node.attributes['lang']
end end
doc doc
end end
private
def lang_tag
if Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml)
Gitlab::Utils::Nokogiri.css_to_xpath('pre')
else
Gitlab::Utils::Nokogiri.css_to_xpath('code')
end
end
end end
end end
end end
...@@ -25,12 +25,7 @@ module Banzai ...@@ -25,12 +25,7 @@ module Banzai
private private
def lang_tag def lang_tag
@lang_tag ||= @lang_tag ||= Gitlab::Utils::Nokogiri.css_to_xpath('pre[lang="plantuml"] > code').freeze
if Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml)
Gitlab::Utils::Nokogiri.css_to_xpath('pre[lang="plantuml"] > code').freeze
else
Gitlab::Utils::Nokogiri.css_to_xpath('pre > code[lang="plantuml"]').freeze
end
end end
def settings def settings
......
...@@ -28,12 +28,10 @@ module Banzai ...@@ -28,12 +28,10 @@ module Banzai
allowlist[:attributes]['li'] = %w[id] allowlist[:attributes]['li'] = %w[id]
allowlist[:transformers].push(self.class.remove_non_footnote_ids) allowlist[:transformers].push(self.class.remove_non_footnote_ids)
if Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml) # Allow section elements with data-footnotes attribute
# Allow section elements with data-footnotes attribute allowlist[:elements].push('section')
allowlist[:elements].push('section') allowlist[:attributes]['section'] = %w(data-footnotes)
allowlist[:attributes]['section'] = %w(data-footnotes) allowlist[:attributes]['a'].push('data-footnote-ref', 'data-footnote-backref')
allowlist[:attributes]['a'].push('data-footnote-ref', 'data-footnote-backref')
end
allowlist allowlist
end end
...@@ -61,13 +59,8 @@ module Banzai ...@@ -61,13 +59,8 @@ module Banzai
return unless node.name == 'a' || node.name == 'li' return unless node.name == 'a' || node.name == 'li'
return unless node.has_attribute?('id') return unless node.has_attribute?('id')
if Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml) return if node.name == 'a' && node['id'] =~ Banzai::Filter::FootnoteFilter::FOOTNOTE_LINK_REFERENCE_PATTERN
return if node.name == 'a' && node['id'] =~ Banzai::Filter::FootnoteFilter::FOOTNOTE_LINK_REFERENCE_PATTERN return if node.name == 'li' && node['id'] =~ Banzai::Filter::FootnoteFilter::FOOTNOTE_LI_REFERENCE_PATTERN
return if node.name == 'li' && node['id'] =~ Banzai::Filter::FootnoteFilter::FOOTNOTE_LI_REFERENCE_PATTERN
else
return if node.name == 'a' && node['id'] =~ Banzai::Filter::FootnoteFilter::FOOTNOTE_LINK_REFERENCE_PATTERN_OLD
return if node.name == 'li' && node['id'] =~ Banzai::Filter::FootnoteFilter::FOOTNOTE_LI_REFERENCE_PATTERN_OLD
end
node.remove_attribute('id') node.remove_attribute('id')
end end
......
...@@ -70,11 +70,11 @@ module Banzai ...@@ -70,11 +70,11 @@ module Banzai
private private
def parse_lang_params(node) def parse_lang_params(node)
node = node.parent if Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml) node = node.parent
# Commonmarker's FULL_INFO_STRING render option works with the space delimiter. # Commonmarker's FULL_INFO_STRING render option works with the space delimiter.
# But the current behavior of GitLab's markdown renderer is different - it grabs everything as the single # But the current behavior of GitLab's markdown renderer is different - it grabs everything as the single
# line, including language and its options. To keep backward compatability, we have to parse the old format and # line, including language and its options. To keep backward compatibility, we have to parse the old format and
# merge with the new one. # merge with the new one.
# #
# Behaviors before separating language and its parameters: # Behaviors before separating language and its parameters:
...@@ -91,11 +91,7 @@ module Banzai ...@@ -91,11 +91,7 @@ module Banzai
return unless language return unless language
language, language_params = language.split(LANG_PARAMS_DELIMITER, 2) language, language_params = language.split(LANG_PARAMS_DELIMITER, 2)
language_params = [node.attr('data-meta'), language_params].compact.join(' ')
if Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml)
language_params = [node.attr('data-meta'), language_params].compact.join(' ')
end
formatted_language_params = format_language_params(language_params) formatted_language_params = format_language_params(language_params)
[language, formatted_language_params] [language, formatted_language_params]
......
# frozen_string_literal: true
# Remove this entire file when removing `use_cmark_renderer` feature flag and switching to the CMARK html renderer.
# https://gitlab.com/gitlab-org/gitlab/-/issues/345744
module Banzai
module Renderer
module CommonMark
class HTML < CommonMarker::HtmlRenderer
def code_block(node)
block do
out("<pre#{sourcepos(node)}><code")
out(' lang="', node.fence_info, '"') if node.fence_info.present?
out('>')
out(escape_html(node.string_content))
out('</code></pre>')
end
end
end
end
end
end
...@@ -7,11 +7,7 @@ module Gitlab ...@@ -7,11 +7,7 @@ module Gitlab
register_for 'gitlab-html-pipeline' register_for 'gitlab-html-pipeline'
def format(node, lang, opts) def format(node, lang, opts)
if Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml) %(<pre #{lang ? %[lang="#{lang}"] : ''}><code>#{node.content}</code></pre>)
%(<pre #{lang ? %[lang="#{lang}"] : ''}><code>#{node.content}</code></pre>)
else
%(<pre><code #{lang ? %[ lang="#{lang}"] : ''}>#{node.content}</code></pre>)
end
end end
end end
end end
......
...@@ -56,52 +56,6 @@ RSpec.describe Banzai::Filter::FootnoteFilter do ...@@ -56,52 +56,6 @@ RSpec.describe Banzai::Filter::FootnoteFilter do
it 'properly adds the necessary ids and classes' do it 'properly adds the necessary ids and classes' do
expect(doc.to_html).to eq filtered_footnote.strip expect(doc.to_html).to eq filtered_footnote.strip
end end
context 'using ruby-based HTML renderer' do
# first[^1] and second[^second]
# [^1]: one
# [^second]: two
let(:footnote) do
<<~EOF
<p>first<sup><a href="#fn1" id="fnref1">1</a></sup> and second<sup><a href="#fn2" id="fnref2">2</a></sup></p>
<p>same reference<sup><a href="#fn1" id="fnref1">1</a></sup></p>
<ol>
<li id="fn1">
<p>one <a href="#fnref1">↩</a></p>
</li>
<li id="fn2">
<p>two <a href="#fnref2">↩</a></p>
</li>
</ol>
EOF
end
let(:filtered_footnote) do
<<~EOF
<p>first<sup class="footnote-ref"><a href="#fn1-#{identifier}" id="fnref1-#{identifier}">1</a></sup> and second<sup class="footnote-ref"><a href="#fn2-#{identifier}" id="fnref2-#{identifier}">2</a></sup></p>
<p>same reference<sup class="footnote-ref"><a href="#fn1-#{identifier}" id="fnref1-#{identifier}">1</a></sup></p>
<section class="footnotes"><ol>
<li id="fn1-#{identifier}">
<p>one <a href="#fnref1-#{identifier}" class="footnote-backref">↩</a></p>
</li>
<li id="fn2-#{identifier}">
<p>two <a href="#fnref2-#{identifier}" class="footnote-backref">↩</a></p>
</li>
</ol></section>
EOF
end
let(:doc) { filter(footnote) }
let(:identifier) { link_node[:id].delete_prefix('fnref1-') }
before do
stub_feature_flags(use_cmark_renderer: false)
end
it 'properly adds the necessary ids and classes' do
expect(doc.to_html).to eq filtered_footnote
end
end
end end
context 'when detecting footnotes' do context 'when detecting footnotes' do
......
...@@ -5,125 +5,90 @@ require 'spec_helper' ...@@ -5,125 +5,90 @@ require 'spec_helper'
RSpec.describe Banzai::Filter::MarkdownFilter do RSpec.describe Banzai::Filter::MarkdownFilter do
include FilterSpecHelper include FilterSpecHelper
shared_examples_for 'renders correct markdown' do describe 'markdown engine from context' do
describe 'markdown engine from context' do it 'defaults to CommonMark' do
it 'defaults to CommonMark' do expect_next_instance_of(Banzai::Filter::MarkdownEngines::CommonMark) do |instance|
expect_next_instance_of(Banzai::Filter::MarkdownEngines::CommonMark) do |instance| expect(instance).to receive(:render).and_return('test')
expect(instance).to receive(:render).and_return('test')
end
filter('test')
end end
it 'uses CommonMark' do filter('test')
expect_next_instance_of(Banzai::Filter::MarkdownEngines::CommonMark) do |instance| end
expect(instance).to receive(:render).and_return('test')
end
filter('test', { markdown_engine: :common_mark }) it 'uses CommonMark' do
expect_next_instance_of(Banzai::Filter::MarkdownEngines::CommonMark) do |instance|
expect(instance).to receive(:render).and_return('test')
end end
filter('test', { markdown_engine: :common_mark })
end end
end
describe 'code block' do describe 'code block' do
context 'using CommonMark' do context 'using CommonMark' do
before do before do
stub_const('Banzai::Filter::MarkdownFilter::DEFAULT_ENGINE', :common_mark) stub_const('Banzai::Filter::MarkdownFilter::DEFAULT_ENGINE', :common_mark)
end
it 'adds language to lang attribute when specified' do
result = filter("```html\nsome code\n```", no_sourcepos: true)
if Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml)
expect(result).to start_with('<pre lang="html"><code>')
else
expect(result).to start_with('<pre><code lang="html">')
end
end
it 'does not add language to lang attribute when not specified' do
result = filter("```\nsome code\n```", no_sourcepos: true)
expect(result).to start_with('<pre><code>')
end
it 'works with utf8 chars in language' do
result = filter("```日\nsome code\n```", no_sourcepos: true)
if Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml)
expect(result).to start_with('<pre lang="日"><code>')
else
expect(result).to start_with('<pre><code lang="日">')
end
end
it 'works with additional language parameters' do
result = filter("```ruby:red gem foo\nsome code\n```", no_sourcepos: true)
if Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml)
expect(result).to start_with('<pre lang="ruby:red" data-meta="gem foo"><code>')
else
expect(result).to start_with('<pre><code lang="ruby:red gem foo">')
end
end
end end
end
describe 'source line position' do it 'adds language to lang attribute when specified' do
context 'using CommonMark' do result = filter("```html\nsome code\n```", no_sourcepos: true)
before do
stub_const('Banzai::Filter::MarkdownFilter::DEFAULT_ENGINE', :common_mark)
end
it 'defaults to add data-sourcepos' do expect(result).to start_with('<pre lang="html"><code>')
result = filter('test') end
expect(result).to eq '<p data-sourcepos="1:1-1:4">test</p>' it 'does not add language to lang attribute when not specified' do
end result = filter("```\nsome code\n```", no_sourcepos: true)
it 'disables data-sourcepos' do expect(result).to start_with('<pre><code>')
result = filter('test', no_sourcepos: true) end
it 'works with utf8 chars in language' do
result = filter("```日\nsome code\n```", no_sourcepos: true)
expect(result).to eq '<p>test</p>' expect(result).to start_with('<pre lang="日"><code>')
end end
it 'works with additional language parameters' do
result = filter("```ruby:red gem foo\nsome code\n```", no_sourcepos: true)
expect(result).to start_with('<pre lang="ruby:red" data-meta="gem foo"><code>')
end end
end end
end
describe 'footnotes in tables' do describe 'source line position' do
it 'processes footnotes in table cells' do context 'using CommonMark' do
text = <<-MD.strip_heredoc before do
| Column1 | stub_const('Banzai::Filter::MarkdownFilter::DEFAULT_ENGINE', :common_mark)
| --------- | end
| foot [^1] |
[^1]: a footnote it 'defaults to add data-sourcepos' do
MD result = filter('test')
result = filter(text, no_sourcepos: true) expect(result).to eq '<p data-sourcepos="1:1-1:4">test</p>'
end
expect(result).to include('<td>foot <sup') it 'disables data-sourcepos' do
result = filter('test', no_sourcepos: true)
if Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml) expect(result).to eq '<p>test</p>'
expect(result).to include('<section class="footnotes" data-footnotes>')
else
expect(result).to include('<section class="footnotes">')
end
end end
end end
end end
context 'using ruby-based HTML renderer' do describe 'footnotes in tables' do
before do it 'processes footnotes in table cells' do
stub_feature_flags(use_cmark_renderer: false) text = <<-MD.strip_heredoc
end | Column1 |
| --------- |
| foot [^1] |
it_behaves_like 'renders correct markdown' [^1]: a footnote
end MD
context 'using c-based HTML renderer' do result = filter(text, no_sourcepos: true)
before do
stub_feature_flags(use_cmark_renderer: true)
end
it_behaves_like 'renders correct markdown' expect(result).to include('<td>foot <sup')
expect(result).to include('<section class="footnotes" data-footnotes>')
end
end end
end end
...@@ -5,67 +5,33 @@ require 'spec_helper' ...@@ -5,67 +5,33 @@ require 'spec_helper'
RSpec.describe Banzai::Filter::PlantumlFilter do RSpec.describe Banzai::Filter::PlantumlFilter do
include FilterSpecHelper include FilterSpecHelper
shared_examples_for 'renders correct markdown' do it 'replaces plantuml pre tag with img tag' do
it 'replaces plantuml pre tag with img tag' do stub_application_setting(plantuml_enabled: true, plantuml_url: "http://localhost:8080")
stub_application_setting(plantuml_enabled: true, plantuml_url: "http://localhost:8080")
input = if Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml) input = '<pre lang="plantuml"><code>Bob -> Sara : Hello</code></pre>'
'<pre lang="plantuml"><code>Bob -> Sara : Hello</code></pre>' output = '<div class="imageblock"><div class="content"><img class="plantuml" src="http://localhost:8080/png/U9npoazIqBLJ24uiIbImKl18pSd91m0rkGMq"></div></div>'
else doc = filter(input)
'<pre><code lang="plantuml">Bob -> Sara : Hello</code></pre>'
end
output = '<div class="imageblock"><div class="content"><img class="plantuml" src="http://localhost:8080/png/U9npoazIqBLJ24uiIbImKl18pSd91m0rkGMq"></div></div>' expect(doc.to_s).to eq output
doc = filter(input) end
expect(doc.to_s).to eq output
end
it 'does not replace plantuml pre tag with img tag if disabled' do
stub_application_setting(plantuml_enabled: false)
if Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml)
input = '<pre lang="plantuml"><code>Bob -> Sara : Hello</code></pre>'
output = '<pre lang="plantuml"><code>Bob -&gt; Sara : Hello</code></pre>'
else
input = '<pre><code lang="plantuml">Bob -> Sara : Hello</code></pre>'
output = '<pre><code lang="plantuml">Bob -&gt; Sara : Hello</code></pre>'
end
doc = filter(input)
expect(doc.to_s).to eq output
end
it 'does not replace plantuml pre tag with img tag if url is invalid' do
stub_application_setting(plantuml_enabled: true, plantuml_url: "invalid")
input = if Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml) it 'does not replace plantuml pre tag with img tag if disabled' do
'<pre lang="plantuml"><code>Bob -> Sara : Hello</code></pre>' stub_application_setting(plantuml_enabled: false)
else
'<pre><code lang="plantuml">Bob -> Sara : Hello</code></pre>'
end
output = '<div class="listingblock"><div class="content"><pre class="plantuml plantuml-error"> Error: cannot connect to PlantUML server at "invalid"</pre></div></div>' input = '<pre lang="plantuml"><code>Bob -> Sara : Hello</code></pre>'
doc = filter(input) output = '<pre lang="plantuml"><code>Bob -&gt; Sara : Hello</code></pre>'
doc = filter(input)
expect(doc.to_s).to eq output expect(doc.to_s).to eq output
end
end end
context 'using ruby-based HTML renderer' do it 'does not replace plantuml pre tag with img tag if url is invalid' do
before do stub_application_setting(plantuml_enabled: true, plantuml_url: "invalid")
stub_feature_flags(use_cmark_renderer: false)
end
it_behaves_like 'renders correct markdown'
end
context 'using c-based HTML renderer' do input = '<pre lang="plantuml"><code>Bob -> Sara : Hello</code></pre>'
before do output = '<div class="listingblock"><div class="content"><pre class="plantuml plantuml-error"> Error: cannot connect to PlantUML server at "invalid"</pre></div></div>'
stub_feature_flags(use_cmark_renderer: true) doc = filter(input)
end
it_behaves_like 'renders correct markdown' expect(doc.to_s).to eq output
end end
end end
...@@ -177,53 +177,6 @@ RSpec.describe Banzai::Filter::SanitizationFilter do ...@@ -177,53 +177,6 @@ RSpec.describe Banzai::Filter::SanitizationFilter do
expect(act.to_html).to eq exp expect(act.to_html).to eq exp
end end
end end
context 'using ruby-based HTML renderer' do
before do
stub_feature_flags(use_cmark_renderer: false)
end
it 'allows correct footnote id property on links' do
exp = %q(<a href="#fn1" id="fnref1">foo/bar.md</a>)
act = filter(exp)
expect(act.to_html).to eq exp
end
it 'allows correct footnote id property on li element' do
exp = %q(<ol><li id="fn1">footnote</li></ol>)
act = filter(exp)
expect(act.to_html).to eq exp
end
it 'removes invalid id for footnote links' do
exp = %q(<a href="#fn1">link</a>)
%w[fnrefx test xfnref1].each do |id|
act = filter(%(<a href="#fn1" id="#{id}">link</a>))
expect(act.to_html).to eq exp
end
end
it 'removes invalid id for footnote li' do
exp = %q(<ol><li>footnote</li></ol>)
%w[fnx test xfn1].each do |id|
act = filter(%(<ol><li id="#{id}">footnote</li></ol>))
expect(act.to_html).to eq exp
end
end
it 'allows footnotes numbered higher than 9' do
exp = %q(<a href="#fn15" id="fnref15">link</a><ol><li id="fn15">footnote</li></ol>)
act = filter(exp)
expect(act.to_html).to eq exp
end
end
end end
end end
end end
...@@ -65,47 +65,6 @@ RSpec.describe Banzai::Pipeline::FullPipeline do ...@@ -65,47 +65,6 @@ RSpec.describe Banzai::Pipeline::FullPipeline do
expect(html.lines.map(&:strip).join("\n")).to eq filtered_footnote.strip expect(html.lines.map(&:strip).join("\n")).to eq filtered_footnote.strip
end end
context 'using ruby-based HTML renderer' do
let(:html) { described_class.to_html(footnote_markdown, project: project) }
let(:identifier) { html[/.*fnref1-(\d+).*/, 1] }
let(:footnote_markdown) do
<<~EOF
first[^1] and second[^second] and twenty[^twenty]
[^1]: one
[^second]: two
[^twenty]: twenty
EOF
end
let(:filtered_footnote) do
<<~EOF
<p dir="auto">first<sup class="footnote-ref"><a href="#fn1-#{identifier}" id="fnref1-#{identifier}">1</a></sup> and second<sup class="footnote-ref"><a href="#fn2-#{identifier}" id="fnref2-#{identifier}">2</a></sup> and twenty<sup class="footnote-ref"><a href="#fn3-#{identifier}" id="fnref3-#{identifier}">3</a></sup></p>
<section class="footnotes"><ol>
<li id="fn1-#{identifier}">
<p>one <a href="#fnref1-#{identifier}" class="footnote-backref"><gl-emoji title="leftwards arrow with hook" data-name="leftwards_arrow_with_hook" data-unicode-version="1.1">↩</gl-emoji></a></p>
</li>
<li id="fn2-#{identifier}">
<p>two <a href="#fnref2-#{identifier}" class="footnote-backref"><gl-emoji title="leftwards arrow with hook" data-name="leftwards_arrow_with_hook" data-unicode-version="1.1">↩</gl-emoji></a></p>
</li>
<li id="fn3-#{identifier}">
<p>twenty <a href="#fnref3-#{identifier}" class="footnote-backref"><gl-emoji title="leftwards arrow with hook" data-name="leftwards_arrow_with_hook" data-unicode-version="1.1">↩</gl-emoji></a></p>
</li>
</ol></section>
EOF
end
before do
stub_feature_flags(use_cmark_renderer: false)
end
it 'properly adds the necessary ids and classes' do
stub_commonmark_sourcepos_disabled
expect(html.lines.map(&:strip).join("\n")).to eq filtered_footnote
end
end
end end
describe 'links are detected as malicious' do describe 'links are detected as malicious' do
......
...@@ -5,117 +5,93 @@ require 'spec_helper' ...@@ -5,117 +5,93 @@ require 'spec_helper'
RSpec.describe Banzai::Pipeline::PlainMarkdownPipeline do RSpec.describe Banzai::Pipeline::PlainMarkdownPipeline do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
shared_examples_for 'renders correct markdown' do describe 'backslash escapes', :aggregate_failures do
describe 'CommonMark tests', :aggregate_failures do let_it_be(:project) { create(:project, :public) }
it 'converts all reference punctuation to literals' do let_it_be(:issue) { create(:issue, project: project) }
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
punctuation.each { |char| expect(output).to include("<span>#{char}</span>") }
expect(result[:escaped_literals]).to be_truthy
end
it 'ensure we handle all the GitLab reference characters', :eager_load do it 'converts all reference punctuation to literals' do
reference_chars = ObjectSpace.each_object(Class).map do |klass| reference_chars = Banzai::Filter::MarkdownPreEscapeFilter::REFERENCE_CHARACTERS
next unless klass.included_modules.include?(Referable) markdown = reference_chars.split('').map {|char| char.prepend("\\") }.join
next unless klass.respond_to?(:reference_prefix) punctuation = Banzai::Filter::MarkdownPreEscapeFilter::REFERENCE_CHARACTERS.split('')
next unless klass.reference_prefix.length == 1 punctuation = punctuation.delete_if {|char| char == '&' }
punctuation << '&amp;'
klass.reference_prefix result = described_class.call(markdown, project: project)
end.compact output = result[:output].to_html
reference_chars.all? do |char| punctuation.each { |char| expect(output).to include("<span>#{char}</span>") }
Banzai::Filter::MarkdownPreEscapeFilter::REFERENCE_CHARACTERS.include?(char) expect(result[:escaped_literals]).to be_truthy
end end
end
it 'does not convert non-reference punctuation to spans' do it 'ensure we handle all the GitLab reference characters', :eager_load do
markdown = %q(\"\'\*\+\,\-\.\/\:\;\<\=\>\?\[\]\_\`\{\|\}) + %q[\(\)\\\\] 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
result = described_class.call(markdown, project: project) klass.reference_prefix
output = result[:output].to_html end.compact
expect(output).not_to include('<span>') reference_chars.all? do |char|
expect(result[:escaped_literals]).to be_falsey Banzai::Filter::MarkdownPreEscapeFilter::REFERENCE_CHARACTERS.include?(char)
end end
end
it 'does not convert other characters to literals' do it 'does not convert non-reference punctuation to spans' do
markdown = %q(\→\A\a\ \3\φ\«) markdown = %q(\"\'\*\+\,\-\.\/\:\;\<\=\>\?\[\]\_\`\{\|\}) + %q[\(\)\\\\]
expected = '\→\A\a\ \3\φ\«'
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 result = described_class.call(markdown, project: project)
where(:markdown, :expected) do output = result[:output].to_html
%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
it { correct_html_included(markdown, expected) }
end
end
describe 'work in all other contexts, including URLs and link titles, link references, and info strings in fenced code blocks' do expect(output).not_to include('<span>')
let(:markdown) { %Q(``` foo\\@bar\nfoo\n```) } expect(result[:escaped_literals]).to be_falsey
it 'renders correct html' do
if Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml)
correct_html_included(markdown, %Q(<pre data-sourcepos="1:1-3:3" lang="foo@bar"><code>foo\n</code></pre>))
else
correct_html_included(markdown, %Q(<code lang="foo@bar">foo\n</code>))
end
end
where(:markdown, :expected) do
%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>)
end
with_them do
it { correct_html_included(markdown, expected) }
end
end
end end
end
describe 'backslash escapes' do
let_it_be(:project) { create(:project, :public) }
let_it_be(:issue) { create(:issue, project: project) }
def correct_html_included(markdown, expected)
result = described_class.call(markdown, {})
expect(result[:output].to_html).to include(expected) it 'does not convert other characters to literals' do
markdown = %q(\→\A\a\ \3\φ\«)
expected = '\→\A\a\ \3\φ\«'
result result = correct_html_included(markdown, expected)
expect(result[:escaped_literals]).to be_falsey
end end
context 'using ruby-based HTML renderer' do describe 'backslash escapes do not work in code blocks, code spans, autolinks, or raw HTML' do
before do where(:markdown, :expected) do
stub_feature_flags(use_cmark_renderer: false) %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 end
it_behaves_like 'renders correct markdown' with_them do
it { correct_html_included(markdown, expected) }
end
end end
context 'using c-based HTML renderer' do describe 'work in all other contexts, including URLs and link titles, link references, and info strings in fenced code blocks' do
before do let(:markdown) { %Q(``` foo\\@bar\nfoo\n```) }
stub_feature_flags(use_cmark_renderer: true)
it 'renders correct html' do
correct_html_included(markdown, %Q(<pre data-sourcepos="1:1-3:3" lang="foo@bar"><code>foo\n</code></pre>))
end
where(:markdown, :expected) do
%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>)
end end
it_behaves_like 'renders correct markdown' with_them do
it { correct_html_included(markdown, expected) }
end
end end
end end
def correct_html_included(markdown, expected)
result = described_class.call(markdown, {})
expect(result[:output].to_html).to include(expected)
result
end
end end
This diff is collapsed.
...@@ -92,12 +92,7 @@ module StubGitlabCalls ...@@ -92,12 +92,7 @@ module StubGitlabCalls
end end
def stub_commonmark_sourcepos_disabled def stub_commonmark_sourcepos_disabled
render_options = render_options = Banzai::Filter::MarkdownEngines::CommonMark::RENDER_OPTIONS
if Feature.enabled?(:use_cmark_renderer, default_enabled: :yaml)
Banzai::Filter::MarkdownEngines::CommonMark::RENDER_OPTIONS_C
else
Banzai::Filter::MarkdownEngines::CommonMark::RENDER_OPTIONS_RUBY
end
allow_any_instance_of(Banzai::Filter::MarkdownEngines::CommonMark) allow_any_instance_of(Banzai::Filter::MarkdownEngines::CommonMark)
.to receive(:render_options) .to receive(:render_options)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment