diff --git a/CHANGELOG.md b/CHANGELOG.md
index 14907e1546e3d520388bc1ef7ed1b75e029a03c2..acf97249265c2ac535bfa29254bc2d692265c097 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,7 @@ Please view this file on the master branch, on stable branches it's out of date.
- Trim leading and trailing whitespace on project_path (Linus Thiel)
- Prevent award emoji via notes for issues/MRs authored by user (barthc)
- Adds an optional path parameter to the Commits API to filter commits by path (Luis HGO)
+- Fix Markdown styling inside reference links (Jan Zdráhal)
- Fix extra space on Build sidebar on Firefox !7060
- Fix mobile layout issues in admin user overview page !7087
- Fix HipChat notifications rendering (airatshigapov, eisnerd)
diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb
index cb213a76a054a1cb9bff4ecfbf260a47ea1a924d..3740d4fb4cd598b047d661126c42d66fbad6114a 100644
--- a/lib/banzai/filter/abstract_reference_filter.rb
+++ b/lib/banzai/filter/abstract_reference_filter.rb
@@ -102,10 +102,10 @@ def call
end
elsif element_node?(node)
- yield_valid_link(node) do |link, text|
+ yield_valid_link(node) do |link, inner_html|
if ref_pattern && link =~ /\A#{ref_pattern}\z/
replace_link_node_with_href(node, link) do
- object_link_filter(link, ref_pattern, link_text: text)
+ object_link_filter(link, ref_pattern, link_content: inner_html)
end
next
@@ -113,9 +113,9 @@ def call
next unless link_pattern
- if link == text && text =~ /\A#{link_pattern}/
+ if link == inner_html && inner_html =~ /\A#{link_pattern}/
replace_link_node_with_text(node, link) do
- object_link_filter(text, link_pattern)
+ object_link_filter(inner_html, link_pattern)
end
next
@@ -123,7 +123,7 @@ def call
if link =~ /\A#{link_pattern}\z/
replace_link_node_with_href(node, link) do
- object_link_filter(link, link_pattern, link_text: text)
+ object_link_filter(link, link_pattern, link_content: inner_html)
end
next
@@ -140,11 +140,11 @@ def call
#
# text - String text to replace references in.
# pattern - Reference pattern to match against.
- # link_text - Original content of the link being replaced.
+ # link_content - Original content of the link being replaced.
#
# Returns a String with references replaced with links. All links
# have `gfm` and `gfm-OBJECT_NAME` class names attached for styling.
- def object_link_filter(text, pattern, link_text: nil)
+ def object_link_filter(text, pattern, link_content: nil)
references_in(text, pattern) do |match, id, project_ref, matches|
project = project_from_ref_cached(project_ref)
@@ -152,7 +152,7 @@ def object_link_filter(text, pattern, link_text: nil)
title = object_link_title(object)
klass = reference_class(object_sym)
- data = data_attributes_for(link_text || match, project, object)
+ data = data_attributes_for(link_content || match, project, object)
if matches.names.include?("url") && matches[:url]
url = matches[:url]
@@ -160,11 +160,11 @@ def object_link_filter(text, pattern, link_text: nil)
url = url_for_object_cached(object, project)
end
- text = link_text || object_link_text(object, matches)
+ content = link_content || object_link_text(object, matches)
%(<a href="#{url}" #{data}
title="#{escape_once(title)}"
- class="#{klass}">#{escape_once(text)}</a>)
+ class="#{klass}">#{content}</a>)
else
match
end
diff --git a/lib/banzai/filter/external_issue_reference_filter.rb b/lib/banzai/filter/external_issue_reference_filter.rb
index 0d20be557a04f989f2c19856a1c77e6ae9f1e027..dce4de3ceaf800acb5dc6d1237f8646796046d70 100644
--- a/lib/banzai/filter/external_issue_reference_filter.rb
+++ b/lib/banzai/filter/external_issue_reference_filter.rb
@@ -37,10 +37,10 @@ def call
end
elsif element_node?(node)
- yield_valid_link(node) do |link, text|
+ yield_valid_link(node) do |link, inner_html|
if link =~ ref_start_pattern
replace_link_node_with_href(node, link) do
- issue_link_filter(link, link_text: text)
+ issue_link_filter(link, link_content: inner_html)
end
end
end
@@ -54,10 +54,11 @@ def call
# issue's details page.
#
# text - String text to replace references in.
+ # link_content - Original content of the link being replaced.
#
# Returns a String with `JIRA-123` references replaced with links. All
# links have `gfm` and `gfm-issue` class names attached for styling.
- def issue_link_filter(text, link_text: nil)
+ def issue_link_filter(text, link_content: nil)
project = context[:project]
self.class.references_in(text, issue_reference_pattern) do |match, id|
@@ -69,11 +70,11 @@ def issue_link_filter(text, link_text: nil)
klass = reference_class(:issue)
data = data_attribute(project: project.id, external_issue: id)
- text = link_text || match
+ content = link_content || match
%(<a href="#{url}" #{data}
title="#{escape_once(title)}"
- class="#{klass}">#{escape_once(text)}</a>)
+ class="#{klass}">#{content}</a>)
end
end
diff --git a/lib/banzai/filter/reference_filter.rb b/lib/banzai/filter/reference_filter.rb
index 2d221290f7eb0e08c3cf01145ebb5e7742173890..84bfeac80417a21ef7fcbcec13f9b1a5bfaafdf3 100644
--- a/lib/banzai/filter/reference_filter.rb
+++ b/lib/banzai/filter/reference_filter.rb
@@ -85,14 +85,14 @@ def nodes
@nodes ||= each_node.to_a
end
- # Yields the link's URL and text whenever the node is a valid <a> tag.
+ # Yields the link's URL and inner HTML whenever the node is a valid <a> tag.
def yield_valid_link(node)
link = CGI.unescape(node.attr('href').to_s)
- text = node.text
+ inner_html = node.inner_html
return unless link.force_encoding('UTF-8').valid_encoding?
- yield link, text
+ yield link, inner_html
end
def replace_text_when_pattern_matches(node, pattern)
diff --git a/lib/banzai/filter/user_reference_filter.rb b/lib/banzai/filter/user_reference_filter.rb
index c6302b586d31358efb8b24c06bab67bcc833445a..f842b1fb779d7b79a9ade9f80814b7eb54ed3847 100644
--- a/lib/banzai/filter/user_reference_filter.rb
+++ b/lib/banzai/filter/user_reference_filter.rb
@@ -35,10 +35,10 @@ def call
user_link_filter(content)
end
elsif element_node?(node)
- yield_valid_link(node) do |link, text|
+ yield_valid_link(node) do |link, inner_html|
if link =~ ref_pattern_start
replace_link_node_with_href(node, link) do
- user_link_filter(link, link_text: text)
+ user_link_filter(link, link_content: inner_html)
end
end
end
@@ -52,15 +52,16 @@ def call
# user's profile page.
#
# text - String text to replace references in.
+ # link_content - Original content of the link being replaced.
#
# Returns a String with `@user` references replaced with links. All links
# have `gfm` and `gfm-project_member` class names attached for styling.
- def user_link_filter(text, link_text: nil)
+ def user_link_filter(text, link_content: nil)
self.class.references_in(text) do |match, username|
if username == 'all'
- link_to_all(link_text: link_text)
+ link_to_all(link_content: link_content)
elsif namespace = namespaces[username]
- link_to_namespace(namespace, link_text: link_text) || match
+ link_to_namespace(namespace, link_content: link_content) || match
else
match
end
@@ -102,49 +103,49 @@ def link_class
reference_class(:project_member)
end
- def link_to_all(link_text: nil)
+ def link_to_all(link_content: nil)
project = context[:project]
author = context[:author]
if author && !project.team.member?(author)
- link_text
+ link_content
else
url = urls.namespace_project_url(project.namespace, project,
only_path: context[:only_path])
data = data_attribute(project: project.id, author: author.try(:id))
- text = link_text || User.reference_prefix + 'all'
+ content = link_content || User.reference_prefix + 'all'
- link_tag(url, data, text, 'All Project and Group Members')
+ link_tag(url, data, content, 'All Project and Group Members')
end
end
- def link_to_namespace(namespace, link_text: nil)
+ def link_to_namespace(namespace, link_content: nil)
if namespace.is_a?(Group)
- link_to_group(namespace.path, namespace, link_text: link_text)
+ link_to_group(namespace.path, namespace, link_content: link_content)
else
- link_to_user(namespace.path, namespace, link_text: link_text)
+ link_to_user(namespace.path, namespace, link_content: link_content)
end
end
- def link_to_group(group, namespace, link_text: nil)
+ def link_to_group(group, namespace, link_content: nil)
url = urls.group_url(group, only_path: context[:only_path])
data = data_attribute(group: namespace.id)
- text = link_text || Group.reference_prefix + group
+ content = link_content || Group.reference_prefix + group
- link_tag(url, data, text, namespace.name)
+ link_tag(url, data, content, namespace.name)
end
- def link_to_user(user, namespace, link_text: nil)
+ def link_to_user(user, namespace, link_content: nil)
url = urls.user_url(user, only_path: context[:only_path])
data = data_attribute(user: namespace.owner_id)
- text = link_text || User.reference_prefix + user
+ content = link_content || User.reference_prefix + user
- link_tag(url, data, text, namespace.owner_name)
+ link_tag(url, data, content, namespace.owner_name)
end
- def link_tag(url, data, text, title)
- %(<a href="#{url}" #{data} class="#{link_class}" title="#{escape_once(title)}">#{escape_once(text)}</a>)
+ def link_tag(url, data, link_content, title)
+ %(<a href="#{url}" #{data} class="#{link_class}" title="#{escape_once(title)}">#{link_content}</a>)
end
end
end
diff --git a/lib/banzai/redactor.rb b/lib/banzai/redactor.rb
index 0df3a72d1c46d3395ea7d9744cf5c93a5c4d004a..de3ebe72720e698849ef1a8d99bdf49e374b6cc5 100644
--- a/lib/banzai/redactor.rb
+++ b/lib/banzai/redactor.rb
@@ -41,10 +41,10 @@ def redact_document_nodes(all_document_nodes)
next if visible.include?(node)
doc_data[:visible_reference_count] -= 1
- # The reference should be replaced by the original text,
- # which is not always the same as the rendered text.
- text = node.attr('data-original') || node.text
- node.replace(text)
+ # The reference should be replaced by the original link's content,
+ # which is not always the same as the rendered one.
+ content = node.attr('data-original') || node.inner_html
+ node.replace(content)
end
end
diff --git a/spec/lib/banzai/filter/external_issue_reference_filter_spec.rb b/spec/lib/banzai/filter/external_issue_reference_filter_spec.rb
index 2f9343fadaff067ba40c74f1e6fd670863f6c988..fbf7a461fa5d55f8955118666b0d9e75483514ac 100644
--- a/spec/lib/banzai/filter/external_issue_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/external_issue_reference_filter_spec.rb
@@ -8,6 +8,8 @@ def helper
end
shared_examples_for "external issue tracker" do
+ it_behaves_like 'a reference containing an element node'
+
it 'requires project context' do
expect { described_class.call('') }.to raise_error(ArgumentError, /:project/)
end
diff --git a/spec/lib/banzai/filter/issue_reference_filter_spec.rb b/spec/lib/banzai/filter/issue_reference_filter_spec.rb
index a2025672ad9d6d9b276a2a873bce8e875f1be4d1..8f0b2db3e8e820755b88343884e8e319d651508b 100644
--- a/spec/lib/banzai/filter/issue_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/issue_reference_filter_spec.rb
@@ -22,6 +22,8 @@ def helper
end
context 'internal reference' do
+ it_behaves_like 'a reference containing an element node'
+
let(:reference) { issue.to_reference }
it 'ignores valid references when using non-default tracker' do
@@ -83,6 +85,20 @@ def helper
expect(link.attr('data-issue')).to eq issue.id.to_s
end
+ it 'includes a data-original attribute' do
+ doc = reference_filter("See #{reference}")
+ link = doc.css('a').first
+
+ expect(link).to have_attribute('data-original')
+ expect(link.attr('data-original')).to eq reference
+ end
+
+ it 'does not escape the data-original attribute' do
+ inner_html = 'element <code>node</code> inside'
+ doc = reference_filter(%{<a href="#{reference}">#{inner_html}</a>})
+ expect(doc.children.first.attr('data-original')).to eq inner_html
+ end
+
it 'supports an :only_path context' do
doc = reference_filter("Issue #{reference}", only_path: true)
link = doc.css('a').first.attr('href')
@@ -101,6 +117,8 @@ def helper
end
context 'cross-project reference' do
+ it_behaves_like 'a reference containing an element node'
+
let(:namespace) { create(:namespace, name: 'cross-reference') }
let(:project2) { create(:empty_project, :public, namespace: namespace) }
let(:issue) { create(:issue, project: project2) }
@@ -141,6 +159,8 @@ def helper
end
context 'cross-project URL reference' do
+ it_behaves_like 'a reference containing an element node'
+
let(:namespace) { create(:namespace, name: 'cross-reference') }
let(:project2) { create(:empty_project, :public, namespace: namespace) }
let(:issue) { create(:issue, project: project2) }
@@ -160,39 +180,45 @@ def helper
end
context 'cross-project reference in link href' do
+ it_behaves_like 'a reference containing an element node'
+
let(:namespace) { create(:namespace, name: 'cross-reference') }
let(:project2) { create(:empty_project, :public, namespace: namespace) }
let(:issue) { create(:issue, project: project2) }
- let(:reference) { %Q{<a href="#{issue.to_reference(project)}">Reference</a>} }
+ let(:reference) { issue.to_reference(project) }
+ let(:reference_link) { %{<a href="#{reference}">Reference</a>} }
it 'links to a valid reference' do
- doc = reference_filter("See #{reference}")
+ doc = reference_filter("See #{reference_link}")
expect(doc.css('a').first.attr('href')).
to eq helper.url_for_issue(issue.iid, project2)
end
it 'links with adjacent text' do
- doc = reference_filter("Fixed (#{reference}.)")
+ doc = reference_filter("Fixed (#{reference_link}.)")
expect(doc.to_html).to match(/\(<a.+>Reference<\/a>\.\)/)
end
end
context 'cross-project URL in link href' do
+ it_behaves_like 'a reference containing an element node'
+
let(:namespace) { create(:namespace, name: 'cross-reference') }
let(:project2) { create(:empty_project, :public, namespace: namespace) }
let(:issue) { create(:issue, project: project2) }
- let(:reference) { %Q{<a href="#{helper.url_for_issue(issue.iid, project2) + "#note_123"}">Reference</a>} }
+ let(:reference) { "#{helper.url_for_issue(issue.iid, project2) + "#note_123"}" }
+ let(:reference_link) { %{<a href="#{reference}">Reference</a>} }
it 'links to a valid reference' do
- doc = reference_filter("See #{reference}")
+ doc = reference_filter("See #{reference_link}")
expect(doc.css('a').first.attr('href')).
to eq helper.url_for_issue(issue.iid, project2) + "#note_123"
end
it 'links with adjacent text' do
- doc = reference_filter("Fixed (#{reference}.)")
+ doc = reference_filter("Fixed (#{reference_link}.)")
expect(doc.to_html).to match(/\(<a.+>Reference<\/a>\.\)/)
end
end
diff --git a/spec/lib/banzai/filter/user_reference_filter_spec.rb b/spec/lib/banzai/filter/user_reference_filter_spec.rb
index 729e77fd43fc28c4feadef6f5bea393831d94159..5bfeb82e738c41e87787eccce894d06c8e2844b8 100644
--- a/spec/lib/banzai/filter/user_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/user_reference_filter_spec.rb
@@ -24,6 +24,8 @@
end
context 'mentioning @all' do
+ it_behaves_like 'a reference containing an element node'
+
let(:reference) { User.reference_prefix + 'all' }
before do
@@ -60,6 +62,8 @@
end
context 'mentioning a user' do
+ it_behaves_like 'a reference containing an element node'
+
it 'links to a User' do
doc = reference_filter("Hey #{reference}")
expect(doc.css('a').first.attr('href')).to eq urls.user_url(user)
@@ -89,6 +93,8 @@
end
context 'mentioning a group' do
+ it_behaves_like 'a reference containing an element node'
+
let(:group) { create(:group) }
let(:reference) { group.to_reference }
diff --git a/spec/lib/banzai/pipeline/full_pipeline_spec.rb b/spec/lib/banzai/pipeline/full_pipeline_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..2501b638774789070bb9eee3805c678b4e43ee19
--- /dev/null
+++ b/spec/lib/banzai/pipeline/full_pipeline_spec.rb
@@ -0,0 +1,28 @@
+require 'rails_helper'
+
+describe Banzai::Pipeline::FullPipeline do
+ describe 'References' do
+ let(:project) { create(:empty_project, :public) }
+ let(:issue) { create(:issue, project: project) }
+
+ it 'handles markdown inside a reference' do
+ markdown = "[some `code` inside](#{issue.to_reference})"
+ result = described_class.call(markdown, project: project)
+ link_content = result[:output].css('a').inner_html
+ expect(link_content).to eq('some <code>code</code> inside')
+ end
+
+ it 'sanitizes reference HTML' do
+ link_label = '<script>bad things</script>'
+ markdown = "[#{link_label}](#{issue.to_reference})"
+ result = described_class.to_html(markdown, project: project)
+ expect(result).not_to include(link_label)
+ end
+
+ it 'escapes the data-original attribute on a reference' do
+ markdown = %Q{[">bad things](#{issue.to_reference})}
+ result = described_class.to_html(markdown, project: project)
+ expect(result).to include(%{data-original='\">bad things'})
+ end
+ end
+end
diff --git a/spec/lib/banzai/redactor_spec.rb b/spec/lib/banzai/redactor_spec.rb
index 254657a881da3a40a038dbca0adfe06e85e44c9b..6d2c141e18b2bde4978b731bc88aaa598042a408 100644
--- a/spec/lib/banzai/redactor_spec.rb
+++ b/spec/lib/banzai/redactor_spec.rb
@@ -6,39 +6,60 @@
let(:redactor) { described_class.new(project, user) }
describe '#redact' do
- it 'redacts an Array of documents' do
- doc1 = Nokogiri::HTML.
- fragment('<a class="gfm" data-reference-type="issue">foo</a>')
-
- doc2 = Nokogiri::HTML.
- fragment('<a class="gfm" data-reference-type="issue">bar</a>')
-
- expect(redactor).to receive(:nodes_visible_to_user).and_return([])
-
- redacted_data = redactor.redact([doc1, doc2])
-
- expect(redacted_data.map { |data| data[:document] }).to eq([doc1, doc2])
- expect(redacted_data.map { |data| data[:visible_reference_count] }).to eq([0, 0])
- expect(doc1.to_html).to eq('foo')
- expect(doc2.to_html).to eq('bar')
+ context 'when reference not visible to user' do
+ before do
+ expect(redactor).to receive(:nodes_visible_to_user).and_return([])
+ end
+
+ it 'redacts an array of documents' do
+ doc1 = Nokogiri::HTML.
+ fragment('<a class="gfm" data-reference-type="issue">foo</a>')
+
+ doc2 = Nokogiri::HTML.
+ fragment('<a class="gfm" data-reference-type="issue">bar</a>')
+
+ redacted_data = redactor.redact([doc1, doc2])
+
+ expect(redacted_data.map { |data| data[:document] }).to eq([doc1, doc2])
+ expect(redacted_data.map { |data| data[:visible_reference_count] }).to eq([0, 0])
+ expect(doc1.to_html).to eq('foo')
+ expect(doc2.to_html).to eq('bar')
+ end
+
+ it 'replaces redacted reference with inner HTML' do
+ doc = Nokogiri::HTML.fragment("<a class='gfm' data-reference-type='issue'>foo</a>")
+ redactor.redact([doc])
+ expect(doc.to_html).to eq('foo')
+ end
+
+ context 'when data-original attribute provided' do
+ let(:original_content) { '<code>foo</code>' }
+ it 'replaces redacted reference with original content' do
+ doc = Nokogiri::HTML.fragment("<a class='gfm' data-reference-type='issue' data-original='#{original_content}'>bar</a>")
+ redactor.redact([doc])
+ expect(doc.to_html).to eq(original_content)
+ end
+ end
end
- it 'does not redact an Array of documents' do
- doc1_html = '<a class="gfm" data-reference-type="issue">foo</a>'
- doc1 = Nokogiri::HTML.fragment(doc1_html)
+ context 'when reference visible to user' do
+ it 'does not redact an array of documents' do
+ doc1_html = '<a class="gfm" data-reference-type="issue">foo</a>'
+ doc1 = Nokogiri::HTML.fragment(doc1_html)
- doc2_html = '<a class="gfm" data-reference-type="issue">bar</a>'
- doc2 = Nokogiri::HTML.fragment(doc2_html)
+ doc2_html = '<a class="gfm" data-reference-type="issue">bar</a>'
+ doc2 = Nokogiri::HTML.fragment(doc2_html)
- nodes = redactor.document_nodes([doc1, doc2]).map { |x| x[:nodes] }
- expect(redactor).to receive(:nodes_visible_to_user).and_return(nodes.flatten)
+ nodes = redactor.document_nodes([doc1, doc2]).map { |x| x[:nodes] }
+ expect(redactor).to receive(:nodes_visible_to_user).and_return(nodes.flatten)
- redacted_data = redactor.redact([doc1, doc2])
+ redacted_data = redactor.redact([doc1, doc2])
- expect(redacted_data.map { |data| data[:document] }).to eq([doc1, doc2])
- expect(redacted_data.map { |data| data[:visible_reference_count] }).to eq([1, 1])
- expect(doc1.to_html).to eq(doc1_html)
- expect(doc2.to_html).to eq(doc2_html)
+ expect(redacted_data.map { |data| data[:document] }).to eq([doc1, doc2])
+ expect(redacted_data.map { |data| data[:visible_reference_count] }).to eq([1, 1])
+ expect(doc1.to_html).to eq(doc1_html)
+ expect(doc2.to_html).to eq(doc2_html)
+ end
end
end
diff --git a/spec/support/banzai/reference_filter_shared_examples.rb b/spec/support/banzai/reference_filter_shared_examples.rb
new file mode 100644
index 0000000000000000000000000000000000000000..eb5da662ab5f9c26aee739fcf971337ee818476b
--- /dev/null
+++ b/spec/support/banzai/reference_filter_shared_examples.rb
@@ -0,0 +1,13 @@
+# Specs for reference links containing HTML.
+#
+# Requires a reference:
+# let(:reference) { '#42' }
+shared_examples 'a reference containing an element node' do
+ let(:inner_html) { 'element <code>node</code> inside' }
+ let(:reference_with_element) { %{<a href="#{reference}">#{inner_html}</a>} }
+
+ it 'does not escape inner html' do
+ doc = reference_filter(reference_with_element)
+ expect(doc.children.first.inner_html).to eq(inner_html)
+ end
+end