Add banzai filter to detect commit message trailers and properly link the users

app/models/commit.rb

@@ -32,7 +32,8 @@ class Commit
   COMMIT_SHA_PATTERN = /\h{#{MIN_SHA_LENGTH},40}/.freeze
 
   def banzai_render_context(field)
-    context = { pipeline: :single_line, project: self.project }
+    pipeline = field == :description ? :commit_description : :single_line
+    context = { pipeline: pipeline, project: self.project }
     context[:author] = self.author if self.author
 
     context

app/views/projects/commit/_commit_box.html.haml

@@ -49,10 +49,10 @@
 
 .commit-box{ data: { project_path: project_path(@project) } }
   %h3.commit-title
-    = markdown(@commit.title, pipeline: :single_line, author: @commit.author)
+    = markdown_field(@commit, :title)
   - if @commit.description.present?
     %pre.commit-description
-      = preserve(markdown(@commit.description, pipeline: :single_line, author: @commit.author))
+      = preserve(markdown_field(@commit, :description))
 
 .info-well
   .well-segment.branch-info

app/views/projects/commits/_commit.atom.builder

@@ -10,5 +10,5 @@ xml.entry do
     xml.email commit.author_email
   end
 
-  xml.summary markdown(commit.description, pipeline: :single_line), type: 'html'
+  xml.summary markdown_field(commit, :description), type: 'html'
 end

changelogs/unreleased/feature_detect_co_authored_commits.yml

@@ -0,0 +1,6 @@
+---
+title: Detect commit message trailers and link users properly to their accounts
+  on Gitlab
+merge_request: 17919
+author: cousine
+type: added

lib/banzai/filter/commit_trailers_filter.rb

@@ -0,0 +1,152 @@
+module Banzai
+  module Filter
+    # HTML filter that replaces users' names and emails in commit trailers
+    # with links to their GitLab accounts or mailto links to their mentioned
+    # emails.
+    #
+    # Commit trailers are special labels in the form of `*-by:` and fall on a
+    # single line, ex:
+    #
+    #   Reported-By: John S. Doe <john.doe@foo.bar>
+    #
+    # More info about this can be found here:
+    # * https://git.wiki.kernel.org/index.php/CommitMessageConventions
+    class CommitTrailersFilter < HTML::Pipeline::Filter
+      include ActionView::Helpers::TagHelper
+      include ApplicationHelper
+      include AvatarsHelper
+
+      TRAILER_REGEXP = /(?<label>[[:alpha:]-]+-by:)/i.freeze
+      AUTHOR_REGEXP = /(?<author_name>.+)/.freeze
+      # Devise.email_regexp wouldn't work here since its designed to match
+      # against strings that only contains email addresses; the \A and \z
+      # around the expression will only match if the string being matched
+      # contains just the email nothing else.
+      MAIL_REGEXP = /&lt;(?<author_email>[^@\s]+@[^@\s]+)&gt;/.freeze
+      FILTER_REGEXP = /(?<trailer>^\s*#{TRAILER_REGEXP}\s*#{AUTHOR_REGEXP}\s+#{MAIL_REGEXP}$)/mi.freeze
+
+      def call
+        doc.xpath('descendant-or-self::text()').each do |node|
+          content = node.to_html
+
+          next unless content.match(FILTER_REGEXP)
+
+          html = trailer_filter(content)
+
+          next if html == content
+
+          node.replace(html)
+        end
+
+        doc
+      end
+
+      private
+
+      # Replace trailer lines with links to GitLab users or mailto links to
+      # non GitLab users.
+      #
+      # text - String text to replace trailers in.
+      #
+      # Returns a String with all trailer lines replaced with links to GitLab
+      # users and mailto links to non GitLab users. All links have `data-trailer`
+      # and `data-user` attributes attached.
+      def trailer_filter(text)
+        text.gsub(FILTER_REGEXP) do |author_match|
+          label = $~[:label]
+          "#{label} #{parse_user($~[:author_name], $~[:author_email], label)}"
+        end
+      end
+
+      # Find a GitLab user using the supplied email and generate
+      # a valid link to them, otherwise, generate a mailto link.
+      #
+      # name - String name used in the commit message for the user
+      # email - String email used in the commit message for the user
+      # trailer - String trailer used in the commit message
+      #
+      # Returns a String with a link to the user.
+      def parse_user(name, email, trailer)
+        link_to_user User.find_by_any_email(email),
+          name: name,
+          email: email,
+          trailer: trailer
+      end
+
+      def urls
+        Gitlab::Routing.url_helpers
+      end
+
+      def link_to_user(user, name:, email:, trailer:)
+        wrapper = link_wrapper(data: {
+          trailer: trailer,
+          user: user.try(:id)
+        })
+
+        avatar = user_avatar_without_link(
+          user: user,
+          user_email: email,
+          css_class: 'avatar-inline',
+          has_tooltip: false
+        )
+
+        link_href = user.nil? ? "mailto:#{email}" : urls.user_url(user)
+
+        avatar_link = link_tag(
+          link_href,
+          content: avatar,
+          title: email
+        )
+
+        name_link = link_tag(
+          link_href,
+          content: name,
+          title: email
+        )
+
+        email_link = link_tag(
+          "mailto:#{email}",
+          content: email,
+          title: email
+        )
+
+        wrapper << "#{avatar_link}#{name_link} <#{email_link}>"
+      end
+
+      def link_wrapper(data: {})
+        data_attributes = data_attributes_from_hash(data)
+
+        doc.document.create_element(
+          'span',
+          data_attributes
+        )
+      end
+
+      def link_tag(url, title: "", content: "", data: {})
+        data_attributes = data_attributes_from_hash(data)
+
+        attributes = data_attributes.merge(
+          href: url,
+          title: title
+        )
+
+        link = doc.document.create_element('a', attributes)
+
+        if content.html_safe?
+          link << content
+        else
+          link.content = content # make sure we escape content using nokogiri's #content=
+        end
+
+        link
+      end
+
+      def data_attributes_from_hash(data = {})
+        data.reject! {|_, value| value.nil?}
+        data.map do |key, value|
+          [%(data-#{key.to_s.dasherize}), value]
+        end.to_h
+      end
+    end
+  end
+end

lib/banzai/pipeline/commit_description_pipeline.rb

@@ -0,0 +1,11 @@
+module Banzai
+  module Pipeline
+    class CommitDescriptionPipeline < SingleLinePipeline
+      def self.filters
+        @filters ||= super.concat FilterArray[
+          Filter::CommitTrailersFilter,
+        ]
+      end
+    end
+  end
+end

spec/lib/banzai/filter/commit_trailers_filter_spec.rb

@@ -0,0 +1,171 @@
+require 'spec_helper'
+require 'ffaker'
+
+describe Banzai::Filter::CommitTrailersFilter do
+  include FilterSpecHelper
+  include CommitTrailersSpecHelper
+
+  let(:secondary_email)     { create(:email, :confirmed) }
+  let(:user)                { create(:user) }
+
+  let(:trailer)             { "#{FFaker::Lorem.word}-by:"}
+
+  let(:commit_message)      { trailer_line(trailer, user.name, user.email) }
+  let(:commit_message_html) { commit_html(commit_message) }
+
+  context 'detects' do
+    let(:email) { FFaker::Internet.email }
+
+    it 'trailers in the form of *-by and replace users with links' do
+      doc = filter(commit_message_html)
+
+      expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
+    end
+
+    it 'trailers prefixed with whitespaces' do
+      message_html = commit_html("\n\r  #{commit_message}")
+
+      doc = filter(message_html)
+
+      expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
+    end
+
+    it 'GitLab users via a secondary email' do
+      _, message_html = build_commit_message(
+        trailer: trailer,
+        name: secondary_email.user.name,
+        email: secondary_email.email
+      )
+
+      doc = filter(message_html)
+
+      expect_to_have_user_link_with_avatar(
+        doc,
+        user: secondary_email.user,
+        trailer: trailer,
+        email: secondary_email.email
+      )
+    end
+
+    it 'non GitLab users and replaces them with mailto links' do
+      _, message_html = build_commit_message(
+        trailer: trailer,
+        name: FFaker::Name.name,
+        email: email
+      )
+
+      doc = filter(message_html)
+
+      expect_to_have_mailto_link(doc, email: email, trailer: trailer)
+    end
+
+    it 'multiple trailers in the same message' do
+      different_trailer = "#{FFaker::Lorem.word}-by:"
+      message = commit_html %(
+        #{commit_message}
+        #{trailer_line(different_trailer, FFaker::Name.name, email)}
+      )
+
+      doc = filter(message)
+
+      expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
+      expect_to_have_mailto_link(doc, email: email, trailer: different_trailer)
+    end
+
+    context 'special names' do
+      where(:name) do
+        [
+          'John S. Doe',
+          'L33t H@x0r'
+        ]
+      end
+
+      with_them do
+        it do
+          message, message_html = build_commit_message(
+            trailer: trailer,
+            name: name,
+            email: email
+          )
+
+          doc = filter(message_html)
+
+          expect_to_have_mailto_link(doc, email: email, trailer: trailer)
+          expect(doc.text).to match Regexp.escape(message)
+        end
+      end
+    end
+  end
+
+  context "ignores" do
+    it 'commit messages without trailers' do
+      exp = message = commit_html(FFaker::Lorem.sentence)
+      doc = filter(message)
+
+      expect(doc.to_html).to match Regexp.escape(exp)
+    end
+
+    it 'trailers that are inline the commit message body' do
+      message = commit_html %(
+        #{FFaker::Lorem.sentence} #{commit_message} #{FFaker::Lorem.sentence}
+      )
+
+      doc = filter(message)
+
+      expect(doc.css('a').size).to eq 0
+    end
+  end
+
+  context "structure" do
+    it 'preserves the commit trailer structure' do
+      doc = filter(commit_message_html)
+
+      expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
+      expect(doc.text).to match Regexp.escape(commit_message)
+    end
+
+    it 'preserves the original name used in the commit message' do
+      message, message_html = build_commit_message(
+        trailer: trailer,
+        name: FFaker::Name.name,
+        email: user.email
+      )
+
+      doc = filter(message_html)
+
+      expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
+      expect(doc.text).to match Regexp.escape(message)
+    end
+
+    it 'preserves the original email used in the commit message' do
+      message, message_html = build_commit_message(
+        trailer: trailer,
+        name: secondary_email.user.name,
+        email: secondary_email.email
+      )
+
+      doc = filter(message_html)
+
+      expect_to_have_user_link_with_avatar(
+        doc,
+        user: secondary_email.user,
+        trailer: trailer,
+        email: secondary_email.email
+      )
+      expect(doc.text).to match Regexp.escape(message)
+    end
+
+    it 'only replaces trailer lines not the full commit message' do
+      commit_body = FFaker::Lorem.paragraph
+      message = commit_html %(
+        #{commit_body}
+        #{commit_message}
+      )
+
+      doc = filter(message)
+
+      expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
+      expect(doc.text).to include(commit_body)
+    end
+  end
+end

spec/support/commit_trailers_spec_helper.rb

@@ -0,0 +1,41 @@
+module CommitTrailersSpecHelper
+  extend ActiveSupport::Concern
+
+  def expect_to_have_user_link_with_avatar(doc, user:, trailer:, email: nil)
+    wrapper = find_user_wrapper(doc, trailer)
+
+    expect_to_have_links_with_url_and_avatar(wrapper, urls.user_url(user), email || user.email)
+    expect(wrapper.attribute('data-user').value).to eq user.id.to_s
+  end
+
+  def expect_to_have_mailto_link(doc, email:, trailer:)
+    wrapper = find_user_wrapper(doc, trailer)
+
+    expect_to_have_links_with_url_and_avatar(wrapper, "mailto:#{CGI.escape_html(email)}", email)
+  end
+
+  def expect_to_have_links_with_url_and_avatar(doc, url, email)
+    expect(doc).not_to be_nil
+    expect(doc.xpath("a[position()<3 and @href='#{url}']").size).to eq 2
+    expect(doc.xpath("a[position()=3 and @href='mailto:#{CGI.escape_html(email)}']").size).to eq 1
+    expect(doc.css('img').size).to eq 1
+  end
+
+  def find_user_wrapper(doc, trailer)
+    doc.xpath("descendant-or-self::node()[@data-trailer='#{trailer}']").first
+  end
+
+  def build_commit_message(trailer:, name:, email:)
+    message = trailer_line(trailer, name, email)
+
+    [message, commit_html(message)]
+  end
+
+  def trailer_line(trailer, name, email)
+    "#{trailer} #{name} <#{email}>"
+  end
+
+  def commit_html(message)
+    "<pre>#{CGI.escape_html(message)}</pre>"
+  end
+end