diff --git a/app/assets/stylesheets/generic/typography.scss b/app/assets/stylesheets/generic/typography.scss index 80190424c1b056de0591b5ac855a78fb94ffbd84..36a9a540747d8aea5f9883ab3a9bb65f66928f8b 100644 --- a/app/assets/stylesheets/generic/typography.scss +++ b/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%; diff --git a/app/helpers/gitlab_markdown_helper.rb b/app/helpers/gitlab_markdown_helper.rb index bc234500000458ed94d118bee01b959c0514f5c2..24263a0f619e45bbba0fa44bff7b88ffe1fbb137 100644 --- a/app/helpers/gitlab_markdown_helper.rb +++ b/app/helpers/gitlab_markdown_helper.rb @@ -34,10 +34,8 @@ 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 - escape_html: false + # Handled further down the line by Gitlab::Markdown::SanitizationFilter + escape_html: false }.merge(options)) # see https://github.com/vmg/redcarpet#and-its-like-really-simple-to-use @@ -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, diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index c3b4731dff303616d9689ca84e3d0686d0d5e425..36d3f371c1b69408a36df65180523ef104780dd8 100644 --- a/app/helpers/issues_helper.rb +++ b/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 diff --git a/app/models/external_issue.rb b/app/models/external_issue.rb index 50efcb32f1b9f93c41dd20e858a9405e97e4543a..85fdb12bfdc7a332d84cc90992ded77a6e027a72 100644 --- a/app/models/external_issue.rb +++ b/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 diff --git a/app/models/project.rb b/app/models/project.rb index 397232e98d8a46c0d8ebf57e5ad86b0c2f04a307..e866681aab947e736a0f071f0bd3fccd87bbbe94 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -329,14 +329,18 @@ 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 gitlab_issue_tracker_service || create_gitlab_issue_tracker_service end @@ -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 diff --git a/features/steps/shared/markdown.rb b/features/steps/shared/markdown.rb index e71700880cde96622a3a307ad47e79deaba25e80..a7231c47d14306829efc0653454a4fa46728d601 100644 --- a/features/steps/shared/markdown.rb +++ b/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) diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb index 44779d7fdd82c746c476668f2934c49a39f55f19..8348e28d0f51b92b4bc6fc5abe1a5fc27c5cb779 100644 --- a/lib/gitlab/markdown.rb +++ b/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">' diff --git a/lib/gitlab/markdown/autolink_filter.rb b/lib/gitlab/markdown/autolink_filter.rb new file mode 100644 index 0000000000000000000000000000000000000000..4e14a048cfb3dc96e3cb9c4ca7024b1347499de3 --- /dev/null +++ b/lib/gitlab/markdown/autolink_filter.rb @@ -0,0 +1,100 @@ +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 diff --git a/lib/gitlab/markdown/issue_reference_filter.rb b/lib/gitlab/markdown/issue_reference_filter.rb index 4b360369d3768f5905c05fd41aa9b275e9706f6f..1e885615163092b9756417e544bad8af59ee8ea5 100644 --- a/lib/gitlab/markdown/issue_reference_filter.rb +++ b/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 diff --git a/lib/gitlab/markdown/merge_request_reference_filter.rb b/lib/gitlab/markdown/merge_request_reference_filter.rb index 7c28fe112efcd0b3a8cb3bf9c811b7bb6e1ec50a..740d72abb3699025c6cdaf25fbb01c5764784d60 100644 --- a/lib/gitlab/markdown/merge_request_reference_filter.rb +++ b/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, diff --git a/lib/gitlab/markdown/sanitization_filter.rb b/lib/gitlab/markdown/sanitization_filter.rb new file mode 100644 index 0000000000000000000000000000000000000000..9a154e0b2feb08447b1e5bd462e61c7d8d8d95d2 --- /dev/null +++ b/lib/gitlab/markdown/sanitization_filter.rb @@ -0,0 +1,38 @@ +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 diff --git a/lib/gitlab/markdown/table_of_contents_filter.rb b/lib/gitlab/markdown/table_of_contents_filter.rb new file mode 100644 index 0000000000000000000000000000000000000000..c97612dafb8eca4510be2688aa27e3e271cbd60c --- /dev/null +++ b/lib/gitlab/markdown/table_of_contents_filter.rb @@ -0,0 +1,62 @@ +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 diff --git a/lib/gitlab/theme.rb b/lib/gitlab/theme.rb index f0e61aa2e81ad6b8c94652f7b8b55c814e04fc48..e5a1f1b44d96b28edae0007406cbafa2b09931a0 100644 --- a/lib/gitlab/theme.rb +++ b/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 diff --git a/lib/redcarpet/render/gitlab_html.rb b/lib/redcarpet/render/gitlab_html.rb index 10efff2ae9f12f7b250c82f2cd7907a27bd7e32b..bea66e6cdc12ee10c1eeefdc702a708e7368a5aa 100644 --- a/lib/redcarpet/render/gitlab_html.rb +++ b/lib/redcarpet/render/gitlab_html.rb @@ -1,5 +1,6 @@ -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("'", "’") + 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!("’", "'") 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 diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 57fa079d753894f7b9f542325222f4888f1b70ce..102678a1d74f1baa768ed9cd9971e04c68193872 100644 --- a/spec/factories/projects.rb +++ b/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 diff --git a/spec/features/markdown_spec.rb b/spec/features/markdown_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..3528200e12bdf445e7d1c01ee6eb473f954847d3 --- /dev/null +++ b/spec/features/markdown_spec.rb @@ -0,0 +1,391 @@ +require 'spec_helper' +require 'erb' + +# This feature spec is intended to be a comprehensive exercising of all of +# GitLab's non-standard Markdown parsing and the integration thereof. +# +# These tests should be very high-level. Anything low-level belongs in the specs +# for the corresponding HTML::Pipeline filter or helper method. +# +# The idea is to pass a Markdown document through our entire processing stack. +# +# The process looks like this: +# +# Raw Markdown +# -> `markdown` helper +# -> Redcarpet::Render::GitlabHTML converts Markdown to HTML +# -> Post-process HTML +# -> `gfm_with_options` helper +# -> HTML::Pipeline +# -> Sanitize +# -> Emoji +# -> Table of Contents +# -> Autolinks +# -> Rinku (http, https, ftp) +# -> Other schemes +# -> References +# -> `html_safe` +# -> Template +# +# See the MarkdownFeature class for setup details. + +describe 'GitLab Markdown' do + include ActionView::Helpers::TagHelper + include ActionView::Helpers::UrlHelper + include Capybara::Node::Matchers + include GitlabMarkdownHelper + + # `markdown` calls these two methods + def current_user + @feat.user + end + + def user_color_scheme_class + :white + end + + # Let's only parse this thing once + before(:all) do + @feat = MarkdownFeature.new + + # `markdown` expects a `@project` variable + @project = @feat.project + + @md = markdown(@feat.raw_markdown) + @doc = Nokogiri::HTML::DocumentFragment.parse(@md) + end + + after(:all) do + @feat.teardown + end + + # Given a header ID, goes to that element's parent (the header), then to its + # second sibling (the body). + def get_section(id) + @doc.at_css("##{id}").parent.next.next + end + + # it 'writes to a file' do + # File.open(Rails.root.join('tmp/capybara/markdown_spec.html'), 'w') do |file| + # file.puts @md + # end + # end + + describe 'Markdown' do + describe 'No Intra Emphasis' do + it 'does not parse emphasis inside of words' do + body = get_section('no-intra-emphasis') + expect(body.to_html).not_to match('foo<em>bar</em>baz') + end + end + + describe 'Tables' do + it 'parses table Markdown' do + body = get_section('tables') + expect(body).to have_selector('th:contains("Header")') + expect(body).to have_selector('th:contains("Row")') + expect(body).to have_selector('th:contains("Example")') + end + + it 'allows Markdown in tables' do + expect(@doc.at_css('td:contains("Baz")').children.to_html). + to eq '<strong>Baz</strong>' + end + end + + describe 'Fenced Code Blocks' do + it 'parses fenced code blocks' do + expect(@doc).to have_selector('pre.code.highlight.white.c') + expect(@doc).to have_selector('pre.code.highlight.white.python') + end + end + + describe 'Strikethrough' do + it 'parses strikethroughs' do + expect(@doc).to have_selector(%{del:contains("and this text doesn't")}) + end + end + + describe 'Superscript' do + it 'parses superscript' do + body = get_section('superscript') + expect(body.to_html).to match('1<sup>st</sup>') + expect(body.to_html).to match('2<sup>nd</sup>') + end + end + end + + describe 'HTML::Pipeline' do + describe 'SanitizationFilter' do + it 'uses a permissive whitelist' do + expect(@doc).to have_selector('b#manual-b') + expect(@doc).to have_selector('em#manual-em') + expect(@doc).to have_selector("code#manual-code") + expect(@doc).to have_selector('kbd:contains("s")') + expect(@doc).to have_selector('strike:contains(Emoji)') + expect(@doc).to have_selector('img#manual-img') + expect(@doc).to have_selector('br#manual-br') + expect(@doc).to have_selector('hr#manual-hr') + end + + it 'permits span elements' do + expect(@doc).to have_selector('span#span-class-light.light') + end + + it 'permits table alignment' do + expect(@doc.at_css('th:contains("Header")')['style']).to eq 'text-align: center' + expect(@doc.at_css('th:contains("Row")')['style']).to eq 'text-align: right' + expect(@doc.at_css('th:contains("Example")')['style']).to eq 'text-align: left' + + expect(@doc.at_css('td:contains("Foo")')['style']).to eq 'text-align: center' + expect(@doc.at_css('td:contains("Bar")')['style']).to eq 'text-align: right' + expect(@doc.at_css('td:contains("Baz")')['style']).to eq 'text-align: left' + end + + it 'removes `rel` attribute from links' do + expect(@doc).to have_selector('a#a-rel-nofollow') + expect(@doc).not_to have_selector('a#a-rel-nofollow[rel]') + end + + it "removes `href` from `a` elements if it's fishy" do + expect(@doc).to have_selector('a#a-href-javascript') + expect(@doc).not_to have_selector('a#a-href-javascript[href]') + end + end + + describe 'Escaping' do + let(:table) { @doc.css('table').last.at_css('tbody') } + + it 'escapes non-tag angle brackets' do + expect(table.at_xpath('.//tr[1]/td[3]').inner_html).to eq '1 < 3 & 5' + end + end + + describe 'EmojiFilter' do + it 'parses Emoji' do + expect(@doc).to have_selector('img.emoji', count: 10) + end + end + + describe 'TableOfContentsFilter' do + it 'creates anchors inside header elements' do + expect(@doc).to have_selector('h1 a#gitlab-markdown') + expect(@doc).to have_selector('h2 a#markdown') + expect(@doc).to have_selector('h3 a#autolinkfilter') + end + end + + describe 'AutolinkFilter' do + let(:list) { get_section('autolinkfilter').parent.search('ul') } + + def item(index) + list.at_css("li:nth-child(#{index})") + end + + it 'autolinks http://' do + expect(item(1).children.first.name).to eq 'a' + expect(item(1).children.first['href']).to eq 'http://about.gitlab.com/' + end + + it 'autolinks https://' do + expect(item(2).children.first.name).to eq 'a' + expect(item(2).children.first['href']).to eq 'https://google.com/' + end + + it 'autolinks ftp://' do + expect(item(3).children.first.name).to eq 'a' + expect(item(3).children.first['href']).to eq 'ftp://ftp.us.debian.org/debian/' + end + + it 'autolinks smb://' do + expect(item(4).children.first.name).to eq 'a' + expect(item(4).children.first['href']).to eq 'smb://foo/bar/baz' + end + + it 'autolinks irc://' do + expect(item(5).children.first.name).to eq 'a' + expect(item(5).children.first['href']).to eq 'irc://irc.freenode.net/git' + end + + it 'autolinks short, invalid URLs' do + expect(item(6).children.first.name).to eq 'a' + expect(item(6).children.first['href']).to eq 'http://localhost:3000' + end + + %w(code a kbd).each do |elem| + it "ignores links inside '#{elem}' element" do + expect(@doc.at_css("#{elem}#autolink-#{elem}").child).to be_text + end + end + end + + describe 'ReferenceFilter' do + it 'handles references in headers' do + header = @doc.at_css('#reference-filters-eg-1').parent + + expect(header.css('a').size).to eq 2 + end + + it "handles references in Markdown" do + body = get_section('reference-filters-eg-1') + expect(body).to have_selector('em a.gfm-merge_request', count: 1) + end + + it 'parses user references' do + body = get_section('userreferencefilter') + expect(body).to have_selector('a.gfm.gfm-project_member', count: 3) + end + + it 'parses issue references' do + body = get_section('issuereferencefilter') + expect(body).to have_selector('a.gfm.gfm-issue', count: 2) + end + + it 'parses merge request references' do + body = get_section('mergerequestreferencefilter') + expect(body).to have_selector('a.gfm.gfm-merge_request', count: 2) + end + + it 'parses snippet references' do + body = get_section('snippetreferencefilter') + expect(body).to have_selector('a.gfm.gfm-snippet', count: 2) + end + + it 'parses commit range references' do + body = get_section('commitrangereferencefilter') + expect(body).to have_selector('a.gfm.gfm-commit_range', count: 2) + end + + it 'parses commit references' do + body = get_section('commitreferencefilter') + expect(body).to have_selector('a.gfm.gfm-commit', count: 2) + end + + it 'parses label references' do + body = get_section('labelreferencefilter') + expect(body).to have_selector('a.gfm.gfm-label', count: 3) + end + end + end +end + +# This is a helper class used by the GitLab Markdown feature spec +# +# Because the feature spec only cares about the output of the Markdown, and the +# test setup and teardown and parsing is fairly expensive, we only want to do it +# once. Unfortunately RSpec will not let you access `let`s in a `before(:all)` +# block, so we fake it by encapsulating all the shared setup in this class. +# +# The class contains the raw Markup used in the test, dynamically substituting +# real objects, created from factories and setup on-demand, when referenced in +# the Markdown. +class MarkdownFeature + include FactoryGirl::Syntax::Methods + + def initialize + DatabaseCleaner.start + end + + def teardown + DatabaseCleaner.clean + end + + def user + @user ||= create(:user) + end + + def group + unless @group + @group = create(:group) + @group.add_user(user, Gitlab::Access::DEVELOPER) + end + + @group + end + + # Direct references ---------------------------------------------------------- + + def project + @project ||= create(:project) + end + + def issue + @issue ||= create(:issue, project: project) + end + + def merge_request + @merge_request ||= create(:merge_request, :simple, source_project: project) + end + + def snippet + @snippet ||= create(:project_snippet, project: project) + end + + def commit + @commit ||= project.repository.commit + end + + def commit_range + unless @commit_range + commit2 = project.repository.commit('HEAD~3') + @commit_range = CommitRange.new("#{commit.id}...#{commit2.id}") + end + + @commit_range + end + + def simple_label + @simple_label ||= create(:label, name: 'gfm', project: project) + end + + def label + @label ||= create(:label, name: 'awaiting feedback', project: project) + end + + # Cross-references ----------------------------------------------------------- + + def xproject + unless @xproject + namespace = create(:namespace, name: 'cross-reference') + @xproject = create(:project, namespace: namespace) + @xproject.team << [user, :developer] + end + + @xproject + end + + # Shortcut to "cross-reference/project" + def xref + xproject.path_with_namespace + end + + def xissue + @xissue ||= create(:issue, project: xproject) + end + + def xmerge_request + @xmerge_request ||= create(:merge_request, :simple, source_project: xproject) + end + + def xsnippet + @xsnippet ||= create(:project_snippet, project: xproject) + end + + def xcommit + @xcommit ||= xproject.repository.commit + end + + def xcommit_range + unless @xcommit_range + xcommit2 = xproject.repository.commit('HEAD~2') + @xcommit_range = CommitRange.new("#{xcommit.id}...#{xcommit2.id}") + end + + @xcommit_range + end + + def raw_markdown + fixture = Rails.root.join('spec/fixtures/markdown.md.erb') + ERB.new(File.read(fixture)).result(binding) + end +end diff --git a/spec/fixtures/markdown.md.erb b/spec/fixtures/markdown.md.erb new file mode 100644 index 0000000000000000000000000000000000000000..0c1407585572d3e57ce10af744c665c344daae7d --- /dev/null +++ b/spec/fixtures/markdown.md.erb @@ -0,0 +1,178 @@ +# 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 < 3 & 5 | 1 < 3 & 5 | +| `<foo>` | <foo> | <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) diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index e309dbb6a2f9a6429b8b35aaec9de3ac6802f7b2..b6be82e41096483ce568a0f54eb5f35e4cff8beb 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/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) ")).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: ")).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 diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb index 54dd8d4aa64dd87686e982499c9e3940f2c8f740..c08ddb4cae1bd6ba25dd6a110b8cb9cead4a67c6 100644 --- a/spec/helpers/issues_helper_spec.rb +++ b/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 diff --git a/spec/lib/gitlab/markdown/autolink_filter_spec.rb b/spec/lib/gitlab/markdown/autolink_filter_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..0bbdc11a9798f6b82c3a7f8632ac63e4f6fe09f8 --- /dev/null +++ b/spec/lib/gitlab/markdown/autolink_filter_spec.rb @@ -0,0 +1,106 @@ +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 diff --git a/spec/lib/gitlab/markdown/external_issue_reference_filter_spec.rb b/spec/lib/gitlab/markdown/external_issue_reference_filter_spec.rb index 27e930ef7daf3df4aae5a249808fa21158aada4f..b19bc125b92cc7eb406df611b9849f704fae3a11 100644 --- a/spec/lib/gitlab/markdown/external_issue_reference_filter_spec.rb +++ b/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/) diff --git a/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb b/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb index 393bf32e19610c632fd310b53af2e8f4e7920902..08382b3e7e87a2c718358423dfe159861f215ffb 100644 --- a/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb +++ b/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 diff --git a/spec/lib/gitlab/markdown/sanitization_filter_spec.rb b/spec/lib/gitlab/markdown/sanitization_filter_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..ab909a686351ac469097b6176d0d0158cd81b126 --- /dev/null +++ b/spec/lib/gitlab/markdown/sanitization_filter_spec.rb @@ -0,0 +1,81 @@ +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 diff --git a/spec/lib/gitlab/markdown/table_of_contents_filter_spec.rb b/spec/lib/gitlab/markdown/table_of_contents_filter_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..f383a5850d5419ca307b45f317225d31f993d938 --- /dev/null +++ b/spec/lib/gitlab/markdown/table_of_contents_filter_spec.rb @@ -0,0 +1,101 @@ +# 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 diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 879a63dd9f954585e08e46ce4618ce62e6dd84c5..37e21a9081835ad0b1cb7db750e83aad048c3f79 100644 --- a/spec/models/project_spec.rb +++ b/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) }