Commit f3ea06eb authored by Douwe Maan's avatar Douwe Maan
Browse files

Autolink first so we don't pick up numeric anchors as issue references.

parent 62c14ba2
......@@ -73,11 +73,8 @@ def self.reference_prefix
# This pattern supports cross-project references.
def self.reference_pattern
%r{
#{link_reference_pattern} |
(?:
(?:#{Project.reference_pattern}#{reference_prefix})?
(?<commit>\h{6,40})
)
(?:#{Project.reference_pattern}#{reference_prefix})?
(?<commit>\h{6,40})
}x
end
......
......@@ -44,11 +44,8 @@ def self.reference_prefix
# This pattern supports cross-project references.
def self.reference_pattern
%r{
#{link_reference_pattern} |
(?:
(?:#{Project.reference_pattern}#{reference_prefix})?
(?<commit_range>#{STRICT_PATTERN})
)
(?:#{Project.reference_pattern}#{reference_prefix})?
(?<commit_range>#{STRICT_PATTERN})
}x
end
......
......@@ -64,11 +64,8 @@ def self.reference_prefix
# This pattern supports cross-project references.
def self.reference_pattern
%r{
#{link_reference_pattern} |
(?:
(#{Project.reference_pattern})?
#{Regexp.escape(reference_prefix)}(?<issue>\d+)
)
(#{Project.reference_pattern})?
#{Regexp.escape(reference_prefix)}(?<issue>\d+)
}x
end
......
......@@ -146,11 +146,8 @@ def self.reference_prefix
# This pattern supports cross-project references.
def self.reference_pattern
%r{
#{link_reference_pattern} |
(?:
(#{Project.reference_pattern})?
#{Regexp.escape(reference_prefix)}(?<merge_request>\d+)
)
(#{Project.reference_pattern})?
#{Regexp.escape(reference_prefix)}(?<merge_request>\d+)
}x
end
......
......@@ -60,11 +60,8 @@ def self.reference_prefix
# This pattern supports cross-project references.
def self.reference_pattern
%r{
#{link_reference_pattern} |
(?:
(#{Project.reference_pattern})?
#{Regexp.escape(reference_prefix)}(?<snippet>\d+)
)
(#{Project.reference_pattern})?
#{Regexp.escape(reference_prefix)}(?<snippet>\d+)
}x
end
......
......@@ -2,7 +2,7 @@ module Gitlab
class ClosingIssueExtractor
ISSUE_CLOSING_REGEX = begin
pattern = Gitlab.config.gitlab.issue_closing_pattern
pattern = pattern.sub('%{issue_ref}', "(?:#{Issue.reference_pattern})")
pattern = pattern.sub('%{issue_ref}', "(?:(?:#{Issue.link_reference_pattern})|(?:#{Issue.reference_pattern}))")
Regexp.new(pattern).freeze
end
......
......@@ -181,6 +181,8 @@ def self.filters
Gitlab::Markdown::RelativeLinkFilter,
Gitlab::Markdown::EmojiFilter,
Gitlab::Markdown::TableOfContentsFilter,
Gitlab::Markdown::AutolinkFilter,
Gitlab::Markdown::ExternalLinkFilter,
Gitlab::Markdown::UserReferenceFilter,
Gitlab::Markdown::IssueReferenceFilter,
......@@ -191,9 +193,6 @@ def self.filters
Gitlab::Markdown::CommitReferenceFilter,
Gitlab::Markdown::LabelReferenceFilter,
Gitlab::Markdown::AutolinkFilter,
Gitlab::Markdown::ExternalLinkFilter,
Gitlab::Markdown::TaskListFilter
]
end
......
......@@ -37,8 +37,8 @@ def self.data_reference
# of the external project reference, and all of the matchdata.
#
# Returns a String replaced with the return of the block.
def self.references_in(text)
text.gsub(object_class.reference_pattern) do |match|
def self.references_in(text, pattern = object_class.reference_pattern)
text.gsub(pattern) do |match|
yield match, $~[object_sym].to_i, $~[:project], $~
end
end
......@@ -61,7 +61,11 @@ def url_for_object(object, project)
def call
replace_text_nodes_matching(object_class.reference_pattern) do |content|
object_link_filter(content)
object_link_filter(content, object_class.reference_pattern)
end
replace_link_nodes_matching(object_class.link_reference_pattern) do |content|
object_link_filter(content, object_class.link_reference_pattern)
end
end
......@@ -72,15 +76,17 @@ def call
#
# 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)
references_in(text) do |match, id, project_ref, matches|
def object_link_filter(text, pattern)
references_in(text, pattern) do |match, id, project_ref, matches|
project = project_from_ref(project_ref)
if project && object = find_object(project, id)
title = escape_once(object_link_title(object))
klass = reference_class(object_sym)
data = data_attribute(project: project.id, object_sym => object.id, original: match)
url = matches[:url] || url_for_object(object, project)
url = matches[:url] if matches.names.include?("url")
url ||= url_for_object(object, project)
text = object.reference_link_text(context[:project])
......@@ -99,7 +105,7 @@ def object_link_filter(text)
def object_link_text_extras(object, matches)
extras = []
if matches[:anchor] && matches[:anchor] =~ /\A\#note_(\d+)\z/
if matches.names.include?("anchor") && matches[:anchor] && matches[:anchor] =~ /\A\#note_(\d+)\z/
extras << "comment #{$1}"
end
......
......@@ -10,8 +10,8 @@ def self.object_class
CommitRange
end
def self.references_in(text)
text.gsub(CommitRange.reference_pattern) do |match|
def self.references_in(text, pattern = CommitRange.reference_pattern)
text.gsub(pattern) do |match|
yield match, $~[:commit_range], $~[:project], $~
end
end
......
......@@ -10,8 +10,8 @@ def self.object_class
Commit
end
def self.references_in(text)
text.gsub(Commit.reference_pattern) do |match|
def self.references_in(text, pattern = Commit.reference_pattern)
text.gsub(pattern) do |match|
yield match, $~[:commit], $~[:project], $~
end
end
......
......@@ -8,12 +8,9 @@ module Markdown
class ExternalLinkFilter < HTML::Pipeline::Filter
def call
doc.search('a').each do |node|
next unless node.has_attribute?('href')
link = node.attr('href')
klass = node.attribute('class')
next if klass && klass.include?('gfm')
link = node.attribute('href').value
next unless link
# Skip non-HTTP(S) links
next unless link.start_with?('http')
......
......@@ -24,7 +24,7 @@ def url_for_object(mr, project)
def object_link_text_extras(object, matches)
extras = super
if matches[:path] && matches[:path] == '/diffs'
if matches.names.include?("path") && matches[:path] && matches[:path] == '/diffs'
extras.unshift "diffs"
end
......
......@@ -14,7 +14,7 @@ def call
unless user_can_reference?(node)
# The reference should be replaced by the original text,
# which is not always the same as the rendered text.
text = node.attribute('data-original') || node.text
text = node.attr('data-original') || node.text
node.replace(text)
end
end
......
......@@ -122,6 +122,29 @@ def replace_text_nodes_matching(pattern)
doc
end
def replace_link_nodes_matching(pattern)
return doc if project.nil?
doc.search('a').each do |node|
klass = node.attr('class')
next if klass && klass.include?('gfm')
link = node.attr('href')
text = node.text
# Ignore ending punctionation like periods or commas
next unless link == text && text =~ /\A#{pattern}/
html = yield text
next if html == text
node.replace(html)
end
doc
end
# Ensure that a :project key exists in context
#
# Note that while the key might exist, its value could be nil!
......
......@@ -41,14 +41,14 @@ def references
# Returns the results Array for the requested filter type
def pipeline_result(filter_type)
return [] if @text.blank?
klass = "#{filter_type.to_s.camelize}ReferenceFilter"
filter = Gitlab::Markdown.const_get(klass)
context = {
project: project,
current_user: current_user,
# We don't actually care about the links generated
only_path: true,
ignore_blockquotes: true,
......@@ -58,7 +58,15 @@ def pipeline_result(filter_type)
reference_filter: filter
}
pipeline = HTML::Pipeline.new([filter, Gitlab::Markdown::ReferenceGathererFilter], context)
# We need to autolink first to finds links to referables, and to prevent
# numeric anchors to be parsed as issue references.
filters = [
Gitlab::Markdown::AutolinkFilter,
filter,
Gitlab::Markdown::ReferenceGathererFilter
]
pipeline = HTML::Pipeline.new(filters, context)
result = pipeline.call(@text)
values = result[:references][filter_type].uniq
......
......@@ -18,7 +18,7 @@ module Gitlab::Markdown
%w(pre code a style).each do |elem|
it "ignores valid references contained inside '#{elem}' element" do
exp = act = "<#{elem}>Commit Range #{range.to_reference}</#{elem}>"
expect(filter(act).to_html).to eq exp
expect(reference_filter(act).to_html).to eq exp
end
end
......@@ -27,14 +27,14 @@ module Gitlab::Markdown
let(:reference2) { range2.to_reference }
it 'links to a valid two-dot reference' do
doc = filter("See #{reference2}")
doc = reference_filter("See #{reference2}")
expect(doc.css('a').first.attr('href')).
to eq urls.namespace_project_compare_url(project.namespace, project, range2.to_param)
end
it 'links to a valid three-dot reference' do
doc = filter("See #{reference}")
doc = reference_filter("See #{reference}")
expect(doc.css('a').first.attr('href')).
to eq urls.namespace_project_compare_url(project.namespace, project, range.to_param)
......@@ -46,12 +46,12 @@ module Gitlab::Markdown
exp = commit1.short_id + '...' + commit2.short_id
expect(filter("See #{reference}").css('a').first.text).to eq exp
expect(filter("See #{reference2}").css('a').first.text).to eq exp
expect(reference_filter("See #{reference}").css('a').first.text).to eq exp
expect(reference_filter("See #{reference2}").css('a').first.text).to eq exp
end
it 'links with adjacent text' do
doc = filter("See (#{reference}.)")
doc = reference_filter("See (#{reference}.)")
exp = Regexp.escape(range.reference_link_text)
expect(doc.to_html).to match(/\(<a.+>#{exp}<\/a>\.\)/)
......@@ -63,21 +63,21 @@ module Gitlab::Markdown
expect(project).to receive(:valid_repo?).and_return(true)
expect(project.repository).to receive(:commit).with(commit1.id.reverse)
expect(project.repository).to receive(:commit).with(commit2.id)
expect(filter(act).to_html).to eq exp
expect(reference_filter(act).to_html).to eq exp
end
it 'includes a title attribute' do
doc = filter("See #{reference}")
doc = reference_filter("See #{reference}")
expect(doc.css('a').first.attr('title')).to eq range.reference_title
end
it 'includes default classes' do
doc = filter("See #{reference}")
doc = reference_filter("See #{reference}")
expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-commit_range'
end
it 'includes a data-project attribute' do
doc = filter("See #{reference}")
doc = reference_filter("See #{reference}")
link = doc.css('a').first
expect(link).to have_attribute('data-project')
......@@ -85,7 +85,7 @@ module Gitlab::Markdown
end
it 'includes a data-commit-range attribute' do
doc = filter("See #{reference}")
doc = reference_filter("See #{reference}")
link = doc.css('a').first
expect(link).to have_attribute('data-commit-range')
......@@ -93,7 +93,7 @@ module Gitlab::Markdown
end
it 'supports an :only_path option' do
doc = filter("See #{reference}", only_path: true)
doc = reference_filter("See #{reference}", only_path: true)
link = doc.css('a').first.attr('href')
expect(link).not_to match %r(https?://)
......@@ -116,14 +116,14 @@ module Gitlab::Markdown
end
it 'links to a valid reference' do
doc = filter("See #{reference}")
doc = reference_filter("See #{reference}")
expect(doc.css('a').first.attr('href')).
to eq urls.namespace_project_compare_url(project2.namespace, project2, range.to_param)
end
it 'links with adjacent text' do
doc = filter("Fixed (#{reference}.)")
doc = reference_filter("Fixed (#{reference}.)")
exp = Regexp.escape("#{project2.to_reference}@#{range.reference_link_text}")
expect(doc.to_html).to match(/\(<a.+>#{exp}<\/a>\.\)/)
......@@ -131,10 +131,10 @@ module Gitlab::Markdown
it 'ignores invalid commit IDs on the referenced project' do
exp = act = "Fixed #{project2.to_reference}@#{commit1.id.reverse}...#{commit2.id}"
expect(filter(act).to_html).to eq exp
expect(reference_filter(act).to_html).to eq exp
exp = act = "Fixed #{project2.to_reference}@#{commit1.id}...#{commit2.id.reverse}"
expect(filter(act).to_html).to eq exp
expect(reference_filter(act).to_html).to eq exp
end
it 'adds to the results hash' do
......@@ -154,14 +154,14 @@ module Gitlab::Markdown
end
it 'links to a valid reference' do
doc = filter("See #{reference}")
doc = reference_filter("See #{reference}")
expect(doc.css('a').first.attr('href')).
to eq reference
end
it 'links with adjacent text' do
doc = filter("Fixed (#{reference}.)")
doc = reference_filter("Fixed (#{reference}.)")
exp = Regexp.escape(range.reference_link_text(project))
expect(doc.to_html).to match(/\(<a.+>#{exp}<\/a>\.\)/)
......@@ -169,10 +169,10 @@ module Gitlab::Markdown
it 'ignores invalid commit IDs on the referenced project' do
exp = act = "Fixed #{project2.to_reference}@#{commit1.id.reverse}...#{commit2.id}"
expect(filter(act).to_html).to eq exp
expect(reference_filter(act).to_html).to eq exp
exp = act = "Fixed #{project2.to_reference}@#{commit1.id}...#{commit2.id.reverse}"
expect(filter(act).to_html).to eq exp
expect(reference_filter(act).to_html).to eq exp
end
it 'adds to the results hash' do
......
......@@ -14,7 +14,7 @@ module Gitlab::Markdown
%w(pre code a style).each do |elem|
it "ignores valid references contained inside '#{elem}' element" do
exp = act = "<#{elem}>Commit #{commit.id}</#{elem}>"
expect(filter(act).to_html).to eq exp
expect(reference_filter(act).to_html).to eq exp
end
end
......@@ -24,7 +24,7 @@ module Gitlab::Markdown
# Let's test a variety of commit SHA sizes just to be paranoid
[6, 8, 12, 18, 20, 32, 40].each do |size|
it "links to a valid reference of #{size} characters" do
doc = filter("See #{reference[0...size]}")
doc = reference_filter("See #{reference[0...size]}")
expect(doc.css('a').first.text).to eq commit.short_id
expect(doc.css('a').first.attr('href')).
......@@ -33,15 +33,15 @@ module Gitlab::Markdown
end
it 'always uses the short ID as the link text' do
doc = filter("See #{commit.id}")
doc = reference_filter("See #{commit.id}")
expect(doc.text).to eq "See #{commit.short_id}"
doc = filter("See #{commit.id[0...6]}")
doc = reference_filter("See #{commit.id[0...6]}")
expect(doc.text).to eq "See #{commit.short_id}"
end
it 'links with adjacent text' do
doc = filter("See (#{reference}.)")
doc = reference_filter("See (#{reference}.)")
expect(doc.to_html).to match(/\(<a.+>#{commit.short_id}<\/a>\.\)/)
end
......@@ -51,28 +51,28 @@ module Gitlab::Markdown
expect(project).to receive(:valid_repo?).and_return(true)
expect(project.repository).to receive(:commit).with(invalid)
expect(filter(act).to_html).to eq exp
expect(reference_filter(act).to_html).to eq exp
end
it 'includes a title attribute' do
doc = filter("See #{reference}")
doc = reference_filter("See #{reference}")
expect(doc.css('a').first.attr('title')).to eq commit.link_title
end
it 'escapes the title attribute' do
allow_any_instance_of(Commit).to receive(:title).and_return(%{"></a>whatever<a title="})
doc = filter("See #{reference}")
doc = reference_filter("See #{reference}")
expect(doc.text).to eq "See #{commit.short_id}"
end
it 'includes default classes' do
doc = filter("See #{reference}")
doc = reference_filter("See #{reference}")
expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-commit'
end
it 'includes a data-project attribute' do
doc = filter("See #{reference}")
doc = reference_filter("See #{reference}")
link = doc.css('a').first
expect(link).to have_attribute('data-project')
......@@ -80,7 +80,7 @@ module Gitlab::Markdown
end
it 'includes a data-commit attribute' do
doc = filter("See #{reference}")
doc = reference_filter("See #{reference}")
link = doc.css('a').first
expect(link).to have_attribute('data-commit')
......@@ -88,7 +88,7 @@ module Gitlab::Markdown
end
it 'supports an :only_path context' do
doc = filter("See #{reference}", only_path: true)
doc = reference_filter("See #{reference}", only_path: true)
link = doc.css('a').first.attr('href')
expect(link).not_to match %r(https?://)
......@@ -108,14 +108,14 @@ module Gitlab::Markdown
let(:reference) { commit.to_reference(project) }
it 'links to a valid reference' do
doc = filter("See #{reference}")
doc = reference_filter("See #{reference}")
expect(doc.css('a').first.attr('href')).
to eq urls.namespace_project_commit_url(project2.namespace, project2, commit.id)
end
it 'links with adjacent text' do
doc = filter("Fixed (#{reference}.)")
doc = reference_filter("Fixed (#{reference}.)")
exp = Regexp.escape(project2.to_reference)
expect(doc.to_html).to match(/\(<a.+>#{exp}@#{commit.short_id}<\/a>\.\)/)
......@@ -123,7 +123,7 @@ module Gitlab::Markdown
it 'ignores invalid commit IDs on the referenced project' do
exp = act = "Committed #{invalidate_reference(reference)}"
expect(filter(act).to_html).to eq exp
expect(reference_filter(act).to_html).to eq exp
end
it 'adds to the results hash' do
......@@ -139,21 +139,21 @@ module Gitlab::Markdown
let(:reference) { urls.namespace_project_commit_url(project2.namespace, project2, commit.id) }
it 'links to a valid reference' do
doc = filter("See #{reference}")
doc = reference_filter("See #{reference}")
expect(doc.css('a').first.attr('href')).
to eq urls.namespace_project_commit_url(project2.namespace, project2, commit.id)
end
it 'links with adjacent text' do
doc = filter("Fixed (#{reference}.)")
doc = reference_filter("Fixed (#{reference}.)")
expect(doc.to_html).to match(/\(<a.+>#{commit.reference_link_text(project)}<\/a>\.\)/)
end
it 'ignores invalid commit IDs on the referenced project' do
exp = act = "Committed #{invalidate_reference(reference)}"
expect(filter(act).to_html).to eq exp
expect(reference_filter(act).to_html).to match(/<a.+>#{Regexp.escape(invalidate_reference(reference))}<\/a>/)
end
it 'adds to the results hash' do
......
......@@ -18,7 +18,7 @@ def helper
%w(pre code a style).each do |elem|
it "ignores valid references contained inside '#{elem}' element" do
exp = act = "<#{elem}>Issue #{issue.to_reference}</#{elem}>"
expect(filter(act).to_html).to eq exp
expect(reference_filter(act).to_html).to eq exp
end
end
......@@ -29,18 +29,18 @@ def helper
expect(project).to receive(:get_issue).with(issue.iid).and_return(nil)
exp = act = "Issue #{reference}"
expect(filter(act).to_html).to eq exp
expect(reference_filter(act).to_html).to eq exp
end
it 'links to a valid reference' do
doc = filter("Fixed #{reference}")
doc = reference_filter("Fixed #{reference}")
expect(doc.css('a').first.attr('href')).
to eq helper.url_for_issue(issue.iid, project)
end
it 'links with adjacent text' do
doc = filter("Fixed (#{reference}.)")
doc = reference_filter("Fixed (#{reference}.)")