Merge branch 'rs-more-pipeline-filters' into 'master'

More HTML::Pipeline filters

The big part of this MR is a feature that is intended to test the entire Markdown-parsing process from beginning to end. See `spec/support/markdown_feature.rb` and `spec/features/markdown_spec.rb`.

One big thing this MR fixes is not being able to type a `<` or `>` anywhere. It now gets properly escaped.

This MR also adds three more custom HTML::Pipeline filters:

### AutolinkFilter

Similar to the built-in Autolink filter in that it still uses Rinku for standard http and ftp links, but then does some further processing to allow auto-linking of any URI scheme. See internal issue https://dev.gitlab.org/gitlab/gitlabhq/issues/2239

### SanitizationFilter

Created a simple custom SanitizationFilter that sub-classes the default one and adds our custom whitelisting.

### TableOfContentsFilter

Adds the anchor links to each header. This removes some processing from our Redcarpet renderer.

Closes #800, #1015, #1528, #1549

Closes GitHub [8535](https://github.com/gitlabhq/gitlabhq/issues/8535)

See merge request !584

app/assets/stylesheets/generic/typography.scss

@@ -35,7 +35,12 @@ pre {
   /* Link to current header. */
   h1, h2, h3, h4, h5, h6 {
     position: relative;
-    &:hover > :last-child {
+
+    a.anchor {
+      display: none;
+    }
+
+    &:hover > a.anchor {
       $size: 16px;
       position: absolute;
       right: 100%;

app/helpers/gitlab_markdown_helper.rb

@@ -34,9 +34,7 @@ module GitlabMarkdownHelper
 
       # 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, {
-        with_toc_data:   true,
-        safe_links_only: true,
-        # Handled further down the line by HTML::Pipeline::SanitizationFilter
+        # Handled further down the line by Gitlab::Markdown::SanitizationFilter
         escape_html: false
       }.merge(options))
 
@@ -45,7 +43,6 @@ module GitlabMarkdownHelper
         no_intra_emphasis:   true,
         tables:              true,
         fenced_code_blocks:  true,
-        autolink:            true,
         strikethrough:       true,
         lax_spacing:         true,
         space_after_headers: true,

app/helpers/issues_helper.rb

@@ -43,17 +43,6 @@ module IssuesHelper
     end
   end
 
-  def title_for_issue(issue_iid, project = @project)
-    return '' if project.nil?
-
-    if project.default_issues_tracker?
-      issue = project.issues.where(iid: issue_iid).first
-      return issue.title if issue
-    end
-
-    ''
-  end
-
   def issue_timestamp(issue)
     # Shows the created at time and the updated at time if different
     ts = "#{time_ago_with_tooltip(issue.created_at, 'bottom', 'note_created_ago')}"
@@ -110,5 +99,5 @@ module IssuesHelper
   end
 
   # Required for Gitlab::Markdown::IssueReferenceFilter
-  module_function :url_for_issue, :title_for_issue
+  module_function :url_for_issue
 end

app/models/external_issue.rb

@@ -15,6 +15,10 @@ class ExternalIssue
     @issue_identifier.to_s
   end
 
+  def title
+    "External Issue #{self}"
+  end
+
   def ==(other)
     other.is_a?(self.class) && (to_s == other.to_s)
   end

app/models/project.rb

@@ -329,12 +329,16 @@ class Project < ActiveRecord::Base
     self.id
   end
 
-  def issue_exists?(issue_id)
+  def get_issue(issue_id)
     if default_issues_tracker?
-      self.issues.where(iid: issue_id).first.present?
+      issues.find_by(iid: issue_id)
     else
-      true
+      ExternalIssue.new(issue_id, self)
+    end
   end
+
+  def issue_exists?(issue_id)
+    get_issue(issue_id)
   end
 
   def default_issue_tracker
@@ -350,11 +354,7 @@ class Project < ActiveRecord::Base
   end
 
   def default_issues_tracker?
-    if external_issue_tracker
-      false
-    else
-      true
-    end
+    !external_issue_tracker
   end
 
   def external_issues_trackers

features/steps/shared/markdown.rb

@@ -2,8 +2,12 @@ module SharedMarkdown
   include Spinach::DSL
 
   def header_should_have_correct_id_and_link(level, text, id, parent = ".wiki")
-    find(:css, "#{parent} h#{level}##{id}").text.should == text
-    find(:css, "#{parent} h#{level}##{id} > :last-child")[:href].should =~ /##{id}$/
+    node = find("#{parent} h#{level} a##{id}")
+    node[:href].should == "##{id}"
+
+    # Work around a weird Capybara behavior where calling `parent` on a node
+    # returns the whole document, not the node's actual parent element
+    find(:xpath, "#{node.path}/..").text.should == text
   end
 
   def create_taskable(type, title)

lib/gitlab/markdown.rb

@@ -3,33 +3,10 @@ require 'html/pipeline'
 module Gitlab
   # Custom parser for GitLab-flavored Markdown
   #
-  # It replaces references in the text with links to the appropriate items in
-  # GitLab.
-  #
-  # Supported reference formats are:
-  #   * @foo for team members
-  #   * #123 for issues
-  #   * JIRA-123 for Jira issues
-  #   * !123 for merge requests
-  #   * $123 for snippets
-  #   * 1c002d for specific commit
-  #   * 1c002d...35cfb2 for commit ranges (comparisons)
-  #
-  # It also parses Emoji codes to insert images. See
-  # http://www.emoji-cheat-sheet.com/ for a list of the supported icons.
-  #
-  # Examples
-  #
-  #   >> gfm("Hey @david, can you fix this?")
-  #   => "Hey <a href="/u/david">@david</a>, can you fix this?"
-  #
-  #   >> gfm("Commit 35d5f7c closes #1234")
-  #   => "Commit <a href="/gitlab/commits/35d5f7c">35d5f7c</a> closes <a href="/gitlab/issues/1234">#1234</a>"
-  #
-  #   >> gfm(":trollface:")
-  #   => "<img alt=\":trollface:\" class=\"emoji\" src=\"/images/trollface.png" title=\":trollface:\" />
+  # See the files in `lib/gitlab/markdown/` for specific processing information.
   module Markdown
     # Provide autoload paths for filters to prevent a circular dependency error
+    autoload :AutolinkFilter,               'gitlab/markdown/autolink_filter'
     autoload :CommitRangeReferenceFilter,   'gitlab/markdown/commit_range_reference_filter'
     autoload :CommitReferenceFilter,        'gitlab/markdown/commit_reference_filter'
     autoload :EmojiFilter,                  'gitlab/markdown/emoji_filter'
@@ -37,7 +14,9 @@ module Gitlab
     autoload :IssueReferenceFilter,         'gitlab/markdown/issue_reference_filter'
     autoload :LabelReferenceFilter,         'gitlab/markdown/label_reference_filter'
     autoload :MergeRequestReferenceFilter,  'gitlab/markdown/merge_request_reference_filter'
+    autoload :SanitizationFilter,           'gitlab/markdown/sanitization_filter'
     autoload :SnippetReferenceFilter,       'gitlab/markdown/snippet_reference_filter'
+    autoload :TableOfContentsFilter,        'gitlab/markdown/table_of_contents_filter'
     autoload :UserReferenceFilter,          'gitlab/markdown/user_reference_filter'
 
     # Public: Parse the provided text with GitLab-Flavored Markdown
@@ -74,13 +53,13 @@ module Gitlab
       pipeline = HTML::Pipeline.new(filters)
 
       context = {
-        # SanitizationFilter
-        whitelist: sanitization_whitelist,
-
         # EmojiFilter
         asset_root: Gitlab.config.gitlab.url,
         asset_host: Gitlab::Application.config.asset_host,
 
+        # TableOfContentsFilter
+        no_header_anchors: options[:no_header_anchors],
+
         # ReferenceFilter
         current_user:    current_user,
         only_path:       options[:reference_only_path],
@@ -111,12 +90,14 @@ module Gitlab
     # SanitizationFilter should come first so that all generated reference HTML
     # goes through untouched.
     #
-    # See https://gitlab.com/gitlab-org/html-pipeline-gitlab for more filters
+    # See https://github.com/jch/html-pipeline#filters for more filters.
     def filters
       [
-        HTML::Pipeline::SanitizationFilter,
+        Gitlab::Markdown::SanitizationFilter,
 
         Gitlab::Markdown::EmojiFilter,
+        Gitlab::Markdown::TableOfContentsFilter,
+        Gitlab::Markdown::AutolinkFilter,
 
         Gitlab::Markdown::UserReferenceFilter,
         Gitlab::Markdown::IssueReferenceFilter,
@@ -125,36 +106,10 @@ module Gitlab
         Gitlab::Markdown::SnippetReferenceFilter,
         Gitlab::Markdown::CommitRangeReferenceFilter,
         Gitlab::Markdown::CommitReferenceFilter,
-        Gitlab::Markdown::LabelReferenceFilter,
+        Gitlab::Markdown::LabelReferenceFilter
       ]
     end
 
-    # Customize the SanitizationFilter whitelist
-    #
-    # - Allow `class` and `id` attributes on all elements
-    # - Allow `span` elements
-    # - Remove `rel` attributes from `a` elements
-    # - Remove `a` nodes with `javascript:` in the `href` attribute
-    def sanitization_whitelist
-      whitelist = HTML::Pipeline::SanitizationFilter::WHITELIST
-      whitelist[:attributes][:all].push('class', 'id')
-      whitelist[:elements].push('span')
-
-      fix_anchors = lambda do |env|
-        name, node = env[:node_name], env[:node]
-        if name == 'a'
-          node.remove_attribute('rel')
-          if node['href'] && node['href'].match('javascript:')
-            node.remove_attribute('href')
-          end
-        end
-      end
-
-      whitelist[:transformers].push(fix_anchors)
-
-      whitelist
-    end
-
     # Turn list items that start with "[ ]" into HTML checkbox inputs.
     def parse_tasks(text)
       li_tag = '<li class="task-list-item">'

lib/gitlab/markdown/autolink_filter.rb

+require 'html/pipeline/filter'
+require 'uri'
+
+module Gitlab
+  module Markdown
+    # HTML Filter for auto-linking URLs in HTML.
+    #
+    # Based on HTML::Pipeline::AutolinkFilter
+    #
+    # Context options:
+    #   :autolink  - Boolean, skips all processing done by this filter when false
+    #   :link_attr - Hash of attributes for the generated links
+    #
+    class AutolinkFilter < HTML::Pipeline::Filter
+      include ActionView::Helpers::TagHelper
+
+      # Pattern to match text that should be autolinked.
+      #
+      # A URI scheme begins with a letter and may contain letters, numbers,
+      # plus, period and hyphen. Schemes are case-insensitive but we're being
+      # picky here and allowing only lowercase for autolinks.
+      #
+      # See http://en.wikipedia.org/wiki/URI_scheme
+      #
+      # The negative lookbehind ensures that users can paste a URL followed by a
+      # period or comma for punctuation without those characters being included
+      # in the generated link.
+      #
+      # Rubular: http://rubular.com/r/cxjPyZc7Sb
+      LINK_PATTERN = %r{([a-z][a-z0-9\+\.-]+://\S+)(?<!,|\.)}
+
+      # Text matching LINK_PATTERN inside these elements will not be linked
+      IGNORE_PARENTS = %w(a code kbd pre script style).to_set
+
+      def call
+        return doc if context[:autolink] == false
+
+        rinku_parse
+        text_parse
+      end
+
+      private
+
+      # Run the text through Rinku as a first pass
+      #
+      # This will quickly autolink http(s) and ftp links.
+      #
+      # `@doc` will be re-parsed with the HTML String from Rinku.
+      def rinku_parse
+        # Convert the options from a Hash to a String that Rinku expects
+        options = tag_options(link_options)
+
+        # NOTE: We don't parse email links because it will erroneously match
+        # external Commit and CommitRange references.
+        #
+        # The final argument tells Rinku to link short URLs that don't include a
+        # period (e.g., http://localhost:3000/)
+        rinku = Rinku.auto_link(html, :urls, options, IGNORE_PARENTS.to_a, 1)
+
+        # Rinku returns a String, so parse it back to a Nokogiri::XML::Document
+        # for further processing.
+        @doc = parse_html(rinku)
+      end
+
+      # Autolinks any text matching LINK_PATTERN that Rinku didn't already
+      # replace
+      def text_parse
+        search_text_nodes(doc).each do |node|
+          content = node.to_html
+
+          next if has_ancestor?(node, IGNORE_PARENTS)
+          next unless content.match(LINK_PATTERN)
+
+          # If Rinku didn't link this, there's probably a good reason, so we'll
+          # skip it too
+          next if content.start_with?(*%w(http https ftp))
+
+          html = autolink_filter(content)
+
+          next if html == content
+
+          node.replace(html)
+        end
+
+        doc
+      end
+
+      def autolink_filter(text)
+        text.gsub(LINK_PATTERN) do |match|
+          options = link_options.merge(href: match)
+          content_tag(:a, match, options)
+        end
+      end
+
+      def link_options
+        @link_options ||= context[:link_attr] || {}
+      end
+    end
+  end
+end

lib/gitlab/markdown/issue_reference_filter.rb

@@ -44,21 +44,20 @@ module Gitlab
       # Returns a String with `#123` references replaced with links. All links
       # have `gfm` and `gfm-issue` class names attached for styling.
       def issue_link_filter(text)
-        self.class.references_in(text) do |match, issue, project_ref|
+        self.class.references_in(text) do |match, id, project_ref|
           project = self.project_from_ref(project_ref)
 
-          if project && project.issue_exists?(issue)
-            # FIXME (rspeicher): Law of Demeter
-            push_result(:issue, project.issues.where(iid: issue).first)
+          if project && issue = project.get_issue(id)
+            push_result(:issue, issue)
 
-            url = url_for_issue(issue, project, only_path: context[:only_path])
+            url = url_for_issue(id, project, only_path: context[:only_path])
 
-            title = escape_once("Issue: #{title_for_issue(issue, project)}")
+            title = escape_once("Issue: #{issue.title}")
             klass = reference_class(:issue)
 
             %(<a href="#{url}"
                  title="#{title}"
-                 class="#{klass}">#{project_ref}##{issue}</a>)
+                 class="#{klass}">#{project_ref}##{id}</a>)
           else
             match
           end
@@ -68,10 +67,6 @@ module Gitlab
       def url_for_issue(*args)
         IssuesHelper.url_for_issue(*args)
       end
-
-      def title_for_issue(*args)
-        IssuesHelper.title_for_issue(*args)
-      end
     end
   end
 end

lib/gitlab/markdown/merge_request_reference_filter.rb

@@ -64,7 +64,6 @@ module Gitlab
         end
       end
 
-      # TODO (rspeicher): Cleanup
       def url_for_merge_request(mr, project)
         h = Rails.application.routes.url_helpers
         h.namespace_project_merge_request_url(project.namespace, project, mr,

lib/gitlab/markdown/sanitization_filter.rb

+require 'html/pipeline/filter'
+require 'html/pipeline/sanitization_filter'
+
+module Gitlab
+  module Markdown
+    # Sanitize HTML
+    #
+    # Extends HTML::Pipeline::SanitizationFilter with a custom whitelist.
+    class SanitizationFilter < HTML::Pipeline::SanitizationFilter
+      def whitelist
+        whitelist = HTML::Pipeline::SanitizationFilter::WHITELIST
+
+        # Allow `class` and `id` on all elements
+        whitelist[:attributes][:all].push('class', 'id')
+
+        # Allow table alignment
+        whitelist[:attributes]['th'] = %w(style)
+        whitelist[:attributes]['td'] = %w(style)
+
+        # Allow span elements
+        whitelist[:elements].push('span')
+
+        # Remove `rel` attribute from `a` elements
+        whitelist[:transformers].push(remove_rel)
+
+        whitelist
+      end
+
+      def remove_rel
+        lambda do |env|
+          if env[:node_name] == 'a'
+            env[:node].remove_attribute('rel')
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/markdown/table_of_contents_filter.rb

+require 'html/pipeline/filter'
+
+module Gitlab
+  module Markdown
+    # HTML filter that adds an anchor child element to all Headers in a
+    # document, so that they can be linked to.
+    #
+    # Generates the Table of Contents with links to each header. See Results.
+    #
+    # Based on HTML::Pipeline::TableOfContentsFilter.
+    #
+    # Context options:
+    #   :no_header_anchors - Skips all processing done by this filter.
+    #
+    # Results:
+    #   :toc - String containing Table of Contents data as a `ul` element with
+    #          `li` child elements.
+    class TableOfContentsFilter < HTML::Pipeline::Filter
+      PUNCTUATION_REGEXP = /[^\p{Word}\- ]/u
+
+      def call
+        return doc if context[:no_header_anchors]
+
+        result[:toc] = ""
+
+        headers = Hash.new(0)
+
+        doc.css('h1, h2, h3, h4, h5, h6').each do |node|
+          text = node.text
+
+          id = text.downcase
+          id.gsub!(PUNCTUATION_REGEXP, '') # remove punctuation
+          id.gsub!(' ', '-') # replace spaces with dash
+          id.squeeze!(' -') # replace multiple spaces or dashes with one
+
+          uniq = (headers[id] > 0) ? "-#{headers[id]}" : ''
+          headers[id] += 1
+
+          if header_content = node.children.first
+            href = "#{id}#{uniq}"
+            push_toc(href, text)
+            header_content.add_previous_sibling(anchor_tag(href))
+          end
+        end
+
+        result[:toc] = %Q{<ul class="section-nav">\n#{result[:toc]}</ul>} unless result[:toc].empty?
+
+        doc
+      end
+
+      private
+
+      def anchor_tag(href)
+        %Q{<a id="#{href}" class="anchor" href="##{href}" aria-hidden="true"></a>}
+      end
+
+      def push_toc(href, text)
+        result[:toc] << %Q{<li><a href="##{href}">#{text}</a></li>\n}
+      end
+    end
+  end
+end

lib/gitlab/theme.rb

@@ -40,7 +40,7 @@ module Gitlab
     end
 
     # Convenience method to get a space-separated String of all the theme
-    # classes that mighty be applied to the `body` element
+    # classes that might be applied to the `body` element
     #
     # Returns a String
     def self.body_classes

lib/redcarpet/render/gitlab_html.rb

-class Redcarpet::Render::GitlabHTML < Redcarpet::Render::HTML
+require 'active_support/core_ext/string/output_safety'
 
+class Redcarpet::Render::GitlabHTML < Redcarpet::Render::HTML
   attr_reader :template
   alias_method :h, :template
 
@@ -8,24 +9,12 @@ class Redcarpet::Render::GitlabHTML < Redcarpet::Render::HTML
     @color_scheme = color_scheme
     @project = @template.instance_variable_get("@project")
     @options = options.dup
-    super options
-  end
 
-  def preprocess(full_document)
-    # Redcarpet doesn't allow SMB links when `safe_links_only` is enabled.
-    # FTP links are allowed, so we trick Redcarpet.
-    full_document.gsub("smb://", "ftp://smb:")
+    super(options)
   end
 
-  # If project has issue number 39, apostrophe will be linked in
-  # regular text to the issue as Redcarpet will convert apostrophe to
-  # #39;
-  # We replace apostrophe with right single quote before Redcarpet
-  # does the processing and put the apostrophe back in postprocessing.
-  # This only influences regular text, code blocks are untouched.
   def normal_text(text)
-    return text unless text.present?
-    text.gsub("'", "&rsquo;")
+    ERB::Util.html_escape_once(text)
   end
 
   # Stolen from Rugments::Plugins::Redcarpet as this module is not required
@@ -37,7 +26,7 @@ class Redcarpet::Render::GitlabHTML < Redcarpet::Render::HTML
     # so we assume you're not using leading spaces that aren't tabs,
     # and just replace them here.
     if lexer.tag == 'make'
-      code.gsub! /^    /, "\t"
+      code.gsub!(/^    /, "\t")
     end
 
     formatter = Rugments::Formatters::HTML.new(
@@ -46,27 +35,11 @@ class Redcarpet::Render::GitlabHTML < Redcarpet::Render::HTML
     formatter.format(lexer.lex(code))
   end
 
-  def link(link, title, content)
-    h.link_to_gfm(content, link, title: title)
-  end
-
-  def header(text, level)
-    if @options[:no_header_anchors]
-      "<h#{level}>#{text}</h#{level}>"
-    else
-      id = ActionController::Base.helpers.strip_tags(h.gfm(text)).downcase() \
-          .gsub(/[^a-z0-9_-]/, '-').gsub(/-+/, '-').gsub(/^-/, '').gsub(/-$/, '')
-      "<h#{level} id=\"#{id}\">#{text}<a href=\"\##{id}\"></a></h#{level}>"
-    end
-  end
-
   def postprocess(full_document)
-    full_document.gsub!("ftp://smb:", "smb://")
-
-    full_document.gsub!("&rsquo;", "'")
     unless @template.instance_variable_get("@project_wiki") || @project.nil?
       full_document = h.create_relative_links(full_document)
     end
+
     h.gfm_with_options(full_document, @options)
   end
 end

spec/factories/projects.rb

@@ -94,10 +94,26 @@ FactoryGirl.define do
           'new_issue_url' => 'http://redmine/projects/project_name_in_redmine/issues/new'
         }
       )
-    end
-    after :create do |project|
+
       project.issues_tracker = 'redmine'
       project.issues_tracker_id = 'project_name_in_redmine'
     end
   end
+
+  factory :jira_project, parent: :project do
+    after :create do |project|
+      project.create_jira_service(
+        active: true,
+        properties: {
+          'title'         => 'JIRA tracker',
+          'project_url'   => 'http://jira.example/issues/?jql=project=A',
+          'issues_url'    => 'http://jira.example/browse/:id',
+          'new_issue_url' => 'http://jira.example/secure/CreateIssue.jspa'
+        }
+      )
+
+      project.issues_tracker = 'jira'
+      project.issues_tracker_id = 'project_name_in_jira'
+    end
+  end
 end

spec/fixtures/markdown.md.erb

+# GitLab Markdown
+
+This document is intended to be a comprehensive example of custom GitLab
+Markdown usage. It will be parsed and then tested for accuracy. Let's get
+started.
+
+## Markdown
+
+GitLab uses [Redcarpet](http://git.io/ld_NVQ) to parse all Markdown into
+HTML.
+
+It has some special features. Let's try 'em out!
+
+### No Intra Emphasis
+
+This string should have no emphasis: foo_bar_baz
+
+### Tables
+
+| Header   | Row  | Example |
+| :------: | ---: | :------ |
+| Foo      | Bar  | **Baz** |
+
+### Fenced Code Blocks
+
+```c
+#include<stdio.h>
+
+main()
+{
+    printf("Hello World");
+
+}
+```
+
+```python
+print "Hello, World!"
+```
+
+### Strikethrough
+
+This text says this, ~~and this text doesn't~~.
+
+### Superscript
+
+This is my 1^(st) time using superscript in Markdown. Now this is my
+2^(nd).
+
+### Next step
+
+After the Markdown has been turned into HTML, it gets passed through...
+
+## HTML::Pipeline
+
+### SanitizationFilter
+
+GitLab uses <a href="http://git.io/vfW8a" class="sanitize" id="sanitize-link">HTML::Pipeline::SanitizationFilter</a>
+to sanitize the generated HTML, stripping dangerous or unwanted tags.
+
+Its default whitelist is pretty permissive. Check it:
+
+<b id="manual-b">This text is bold</b> and <em id="manual-em">this text is emphasized</em>.
+
+<code id="manual-code">echo "Hello, world!"</code>
+
+Press <kbd>s</kbd> to search.
+
+<strike>Emoji</strike> Plain old images! <img
+src="http://www.emoji-cheat-sheet.com/graphics/emojis/smile.png" width="20"
+height="20" id="manual-img" />
+
+Here comes a line break:
+
+<br id="manual-br" />
+
+And a horizontal rule:
+
+<hr id="manual-hr" />
+
+As permissive as it is, we've allowed even more stuff:
+
+<span class="light" id="span-class-light">Span elements</span>
+
+<a href="#" rel="nofollow" id="a-rel-nofollow">This is a link with a defined rel attribute, which should be removed</a>
+
+<a href="javascript:alert('Hi')" id="a-href-javascript">This is a link trying to be sneaky. It gets its link removed entirely.</a>
+
+### Escaping
+
+The problem with SanitizationFilter is that it can be too aggressive.
+
+| Input       | Expected         | Actual    |
+| ----------- | ---------------- | --------- |
+| `1 < 3 & 5` | 1 &lt; 3 &amp; 5 | 1 < 3 & 5 |
+| `<foo>`     | &lt;foo&gt;      | <foo>     |
+
+### EmojiFilter
+
+Because life would be :zzz: without Emoji, right? :rocket:
+
+Get ready for the Emoji :bomb:: :+1::-1::ok_hand::wave::v::raised_hand::muscle:
+
+### TableOfContentsFilter
+
+All headers in this document should be linkable. Try it.
+
+### AutolinkFilter
+
+These are all plain text that should get turned into links:
+
+- http://about.gitlab.com/
+- https://google.com/
+- ftp://ftp.us.debian.org/debian/
+- smb://foo/bar/baz
+- irc://irc.freenode.net/git
+- http://localhost:3000
+
+But it shouldn't autolink text inside certain tags:
+
+- <code id="autolink-code">http://about.gitlab.com/</code>
+- <a id="autolink-a">http://about.gitlab.com/</a>
+- <kbd id="autolink-kbd">http://about.gitlab.com/</kbd>
+
+### Reference Filters (e.g., #<%= issue.iid %>)
+
+References should be parseable even inside _!<%= merge_request.iid %>_ emphasis.
+
+#### UserReferenceFilter
+
+- All: @all
+- User: @<%= user.username %>
+- Group: @<%= group.name %>
+- Ignores invalid: @fake_user
+- Ignored in code: `@<%= user.username %>`
+- Ignored in links: [Link to @<%= user.username %>](#user-link)
+
+#### IssueReferenceFilter
+
+- Issue: #<%= issue.iid %>
+- Issue in another project: <%= xref %>#<%= xissue.iid %>
+- Ignored in code: `#<%= issue.iid %>`
+- Ignored in links: [Link to #<%= issue.iid %>](#issue-link)
+
+#### MergeRequestReferenceFilter
+
+- Merge request: !<%= merge_request.iid %>
+- Merge request in another project: <%= xref %>!<%= xmerge_request.iid %>
+- Ignored in code: `!<%= merge_request.iid %>`
+- Ignored in links: [Link to !<%= merge_request.iid %>](#merge-request-link)
+
+#### SnippetReferenceFilter
+
+- Snippet: $<%= snippet.id %>
+- Snippet in another project: <%= xref %>$<%= xsnippet.id %>
+- Ignored in code: `$<%= snippet.id %>`
+- Ignored in links: [Link to $<%= snippet.id %>](#snippet-link)
+
+#### CommitRangeReferenceFilter
+
+- Range: <%= commit_range %>
+- Range in another project: <%= xref %>@<%= xcommit_range %>
+- Ignored in code: `<%= commit_range %>`
+- Ignored in links: [Link to <%= commit_range %>](#commit-range-link)
+
+#### CommitReferenceFilter
+
+- Commit: <%= commit.id %>
+- Commit in another project: <%= xref %>@<%= xcommit.id %>
+- Ignored in code: `<%= commit.id %>`
+- Ignored in links: [Link to <%= commit.id %>](#commit-link)
+
+#### LabelReferenceFilter
+
+- Label by ID: ~<%= simple_label.id %>
+- Label by name: ~<%= simple_label.name %>
+- Label by name in quotes: ~"<%= label.name %>"
+- Ignored in code: `~<%= simple_label.name %>`
+- Ignored in links: [Link to ~<%= simple_label.id %>](#label-link)

spec/helpers/gitlab_markdown_helper_spec.rb

@@ -107,8 +107,7 @@ describe GitlabMarkdownHelper do
       end
 
       it 'should not be confused by whitespace before bullets' do
-        rendered_text_asterisk = markdown(@source_text_asterisk,
-                                          parse_tasks: true)
+        rendered_text_asterisk = markdown(@source_text_asterisk, parse_tasks: true)
         rendered_text_dash = markdown(@source_text_dash, parse_tasks: true)
 
         expect(rendered_text_asterisk).to match(
@@ -207,78 +206,7 @@ describe GitlabMarkdownHelper do
   end
 
   describe "#markdown" do
-    # TODO (rspeicher) - This block tests multiple different contexts. Break this up!
-
-    it "should add ids and links to headers" do
-      # Test every rule except nested tags.
-      text = '..Ab_c-d. e..'
-      id = 'ab_c-d-e'
-      expect(markdown("# #{text}")).
-        to match(%r{<h1 id="#{id}">#{text}<a href="[^"]*##{id}"></a></h1>})
-      expect(markdown("# #{text}", {no_header_anchors:true})).
-      to eq("<h1>#{text}</h1>")
-
-      id = 'link-text'
-      expect(markdown("# [link text](url) ![img alt](url)")).to match(
-        %r{<h1 id="#{id}"><a href="[^"]*url">link text</a> <img[^>]*><a href="[^"]*##{id}"></a></h1>}
-      )
-    end
-
-    # REFERENCES (PART TWO: THE REVENGE) ---------------------------------------
-
-    it "should handle references in headers" do
-      actual = "\n# Working around ##{issue.iid}\n## Apply !#{merge_request.iid}"
-
-      expect(markdown(actual, no_header_anchors: true)).
-        to match(%r{<h1[^<]*>Working around <a.+>##{issue.iid}</a></h1>})
-      expect(markdown(actual, no_header_anchors: true)).
-        to match(%r{<h2[^<]*>Apply <a.+>!#{merge_request.iid}</a></h2>})
-    end
-
-    it "should handle references in <em>" do
-      actual = "Apply _!#{merge_request.iid}_ ASAP"
-
-      expect(markdown(actual)).
-        to match(%r{Apply <em><a.+>!#{merge_request.iid}</a></em>})
-    end
-
-    # CODE BLOCKS -------------------------------------------------------------
-
-    it "should leave code blocks untouched" do
-      allow(helper).to receive(:current_user).and_return(user)
-      allow(helper).to receive(:user_color_scheme_class).and_return(:white)
-
-      target_html = "<pre class=\"code highlight white plaintext\"><code>some code from $#{snippet.id}\nhere too\n</code></pre>\n"
-
-      expect(markdown("\n    some code from $#{snippet.id}\n    here too\n")).
-        to eq(target_html)
-      expect(markdown("\n```\nsome code from $#{snippet.id}\nhere too\n```\n")).
-        to eq(target_html)
-    end
-
-    it "should leave inline code untouched" do
-      expect(markdown("Don't use `$#{snippet.id}` here.")).
-        to eq "<p>Don't use <code>$#{snippet.id}</code> here.</p>\n"
-    end
-
-    # REF-LIKE AUTOLINKS? -----------------------------------------------------
-    # Basically: Don't parse references inside `<a>` tags.
-
-    it "should leave ref-like autolinks untouched" do
-      expect(markdown("look at http://example.tld/#!#{merge_request.iid}")).to eq("<p>look at <a href=\"http://example.tld/#!#{merge_request.iid}\">http://example.tld/#!#{merge_request.iid}</a></p>\n")
-    end
-
-    it "should leave ref-like href of 'manual' links untouched" do
-      expect(markdown("why not [inspect !#{merge_request.iid}](http://example.tld/#!#{merge_request.iid})")).to eq("<p>why not <a href=\"http://example.tld/#!#{merge_request.iid}\">inspect </a><a href=\"#{namespace_project_merge_request_path(project.namespace, project, merge_request)}\" title=\"Merge Request: #{merge_request.title}\" class=\"gfm gfm-merge_request\">!#{merge_request.iid}</a><a href=\"http://example.tld/#!#{merge_request.iid}\"></a></p>\n")
-    end
-
-    it "should leave ref-like src of images untouched" do
-      expect(markdown("screen shot: ![some image](http://example.tld/#!#{merge_request.iid})")).to eq("<p>screen shot: <img src=\"http://example.tld/#!#{merge_request.iid}\" alt=\"some image\"></p>\n")
-    end
-
-    # RELATIVE URLS -----------------------------------------------------------
     # TODO (rspeicher): These belong in a relative link filter spec
-
     context 'relative links' do
       context 'with a valid repository' do
         before do
@@ -333,11 +261,6 @@ describe GitlabMarkdownHelper do
           expected = ""
           expect(markdown(actual)).to match(expected)
         end
-
-        it 'should allow whitelisted HTML tags from the user' do
-          actual = '<dl><dt>Term</dt><dd>Definition</dd></dl>'
-          expect(markdown(actual)).to match(actual)
-        end
       end
 
       context 'with an empty repository' do
@@ -353,34 +276,6 @@ describe GitlabMarkdownHelper do
         end
       end
     end
-
-    # SANITIZATION ------------------------------------------------------------
-    # TODO (rspeicher): These are testing SanitizationFilter, not `markdown`
-
-    it 'should sanitize tags that are not whitelisted' do
-      actual = '<textarea>no inputs allowed</textarea> <blink>no blinks</blink>'
-      expected = 'no inputs allowed no blinks'
-      expect(markdown(actual)).to match(expected)
-      expect(markdown(actual)).not_to match('<.textarea>')
-      expect(markdown(actual)).not_to match('<.blink>')
-    end
-
-    it 'should allow whitelisted tag attributes from the user' do
-      actual = '<a class="custom">link text</a>'
-      expect(markdown(actual)).to match(actual)
-    end
-
-    it 'should sanitize tag attributes that are not whitelisted' do
-      actual = '<a href="http://example.com/bar.html" foo="bar">link text</a>'
-      expected = '<a href="http://example.com/bar.html">link text</a>'
-      expect(markdown(actual)).to match(expected)
-    end
-
-    it 'should sanitize javascript in attributes' do
-      actual = %q(<a href="javascript:alert('foo')">link text</a>)
-      expected = '<a>link text</a>'
-      expect(markdown(actual)).to match(expected)
-    end
   end
 
   describe '#render_wiki_content' do

spec/helpers/issues_helper_spec.rb

@@ -5,24 +5,6 @@ describe IssuesHelper do
   let(:issue) { create :issue, project: project }
   let(:ext_project) { create :redmine_project }
 
-  describe "title_for_issue" do
-    it "should return issue title if used internal tracker" do
-      @project = project
-      expect(title_for_issue(issue.iid)).to eq issue.title
-    end
-
-    it "should always return empty string if used external tracker" do
-      @project = ext_project
-      expect(title_for_issue(rand(100))).to eq ""
-    end
-
-    it "should always return empty string if project nil" do
-      @project = nil
-
-      expect(title_for_issue(rand(100))).to eq ""
-    end
-  end
-
   describe "url_for_project_issues" do
     let(:project_url) { ext_project.external_issue_tracker.project_url }
     let(:ext_expected) do

spec/lib/gitlab/markdown/autolink_filter_spec.rb

+require 'spec_helper'
+
+module Gitlab::Markdown
+  describe AutolinkFilter do
+    let(:link) { 'http://about.gitlab.com/' }
+
+    def filter(html, options = {})
+      described_class.call(html, options)
+    end
+
+    it 'does nothing when :autolink is false' do
+      exp = act = link
+      expect(filter(act, autolink: false).to_html).to eq exp
+    end
+
+    it 'does nothing with non-link text' do
+      exp = act = 'This text contains no links to autolink'
+      expect(filter(act).to_html).to eq exp
+    end
+
+    context 'Rinku schemes' do
+      it 'autolinks http' do
+        doc = filter("See #{link}")
+        expect(doc.at_css('a').text).to eq link
+        expect(doc.at_css('a')['href']).to eq link
+      end
+
+      it 'autolinks https' do
+        link = 'https://google.com/'
+        doc = filter("See #{link}")
+
+        expect(doc.at_css('a').text).to eq link
+        expect(doc.at_css('a')['href']).to eq link
+      end
+
+      it 'autolinks ftp' do
+        link = 'ftp://ftp.us.debian.org/debian/'
+        doc = filter("See #{link}")
+
+        expect(doc.at_css('a').text).to eq link
+        expect(doc.at_css('a')['href']).to eq link
+      end
+
+      it 'autolinks short URLs' do
+        link = 'http://localhost:3000/'
+        doc = filter("See #{link}")
+
+        expect(doc.at_css('a').text).to eq link
+        expect(doc.at_css('a')['href']).to eq link
+      end
+
+      it 'accepts link_attr options' do
+        doc = filter("See #{link}", link_attr: {class: 'custom'})
+
+        expect(doc.at_css('a')['class']).to eq 'custom'
+      end
+
+      described_class::IGNORE_PARENTS.each do |elem|
+        it "ignores valid links contained inside '#{elem}' element" do
+          exp = act = "<#{elem}>See #{link}</#{elem}>"
+          expect(filter(act).to_html).to eq exp
+        end
+      end
+    end
+
+    context 'other schemes' do
+      let(:link) { 'foo://bar.baz/' }
+
+      it 'autolinks smb' do
+        link = 'smb:///Volumes/shared/foo.pdf'
+        doc = filter("See #{link}")
+
+        expect(doc.at_css('a').text).to eq link
+        expect(doc.at_css('a')['href']).to eq link
+      end
+
+      it 'autolinks irc' do
+        link = 'irc://irc.freenode.net/git'
+        doc = filter("See #{link}")
+
+        expect(doc.at_css('a').text).to eq link
+        expect(doc.at_css('a')['href']).to eq link
+      end
+
+      it 'does not include trailing punctuation' do
+        doc = filter("See #{link}.")
+        expect(doc.at_css('a').text).to eq link
+
+        doc = filter("See #{link}, ok?")
+        expect(doc.at_css('a').text).to eq link
+      end
+
+      it 'accepts link_attr options' do
+        doc = filter("See #{link}", link_attr: {class: 'custom'})
+        expect(doc.at_css('a')['class']).to eq 'custom'
+      end
+
+      described_class::IGNORE_PARENTS.each do |elem|
+        it "ignores valid links contained inside '#{elem}' element" do
+          exp = act = "<#{elem}>See #{link}</#{elem}>"
+          expect(filter(act).to_html).to eq exp
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/markdown/external_issue_reference_filter_spec.rb

@@ -8,29 +8,12 @@ module Gitlab::Markdown
       IssuesHelper
     end
 
-    let(:project) { create(:empty_project) }
+    let(:project) { create(:jira_project) }
     let(:issue)   { double('issue', iid: 123) }
 
     context 'JIRA issue references' do
       let(:reference) { "JIRA-#{issue.iid}" }
 
-      before do
-        jira = project.create_jira_service
-
-        props = {
-          'title'         => 'JIRA tracker',
-          'project_url'   => 'http://jira.example/issues/?jql=project=A',
-          'issues_url'    => 'http://jira.example/browse/:id',
-          'new_issue_url' => 'http://jira.example/secure/CreateIssue.jspa'
-        }
-
-        jira.update_attributes(properties: props, active: true)
-      end
-
-      after do
-        project.jira_service.destroy
-      end
-
       it 'requires project context' do
         expect { described_class.call('Issue JIRA-123', {}) }.
           to raise_error(ArgumentError, /:project/)

spec/lib/gitlab/markdown/issue_reference_filter_spec.rb

@@ -27,7 +27,7 @@ module Gitlab::Markdown
       let(:reference) { "##{issue.iid}" }
 
       it 'ignores valid references when using non-default tracker' do
-        expect(project).to receive(:issue_exists?).with(issue.iid).and_return(false)
+        expect(project).to receive(:get_issue).with(issue.iid).and_return(nil)
 
         exp = act = "Issue ##{issue.iid}"
         expect(filter(act).to_html).to eq exp
@@ -48,7 +48,7 @@ module Gitlab::Markdown
       it 'ignores invalid issue IDs' do
         exp = act = "Fixed ##{issue.iid + 1}"
 
-        expect(project).to receive(:issue_exists?).with(issue.iid + 1)
+        expect(project).to receive(:get_issue).with(issue.iid + 1).and_return(nil)
         expect(filter(act).to_html).to eq exp
       end
 
@@ -98,8 +98,8 @@ module Gitlab::Markdown
         before { allow_cross_reference! }
 
         it 'ignores valid references when cross-reference project uses external tracker' do
-          expect_any_instance_of(Project).to receive(:issue_exists?).
-            with(issue.iid).and_return(false)
+          expect_any_instance_of(Project).to receive(:get_issue).
+            with(issue.iid).and_return(nil)
 
           exp = act = "Issue ##{issue.iid}"
           expect(filter(act).to_html).to eq exp

spec/lib/gitlab/markdown/sanitization_filter_spec.rb

+require 'spec_helper'
+
+module Gitlab::Markdown
+  describe SanitizationFilter do
+    def filter(html, options = {})
+      described_class.call(html, options)
+    end
+
+    describe 'default whitelist' do
+      it 'sanitizes tags that are not whitelisted' do
+        act = %q{<textarea>no inputs</textarea> and <blink>no blinks</blink>}
+        exp = 'no inputs and no blinks'
+        expect(filter(act).to_html).to eq exp
+      end
+
+      it 'sanitizes tag attributes' do
+        act = %q{<a href="http://example.com/bar.html" onclick="bar">Text</a>}
+        exp = %q{<a href="http://example.com/bar.html">Text</a>}
+        expect(filter(act).to_html).to eq exp
+      end
+
+      it 'sanitizes javascript in attributes' do
+        act = %q(<a href="javascript:alert('foo')">Text</a>)
+        exp = '<a>Text</a>'
+        expect(filter(act).to_html).to eq exp
+      end
+
+      it 'allows whitelisted HTML tags from the user' do
+        exp = act = "<dl>\n<dt>Term</dt>\n<dd>Definition</dd>\n</dl>"
+        expect(filter(act).to_html).to eq exp
+      end
+    end
+
+    describe 'custom whitelist' do
+      it 'allows `class` attribute on any element' do
+        exp = act = %q{<strong class="foo">Strong</strong>}
+        expect(filter(act).to_html).to eq exp
+      end
+
+      it 'allows `id` attribute on any element' do
+        exp = act = %q{<em id="foo">Emphasis</em>}
+        expect(filter(act).to_html).to eq exp
+      end
+
+      it 'allows `style` attribute on table elements' do
+        html = <<-HTML.strip_heredoc
+        <table>
+          <tr><th style="text-align: center">Head</th></tr>
+          <tr><td style="text-align: right">Body</th></tr>
+        </table>
+        HTML
+
+        doc = filter(html)
+
+        expect(doc.at_css('th')['style']).to eq 'text-align: center'
+        expect(doc.at_css('td')['style']).to eq 'text-align: right'
+      end
+
+      it 'allows `span` elements' do
+        exp = act = %q{<span>Hello</span>}
+        expect(filter(act).to_html).to eq exp
+      end
+
+      it 'removes `rel` attribute from `a` elements' do
+        doc = filter(%q{<a href="#" rel="nofollow">Link</a>})
+
+        expect(doc.css('a').size).to eq 1
+        expect(doc.at_css('a')['href']).to eq '#'
+        expect(doc.at_css('a')['rel']).to be_nil
+      end
+
+      it 'removes script-like `href` attribute from `a` elements' do
+        html = %q{<a href="javascript:alert('Hi')">Hi</a>}
+        doc = filter(html)
+
+        expect(doc.css('a').size).to eq 1
+        expect(doc.at_css('a')['href']).to be_nil
+      end
+    end
+  end
+end

spec/lib/gitlab/markdown/table_of_contents_filter_spec.rb

+# encoding: UTF-8
+
+require 'spec_helper'
+
+module Gitlab::Markdown
+  describe TableOfContentsFilter do
+    def filter(html, options = {})
+      described_class.call(html, options)
+    end
+
+    def header(level, text)
+      "<h#{level}>#{text}</h#{level}>\n"
+    end
+
+    it 'does nothing when :no_header_anchors is truthy' do
+      exp = act = header(1, 'Header')
+      expect(filter(act, no_header_anchors: 1).to_html).to eq exp
+    end
+
+    it 'does nothing with empty headers' do
+      exp = act = header(1, nil)
+      expect(filter(act).to_html).to eq exp
+    end
+
+    1.upto(6) do |i|
+      it "processes h#{i} elements" do
+        html = header(i, "Header #{i}")
+        doc = filter(html)
+
+        expect(doc.css("h#{i} a").first.attr('id')).to eq "header-#{i}"
+      end
+    end
+
+    describe 'anchor tag' do
+      it 'has an `anchor` class' do
+        doc = filter(header(1, 'Header'))
+        expect(doc.css('h1 a').first.attr('class')).to eq 'anchor'
+      end
+
+      it 'links to the id' do
+        doc = filter(header(1, 'Header'))
+        expect(doc.css('h1 a').first.attr('href')).to eq '#header'
+      end
+
+      describe 'generated IDs' do
+        it 'translates spaces to dashes' do
+          doc = filter(header(1, 'This header has spaces in it'))
+          expect(doc.css('h1 a').first.attr('id')).to eq 'this-header-has-spaces-in-it'
+        end
+
+        it 'squeezes multiple spaces and dashes' do
+          doc = filter(header(1, 'This---header     is poorly-formatted'))
+          expect(doc.css('h1 a').first.attr('id')).to eq 'this-header-is-poorly-formatted'
+        end
+
+        it 'removes punctuation' do
+          doc = filter(header(1, "This, header! is, filled. with @ punctuation?"))
+          expect(doc.css('h1 a').first.attr('id')).to eq 'this-header-is-filled-with-punctuation'
+        end
+
+        it 'appends a unique number to duplicates' do
+          doc = filter(header(1, 'One') + header(2, 'One'))
+
+          expect(doc.css('h1 a').first.attr('id')).to eq 'one'
+          expect(doc.css('h2 a').first.attr('id')).to eq 'one-1'
+        end
+
+        it 'supports Unicode' do
+          doc = filter(header(1, '한글'))
+          expect(doc.css('h1 a').first.attr('id')).to eq '한글'
+          expect(doc.css('h1 a').first.attr('href')).to eq '#한글'
+        end
+      end
+    end
+
+    describe 'result' do
+      def result(html)
+        HTML::Pipeline.new([described_class]).call(html)
+      end
+
+      let(:results) { result(header(1, 'Header 1') + header(2, 'Header 2')) }
+      let(:doc) { Nokogiri::XML::DocumentFragment.parse(results[:toc]) }
+
+      it 'is contained within a `ul` element' do
+        expect(doc.children.first.name).to eq 'ul'
+        expect(doc.children.first.attr('class')).to eq 'section-nav'
+      end
+
+      it 'contains an `li` element for each header' do
+        expect(doc.css('li').length).to eq 2
+
+        links = doc.css('li a')
+
+        expect(links.first.attr('href')).to eq '#header-1'
+        expect(links.first.text).to eq 'Header 1'
+        expect(links.last.attr('href')).to eq '#header-2'
+        expect(links.last.text).to eq 'Header 2'
+      end
+    end
+  end
+end

spec/models/project_spec.rb

@@ -129,6 +129,48 @@ describe Project do
     end
   end
 
+  describe '#get_issue' do
+    let(:project) { create(:empty_project) }
+    let(:issue)   { create(:issue, project: project) }
+
+    context 'with default issues tracker' do
+      it 'returns an issue' do
+        expect(project.get_issue(issue.iid)).to eq issue
+      end
+
+      it 'returns nil when no issue found' do
+        expect(project.get_issue(999)).to be_nil
+      end
+    end
+
+    context 'with external issues tracker' do
+      before do
+        allow(project).to receive(:default_issues_tracker?).and_return(false)
+      end
+
+      it 'returns an ExternalIssue' do
+        issue = project.get_issue('FOO-1234')
+        expect(issue).to be_kind_of(ExternalIssue)
+        expect(issue.iid).to eq 'FOO-1234'
+        expect(issue.project).to eq project
+      end
+    end
+  end
+
+  describe '#issue_exists?' do
+    let(:project) { create(:empty_project) }
+
+    it 'is truthy when issue exists' do
+      expect(project).to receive(:get_issue).and_return(double)
+      expect(project.issue_exists?(1)).to be_truthy
+    end
+
+    it 'is falsey when issue does not exist' do
+      expect(project).to receive(:get_issue).and_return(nil)
+      expect(project.issue_exists?(1)).to be_falsey
+    end
+  end
+
   describe :update_merge_requests do
     let(:project) { create(:project) }
     let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
@@ -180,25 +222,6 @@ describe Project do
     end
   end
 
-  describe :issue_exists? do
-    let(:project) { create(:project) }
-    let(:existed_issue) { create(:issue, project: project) }
-    let(:not_existed_issue) { create(:issue) }
-    let(:ext_project) { create(:redmine_project) }
-
-    it 'should be true or if used internal tracker and issue exists' do
-      expect(project.issue_exists?(existed_issue.iid)).to be_truthy
-    end
-
-    it 'should be false or if used internal tracker and issue not exists' do
-      expect(project.issue_exists?(not_existed_issue.iid)).to be_falsey
-    end
-
-    it 'should always be true if used other tracker' do
-      expect(ext_project.issue_exists?(rand(100))).to be_truthy
-    end
-  end
-
   describe :default_issues_tracker? do
     let(:project) { create(:project) }
     let(:ext_project) { create(:redmine_project) }