From fe78984f2045a79554ae52478d01d9102c6b6a77 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 2 Jun 2015 13:17:11 +0200 Subject: [PATCH 1/5] Actually ignore references in code blocks etc. --- app/helpers/gitlab_markdown_helper.rb | 27 +++++----- lib/gitlab/reference_extractor.rb | 59 ++++++++------------- lib/redcarpet/render/gitlab_html.rb | 2 + spec/lib/gitlab/reference_extractor_spec.rb | 20 +++++++ 4 files changed, 55 insertions(+), 53 deletions(-) diff --git a/app/helpers/gitlab_markdown_helper.rb b/app/helpers/gitlab_markdown_helper.rb index 3c207619adf..2777944fc9d 100644 --- a/app/helpers/gitlab_markdown_helper.rb +++ b/app/helpers/gitlab_markdown_helper.rb @@ -41,29 +41,26 @@ module GitlabMarkdownHelper fragment.to_html.html_safe end + MARKDOWN_OPTIONS = { + no_intra_emphasis: true, + tables: true, + fenced_code_blocks: true, + strikethrough: true, + lax_spacing: true, + space_after_headers: true, + superscript: true, + footnotes: true + }.freeze + def markdown(text, options={}) unless @markdown && options == @options @options = options - options.merge!( - # Handled further down the line by Gitlab::Markdown::SanitizationFilter - escape_html: false - ) - # see https://github.com/vmg/redcarpet#darling-i-packed-you-a-couple-renderers-for-lunch rend = Redcarpet::Render::GitlabHTML.new(self, user_color_scheme_class, options) # see https://github.com/vmg/redcarpet#and-its-like-really-simple-to-use - @markdown = Redcarpet::Markdown.new(rend, - no_intra_emphasis: true, - tables: true, - fenced_code_blocks: true, - strikethrough: true, - lax_spacing: true, - space_after_headers: true, - superscript: true, - footnotes: true - ) + @markdown = Redcarpet::Markdown.new(rend, MARKDOWN_OPTIONS) end @markdown.render(text).html_safe diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb index e35f848fa6e..80b8ab8cbcc 100644 --- a/lib/gitlab/reference_extractor.rb +++ b/lib/gitlab/reference_extractor.rb @@ -1,7 +1,7 @@ module Gitlab # Extract possible GFM references from an arbitrary String for further processing. class ReferenceExtractor - attr_accessor :project, :current_user, :references + attr_accessor :project, :current_user def initialize(project, current_user = nil) @project = project @@ -9,48 +9,31 @@ module Gitlab end def analyze(text) - @_text = text.dup + references.clear + @text = markdown.render(text.dup) end - def users - result = pipeline_result(:user) - result.uniq - end - - def labels - result = pipeline_result(:label) - result.uniq - end - - def issues - # TODO (rspeicher): What about external issues? - - result = pipeline_result(:issue) - result.uniq - end - - def merge_requests - result = pipeline_result(:merge_request) - result.uniq - end - - def snippets - result = pipeline_result(:snippet) - result.uniq - end - - def commits - result = pipeline_result(:commit) - result.uniq - end - - def commit_ranges - result = pipeline_result(:commit_range) - result.uniq + %i(user label issue merge_request snippet commit commit_range).each do |type| + define_method("#{type}s") do + references[type] + end end private + def markdown + @markdown ||= Redcarpet::Markdown.new(Redcarpet::Render::HTML, GitlabMarkdownHelper::MARKDOWN_OPTIONS) + end + + def references + @references ||= Hash.new do |references, type| + type = type.to_sym + return references[type] if references.has_key?(type) + + references[type] = pipeline_result(type).uniq + end + end + # Instantiate and call HTML::Pipeline with a single reference filter type, # returning the result # @@ -69,7 +52,7 @@ module Gitlab } pipeline = HTML::Pipeline.new([filter], context) - result = pipeline.call(@_text) + result = pipeline.call(@text) result[:references][filter_type] end diff --git a/lib/redcarpet/render/gitlab_html.rb b/lib/redcarpet/render/gitlab_html.rb index 7dcecc2ecf6..133798852ee 100644 --- a/lib/redcarpet/render/gitlab_html.rb +++ b/lib/redcarpet/render/gitlab_html.rb @@ -10,6 +10,8 @@ class Redcarpet::Render::GitlabHTML < Redcarpet::Render::HTML @options = options.dup @options.reverse_merge!( + # Handled further down the line by Gitlab::Markdown::SanitizationFilter + escape_html: false project: @template.instance_variable_get("@project") ) diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb index c14f4ac6bf6..951e738cb67 100644 --- a/spec/lib/gitlab/reference_extractor_spec.rb +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -16,6 +16,26 @@ describe Gitlab::ReferenceExtractor do expect(subject.users).to eq([@u_foo, @u_bar, @u_offteam]) end + it 'ignores user mentions inside specific elements' do + @u_foo = create(:user, username: 'foo') + @u_bar = create(:user, username: 'bar') + @u_offteam = create(:user, username: 'offteam') + + project.team << [@u_foo, :reporter] + project.team << [@u_bar, :guest] + + subject.analyze(%Q{ + Inline code: `@foo` + + Code block: + + ``` + @bar + ``` + }) + expect(subject.users).to eq([]) + end + it 'accesses valid issue objects' do @i0 = create(:issue, project: project) @i1 = create(:issue, project: project) From 94919c7ef6cf5786d380ae65623de0697eff9188 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 2 Jun 2015 13:17:21 +0200 Subject: [PATCH 2/5] Ignore references in blockquotes. --- lib/gitlab/markdown/reference_filter.rb | 14 ++++++++++---- lib/gitlab/reference_extractor.rb | 3 ++- spec/lib/gitlab/reference_extractor_spec.rb | 4 ++++ 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/lib/gitlab/markdown/reference_filter.rb b/lib/gitlab/markdown/reference_filter.rb index be4d26af0fc..a84bacd3d4f 100644 --- a/lib/gitlab/markdown/reference_filter.rb +++ b/lib/gitlab/markdown/reference_filter.rb @@ -25,12 +25,18 @@ module Gitlab ERB::Util.html_escape_once(html) end - # Don't look for references in text nodes that are children of these - # elements. - IGNORE_PARENTS = %w(pre code a style).to_set + def ignore_parents + @ignore_parents ||= begin + # Don't look for references in text nodes that are children of these + # elements. + parents = %w(pre code a style) + parents << 'blockquote' if context[:ignore_blockquotes] + parents.to_set + end + end def ignored_ancestry?(node) - has_ancestor?(node, IGNORE_PARENTS) + has_ancestor?(node, ignore_parents) end def project diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb index 80b8ab8cbcc..e836b05ff25 100644 --- a/lib/gitlab/reference_extractor.rb +++ b/lib/gitlab/reference_extractor.rb @@ -48,7 +48,8 @@ module Gitlab project: project, current_user: current_user, # We don't actually care about the links generated - only_path: true + only_path: true, + ignore_blockquotes: true } pipeline = HTML::Pipeline.new([filter], context) diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb index 951e738cb67..f921dd9cc09 100644 --- a/spec/lib/gitlab/reference_extractor_spec.rb +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -32,6 +32,10 @@ describe Gitlab::ReferenceExtractor do ``` @bar ``` + + Quote: + + > @offteam }) expect(subject.users).to eq([]) end From 1f908dc48176d4f6f5e5d9c6709b137288ce2548 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 2 Jun 2015 13:21:34 +0200 Subject: [PATCH 3/5] Fix typo. --- lib/redcarpet/render/gitlab_html.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/redcarpet/render/gitlab_html.rb b/lib/redcarpet/render/gitlab_html.rb index 133798852ee..2f7aff03c2a 100644 --- a/lib/redcarpet/render/gitlab_html.rb +++ b/lib/redcarpet/render/gitlab_html.rb @@ -11,7 +11,7 @@ class Redcarpet::Render::GitlabHTML < Redcarpet::Render::HTML @options.reverse_merge!( # Handled further down the line by Gitlab::Markdown::SanitizationFilter - escape_html: false + escape_html: false, project: @template.instance_variable_get("@project") ) From 156c43c0dcdaad0eb3a351dfb8b62e600b7d9a08 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 2 Jun 2015 13:23:22 +0200 Subject: [PATCH 4/5] Add changelog entry. --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 870ab59afa5..0d65bb345e1 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.12.0 (unreleased) + - Don't notify users mentioned in code blocks or blockquotes. - Disable changing of the source branch in merge request update API (Stan Hu) - Shorten merge request WIP text. - Add option to disallow users from registering any application to use GitLab as an OAuth provider From a916936f3feeda0a6d58fef2c06c51f95f10c45a Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 2 Jun 2015 15:00:51 +0200 Subject: [PATCH 5/5] Fix spec. --- spec/support/mentionable_shared_examples.rb | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb index ede62e8f37a..d29c8a55c82 100644 --- a/spec/support/mentionable_shared_examples.rb +++ b/spec/support/mentionable_shared_examples.rb @@ -107,17 +107,26 @@ shared_examples 'an editable mentionable' do it 'creates new cross-reference notes when the mentionable text is edited' do subject.save - new_text = <<-MSG + new_text = <<-MSG.strip_heredoc These references already existed: - Issue: #{mentioned_issue.to_reference} - Commit: #{mentioned_commit.to_reference} + + Issue: #{mentioned_issue.to_reference} + + Commit: #{mentioned_commit.to_reference} + + --- This cross-project reference already existed: - Issue: #{ext_issue.to_reference(project)} + + Issue: #{ext_issue.to_reference(project)} + + --- These two references are introduced in an edit: - Issue: #{new_issues[0].to_reference} - Cross: #{new_issues[1].to_reference(project)} + + Issue: #{new_issues[0].to_reference} + + Cross: #{new_issues[1].to_reference(project)} MSG # These three objects were already referenced, and should not receive new