From 2a5c963b7cd2dc1cf1e6b4d1a291c37d313b5813 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 28 May 2015 17:56:52 -0400 Subject: Render Group and Project descriptions with our Markdown pipeline --- app/assets/stylesheets/pages/projects.scss | 10 ++++++---- app/views/groups/show.html.haml | 2 +- app/views/projects/_home_panel.html.haml | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 12489ccc2d7..b93ea0f020e 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -48,14 +48,16 @@ } .project-home-desc { + color: $gray; + float: left; font-size: 16px; line-height: 1.3; margin-right: 250px; - } - .project-home-desc { - float: left; - color: $gray; + // Render Markdown-generated HTML inline for this block + p { + display: inline; + } } } diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml index 1678311141e..f42007da073 100644 --- a/app/views/groups/show.html.haml +++ b/app/views/groups/show.html.haml @@ -11,7 +11,7 @@ @#{@group.path} - if @group.description.present? .description - = escaped_autolink(@group.description) + = markdown(@group.description) %hr = render 'shared/show_aside' diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml index f9cdda4a3ba..05f44acd3cb 100644 --- a/app/views/projects/_home_panel.html.haml +++ b/app/views/projects/_home_panel.html.haml @@ -5,7 +5,7 @@ .project-home-row.project-home-row-top .project-home-desc - if @project.description.present? - = escaped_autolink(@project.description) + = markdown(@project.description) - if can?(current_user, :admin_project, @project) – = link_to 'Edit', edit_namespace_project_path -- cgit v1.2.1 From 1a52f19c456dfa307dd7fa0e5adbaa2ed1a68889 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 28 May 2015 17:57:23 -0400 Subject: Remove now-unused `escaped_autolink` helper and rails_autolink gem --- Gemfile | 3 --- Gemfile.lock | 3 --- app/helpers/application_helper.rb | 4 ---- 3 files changed, 10 deletions(-) diff --git a/Gemfile b/Gemfile index 78af7f5db69..94e2129f3c6 100644 --- a/Gemfile +++ b/Gemfile @@ -10,9 +10,6 @@ end gem "rails", "~> 4.1.0" -# Make links from text -gem 'rails_autolink', '~> 1.1' - # Default values for AR models gem "default_value_for", "~> 3.0.0" diff --git a/Gemfile.lock b/Gemfile.lock index bbc5639c84f..80ae41dc8fc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -448,8 +448,6 @@ GEM sprockets-rails (~> 2.0) rails-observers (0.1.2) activemodel (~> 4.0) - rails_autolink (1.1.6) - rails (> 3.1) railties (4.1.9) actionpack (= 4.1.9) activesupport (= 4.1.9) @@ -779,7 +777,6 @@ DEPENDENCIES rack-mini-profiler rack-oauth2 (~> 1.0.5) rails (~> 4.1.0) - rails_autolink (~> 1.1) raphael-rails (~> 2.1.2) rb-fsevent rb-inotify diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 89dcdf57798..a539ec49f7a 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -279,10 +279,6 @@ module ApplicationHelper html_options end - def escaped_autolink(text) - auto_link ERB::Util.html_escape(text), link: :urls - end - def promo_host 'about.gitlab.com' end -- cgit v1.2.1 From 023dd2907b4afa0bae5f8482cae75e1edd6954a8 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 29 May 2015 19:01:12 -0400 Subject: Add a `pipeline` context option for SanitizationFilter When this option is `:description`, we use a more restrictive whitelist. This is used for Project and Group description fields. --- app/views/groups/show.html.haml | 2 +- app/views/projects/_home_panel.html.haml | 2 +- lib/gitlab/markdown.rb | 3 ++ lib/gitlab/markdown/sanitization_filter.rb | 58 ++++++++++++++-------- .../gitlab/markdown/sanitization_filter_spec.rb | 14 ++++++ 5 files changed, 56 insertions(+), 23 deletions(-) diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml index f42007da073..0687840af39 100644 --- a/app/views/groups/show.html.haml +++ b/app/views/groups/show.html.haml @@ -11,7 +11,7 @@ @#{@group.path} - if @group.description.present? .description - = markdown(@group.description) + = markdown(@group.description, pipeline: :description) %hr = render 'shared/show_aside' diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml index 05f44acd3cb..076afb11a9d 100644 --- a/app/views/projects/_home_panel.html.haml +++ b/app/views/projects/_home_panel.html.haml @@ -5,7 +5,7 @@ .project-home-row.project-home-row-top .project-home-desc - if @project.description.present? - = markdown(@project.description) + = markdown(@project.description, pipeline: :description) - if can?(current_user, :admin_project, @project) – = link_to 'Edit', edit_namespace_project_path diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb index 5db1566f55d..fa9c0975bb8 100644 --- a/lib/gitlab/markdown.rb +++ b/lib/gitlab/markdown.rb @@ -57,6 +57,9 @@ module Gitlab pipeline = HTML::Pipeline.new(filters) context = { + # SanitizationFilter + pipeline: options[:pipeline], + # EmojiFilter asset_root: Gitlab.config.gitlab.url, asset_host: Gitlab::Application.config.asset_host, diff --git a/lib/gitlab/markdown/sanitization_filter.rb b/lib/gitlab/markdown/sanitization_filter.rb index 88781fea0c8..fc29d09081a 100644 --- a/lib/gitlab/markdown/sanitization_filter.rb +++ b/lib/gitlab/markdown/sanitization_filter.rb @@ -8,33 +8,53 @@ module Gitlab # Extends HTML::Pipeline::SanitizationFilter with a custom whitelist. class SanitizationFilter < HTML::Pipeline::SanitizationFilter def whitelist - whitelist = super + # Descriptions are more heavily sanitized, allowing only a few elements. + # See http://git.io/vkuAN + if pipeline == :description + whitelist = LIMITED + else + whitelist = super + end + + customize_whitelist(whitelist) + + whitelist + end + private + + def pipeline + context[:pipeline] || :default + end + + def customized?(transformers) + transformers.last.source_location[0] == __FILE__ + end + + def customize_whitelist(whitelist) # Only push these customizations once - unless customized?(whitelist[:transformers]) - # Allow code highlighting - whitelist[:attributes]['pre'] = %w(class) - whitelist[:attributes]['span'] = %w(class) + return if customized?(whitelist[:transformers]) - # Allow table alignment - whitelist[:attributes]['th'] = %w(style) - whitelist[:attributes]['td'] = %w(style) + # Allow code highlighting + whitelist[:attributes]['pre'] = %w(class) + whitelist[:attributes]['span'] = %w(class) - # Allow span elements - whitelist[:elements].push('span') + # Allow table alignment + whitelist[:attributes]['th'] = %w(style) + whitelist[:attributes]['td'] = %w(style) - # Remove `rel` attribute from `a` elements - whitelist[:transformers].push(remove_rel) + # Allow span elements + whitelist[:elements].push('span') - # Remove `class` attribute from non-highlight spans - whitelist[:transformers].push(clean_spans) - end + # Remove `rel` attribute from `a` elements + whitelist[:transformers].push(remove_rel) + + # Remove `class` attribute from non-highlight spans + whitelist[:transformers].push(clean_spans) whitelist end - private - def remove_rel lambda do |env| if env[:node_name] == 'a' @@ -53,10 +73,6 @@ module Gitlab end end end - - def customized?(transformers) - transformers.last.source_location[0] == __FILE__ - end end end end diff --git a/spec/lib/gitlab/markdown/sanitization_filter_spec.rb b/spec/lib/gitlab/markdown/sanitization_filter_spec.rb index 4a1aa766149..80f3d2f2634 100644 --- a/spec/lib/gitlab/markdown/sanitization_filter_spec.rb +++ b/spec/lib/gitlab/markdown/sanitization_filter_spec.rb @@ -42,6 +42,13 @@ module Gitlab::Markdown end describe 'custom whitelist' do + it 'customizes the whitelist only once' do + instance = described_class.new('Foo') + 3.times { instance.whitelist } + + expect(instance.whitelist[:transformers].size).to eq 4 + end + it 'allows syntax highlighting' do exp = act = %q{
def
} expect(filter(act).to_html).to eq exp @@ -87,5 +94,12 @@ module Gitlab::Markdown expect(doc.at_css('a')['href']).to be_nil end end + + context 'when pipeline is :description' do + it 'uses a stricter whitelist' do + doc = filter('

My Project

', pipeline: :description) + expect(doc.to_html.strip).to eq 'My Project' + end + end end end -- cgit v1.2.1 From 442a0663da437abcdec7fbd86967b6d8980d4090 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 29 May 2015 19:02:11 -0400 Subject: Add feature specs for Project and Group description rendering --- spec/features/groups_spec.rb | 36 +++++++++++++++++++++++++++ spec/features/markdown_spec.rb | 2 ++ spec/features/projects_spec.rb | 56 +++++++++++++++++++++++++++++++++--------- 3 files changed, 82 insertions(+), 12 deletions(-) create mode 100644 spec/features/groups_spec.rb diff --git a/spec/features/groups_spec.rb b/spec/features/groups_spec.rb new file mode 100644 index 00000000000..edc1c63a0aa --- /dev/null +++ b/spec/features/groups_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +feature 'Group' do + describe 'description' do + let(:group) { create(:group) } + let(:path) { group_path(group) } + + before do + login_as(:admin) + end + + it 'parses Markdown' do + group.update_attribute(:description, 'This is **my** group') + visit path + expect(page).to have_css('.description > p > strong') + end + + it 'passes through html-pipeline' do + group.update_attribute(:description, 'This group is the :poop:') + visit path + expect(page).to have_css('.description > p > img') + end + + it 'sanitizes unwanted tags' do + group.update_attribute(:description, '# Group Description') + visit path + expect(page).not_to have_css('.description h1') + end + + it 'permits `rel` attribute on links' do + group.update_attribute(:description, 'https://google.com/') + visit path + expect(page).to have_css('.description a[rel]') + end + end +end diff --git a/spec/features/markdown_spec.rb b/spec/features/markdown_spec.rb index ee1b3bf749d..902968cebcb 100644 --- a/spec/features/markdown_spec.rb +++ b/spec/features/markdown_spec.rb @@ -18,11 +18,13 @@ require 'erb' # -> `gfm_with_options` helper # -> HTML::Pipeline # -> Sanitize +# -> RelativeLink # -> Emoji # -> Table of Contents # -> Autolinks # -> Rinku (http, https, ftp) # -> Other schemes +# -> ExternalLink # -> References # -> TaskList # -> `html_safe` diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index cae11be7cdd..56523f6e1a8 100644 --- a/spec/features/projects_spec.rb +++ b/spec/features/projects_spec.rb @@ -1,24 +1,56 @@ require 'spec_helper' -describe "Projects", feature: true, js: true do - before { login_as :user } +feature 'Project' do + describe 'description' do + let(:project) { create(:project) } + let(:path) { namespace_project_path(project.namespace, project) } - describe "DELETE /projects/:id" do before do - @project = create(:project, namespace: @user.namespace) - @project.team << [@user, :master] - visit edit_namespace_project_path(@project.namespace, @project) + login_as(:admin) end - it "should remove project" do + it 'parses Markdown' do + project.update_attribute(:description, 'This is **my** project') + visit path + expect(page).to have_css('.project-home-desc > p > strong') + end + + it 'passes through html-pipeline' do + project.update_attribute(:description, 'This project is the :poop:') + visit path + expect(page).to have_css('.project-home-desc > p > img') + end + + it 'sanitizes unwanted tags' do + project.update_attribute(:description, '# Project Description') + visit path + expect(page).not_to have_css('.project-home-desc h1') + end + + it 'permits `rel` attribute on links' do + project.update_attribute(:description, 'https://google.com/') + visit path + expect(page).to have_css('.project-home-desc a[rel]') + end + end + + describe 'removal', js: true do + let(:user) { create(:user) } + let(:project) { create(:project, namespace: user.namespace) } + + before do + login_with(user) + project.team << [user, :master] + visit edit_namespace_project_path(project.namespace, project) + end + + it 'should remove project' do expect { remove_project }.to change {Project.count}.by(-1) end it 'should delete the project from disk' do - expect(GitlabShellWorker).to( - receive(:perform_async).with(:remove_repository, - /#{@project.path_with_namespace}/) - ).twice + expect(GitlabShellWorker).to receive(:perform_async). + with(:remove_repository, /#{project.path_with_namespace}/).twice remove_project end @@ -26,7 +58,7 @@ describe "Projects", feature: true, js: true do def remove_project click_link "Remove project" - fill_in 'confirm_name_input', with: @project.path + fill_in 'confirm_name_input', with: project.path click_button 'Confirm' end end -- cgit v1.2.1 From 79c4e3899fa7697afdefb13d64c4add08ca84aac Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 2 Jun 2015 13:27:53 -0400 Subject: Rename ReferenceFilterSpecHelper to FilterSpecHelper And make it more generalized for all filter specs. --- spec/lib/gitlab/markdown/autolink_filter_spec.rb | 6 +- .../markdown/commit_range_reference_filter_spec.rb | 2 +- .../markdown/commit_reference_filter_spec.rb | 2 +- spec/lib/gitlab/markdown/emoji_filter_spec.rb | 4 +- .../external_issue_reference_filter_spec.rb | 2 +- .../gitlab/markdown/external_link_filter_spec.rb | 4 +- .../gitlab/markdown/issue_reference_filter_spec.rb | 2 +- .../gitlab/markdown/label_reference_filter_spec.rb | 2 +- .../merge_request_reference_filter_spec.rb | 2 +- .../gitlab/markdown/sanitization_filter_spec.rb | 4 +- .../markdown/snippet_reference_filter_spec.rb | 2 +- .../markdown/table_of_contents_filter_spec.rb | 4 +- spec/lib/gitlab/markdown/task_list_filter_spec.rb | 4 +- .../gitlab/markdown/user_reference_filter_spec.rb | 2 +- spec/support/filter_spec_helper.rb | 77 ++++++++++++++++++++++ spec/support/reference_filter_spec_helper.rb | 70 -------------------- 16 files changed, 92 insertions(+), 97 deletions(-) create mode 100644 spec/support/filter_spec_helper.rb delete mode 100644 spec/support/reference_filter_spec_helper.rb diff --git a/spec/lib/gitlab/markdown/autolink_filter_spec.rb b/spec/lib/gitlab/markdown/autolink_filter_spec.rb index 0bbdc11a979..a14cb2da089 100644 --- a/spec/lib/gitlab/markdown/autolink_filter_spec.rb +++ b/spec/lib/gitlab/markdown/autolink_filter_spec.rb @@ -2,11 +2,9 @@ require 'spec_helper' module Gitlab::Markdown describe AutolinkFilter do - let(:link) { 'http://about.gitlab.com/' } + include FilterSpecHelper - def filter(html, options = {}) - described_class.call(html, options) - end + let(:link) { 'http://about.gitlab.com/' } it 'does nothing when :autolink is false' do exp = act = link diff --git a/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb b/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb index d3695ee46d0..e8391cc7aca 100644 --- a/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' module Gitlab::Markdown describe CommitRangeReferenceFilter do - include ReferenceFilterSpecHelper + include FilterSpecHelper let(:project) { create(:project) } let(:commit1) { project.commit } diff --git a/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb b/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb index a0d2cd7e22b..a10d43c9a02 100644 --- a/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' module Gitlab::Markdown describe CommitReferenceFilter do - include ReferenceFilterSpecHelper + include FilterSpecHelper let(:project) { create(:project) } let(:commit) { project.commit } diff --git a/spec/lib/gitlab/markdown/emoji_filter_spec.rb b/spec/lib/gitlab/markdown/emoji_filter_spec.rb index 18d55c4818f..11efd9bb4cd 100644 --- a/spec/lib/gitlab/markdown/emoji_filter_spec.rb +++ b/spec/lib/gitlab/markdown/emoji_filter_spec.rb @@ -2,9 +2,7 @@ require 'spec_helper' module Gitlab::Markdown describe EmojiFilter do - def filter(html, contexts = {}) - described_class.call(html, contexts) - end + include FilterSpecHelper before do ActionController::Base.asset_host = 'https://foo.com' 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 bf9409589fa..f16095bc2b2 100644 --- a/spec/lib/gitlab/markdown/external_issue_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/external_issue_reference_filter_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' module Gitlab::Markdown describe ExternalIssueReferenceFilter do - include ReferenceFilterSpecHelper + include FilterSpecHelper def helper IssuesHelper diff --git a/spec/lib/gitlab/markdown/external_link_filter_spec.rb b/spec/lib/gitlab/markdown/external_link_filter_spec.rb index c2ff4f80a42..a040b34577b 100644 --- a/spec/lib/gitlab/markdown/external_link_filter_spec.rb +++ b/spec/lib/gitlab/markdown/external_link_filter_spec.rb @@ -2,9 +2,7 @@ require 'spec_helper' module Gitlab::Markdown describe ExternalLinkFilter do - def filter(html, options = {}) - described_class.call(html, options) - end + include FilterSpecHelper it 'ignores elements without an href attribute' do exp = act = %q(Ignore Me) diff --git a/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb b/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb index a838d7570c8..fa43d33794d 100644 --- a/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' module Gitlab::Markdown describe IssueReferenceFilter do - include ReferenceFilterSpecHelper + include FilterSpecHelper def helper IssuesHelper diff --git a/spec/lib/gitlab/markdown/label_reference_filter_spec.rb b/spec/lib/gitlab/markdown/label_reference_filter_spec.rb index 41987f57bca..cf3337b1ba1 100644 --- a/spec/lib/gitlab/markdown/label_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/label_reference_filter_spec.rb @@ -3,7 +3,7 @@ require 'html/pipeline' module Gitlab::Markdown describe LabelReferenceFilter do - include ReferenceFilterSpecHelper + include FilterSpecHelper let(:project) { create(:empty_project) } let(:label) { create(:label, project: project) } diff --git a/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb b/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb index 6aeb1093602..5945302a2da 100644 --- a/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' module Gitlab::Markdown describe MergeRequestReferenceFilter do - include ReferenceFilterSpecHelper + include FilterSpecHelper let(:project) { create(:project) } let(:merge) { create(:merge_request, source_project: project) } diff --git a/spec/lib/gitlab/markdown/sanitization_filter_spec.rb b/spec/lib/gitlab/markdown/sanitization_filter_spec.rb index 80f3d2f2634..8627cb288ab 100644 --- a/spec/lib/gitlab/markdown/sanitization_filter_spec.rb +++ b/spec/lib/gitlab/markdown/sanitization_filter_spec.rb @@ -2,9 +2,7 @@ require 'spec_helper' module Gitlab::Markdown describe SanitizationFilter do - def filter(html, options = {}) - described_class.call(html, options) - end + include FilterSpecHelper describe 'default whitelist' do it 'sanitizes tags that are not whitelisted' do diff --git a/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb b/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb index 07ece66e903..38619a3c07f 100644 --- a/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' module Gitlab::Markdown describe SnippetReferenceFilter do - include ReferenceFilterSpecHelper + include FilterSpecHelper let(:project) { create(:empty_project) } let(:snippet) { create(:project_snippet, project: project) } diff --git a/spec/lib/gitlab/markdown/table_of_contents_filter_spec.rb b/spec/lib/gitlab/markdown/table_of_contents_filter_spec.rb index f383a5850d5..ddf583a72c1 100644 --- a/spec/lib/gitlab/markdown/table_of_contents_filter_spec.rb +++ b/spec/lib/gitlab/markdown/table_of_contents_filter_spec.rb @@ -4,9 +4,7 @@ require 'spec_helper' module Gitlab::Markdown describe TableOfContentsFilter do - def filter(html, options = {}) - described_class.call(html, options) - end + include FilterSpecHelper def header(level, text) "#{text}\n" diff --git a/spec/lib/gitlab/markdown/task_list_filter_spec.rb b/spec/lib/gitlab/markdown/task_list_filter_spec.rb index 2a1e1cc5127..94f39cc966e 100644 --- a/spec/lib/gitlab/markdown/task_list_filter_spec.rb +++ b/spec/lib/gitlab/markdown/task_list_filter_spec.rb @@ -2,9 +2,7 @@ require 'spec_helper' module Gitlab::Markdown describe TaskListFilter do - def filter(html, options = {}) - described_class.call(html, options) - end + include FilterSpecHelper it 'does not apply `task-list` class to non-task lists' do exp = act = %(
  • Item
) diff --git a/spec/lib/gitlab/markdown/user_reference_filter_spec.rb b/spec/lib/gitlab/markdown/user_reference_filter_spec.rb index 0ecbdee9b9e..08e6941028f 100644 --- a/spec/lib/gitlab/markdown/user_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/user_reference_filter_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' module Gitlab::Markdown describe UserReferenceFilter do - include ReferenceFilterSpecHelper + include FilterSpecHelper let(:project) { create(:empty_project) } let(:user) { create(:user) } diff --git a/spec/support/filter_spec_helper.rb b/spec/support/filter_spec_helper.rb new file mode 100644 index 00000000000..755964e9a3d --- /dev/null +++ b/spec/support/filter_spec_helper.rb @@ -0,0 +1,77 @@ +# Helper methods for Gitlab::Markdown filter specs +# +# Must be included into specs manually +module FilterSpecHelper + extend ActiveSupport::Concern + + # Perform `call` on the described class + # + # Automatically passes the current `project` value, if defined, to the context + # if none is provided. + # + # html - HTML String to pass to the filter's `call` method. + # contexts - Hash context for the filter. (default: {project: project}) + # + # Returns a Nokogiri::XML::DocumentFragment + def filter(html, contexts = {}) + if defined?(project) + contexts.reverse_merge!(project: project) + end + + described_class.call(html, contexts) + end + + # Run text through HTML::Pipeline with the current filter and return the + # result Hash + # + # body - String text to run through the pipeline + # contexts - Hash context for the filter. (default: {project: project}) + # + # Returns the Hash + def pipeline_result(body, contexts = {}) + contexts.reverse_merge!(project: project) + + pipeline = HTML::Pipeline.new([described_class], contexts) + pipeline.call(body) + end + + # Modify a String reference to make it invalid + # + # Commit SHAs get reversed, IDs get incremented by 1, all other Strings get + # their word characters reversed. + # + # reference - String reference to modify + # + # Returns a String + def invalidate_reference(reference) + if reference =~ /\A(.+)?.\d+\z/ + # Integer-based reference with optional project prefix + reference.gsub(/\d+\z/) { |i| i.to_i + 1 } + elsif reference =~ /\A(.+@)?(\h{6,40}\z)/ + # SHA-based reference with optional prefix + reference.gsub(/\h{6,40}\z/) { |v| v.reverse } + else + reference.gsub(/\w+\z/) { |v| v.reverse } + end + end + + # Stub CrossProjectReference#user_can_reference_project? to return true for + # the current test + def allow_cross_reference! + allow_any_instance_of(described_class). + to receive(:user_can_reference_project?).and_return(true) + end + + # Stub CrossProjectReference#user_can_reference_project? to return false for + # the current test + def disallow_cross_reference! + allow_any_instance_of(described_class). + to receive(:user_can_reference_project?).and_return(false) + end + + # Shortcut to Rails' auto-generated routes helpers, to avoid including the + # module + def urls + Rails.application.routes.url_helpers + end +end diff --git a/spec/support/reference_filter_spec_helper.rb b/spec/support/reference_filter_spec_helper.rb deleted file mode 100644 index afbea55ab99..00000000000 --- a/spec/support/reference_filter_spec_helper.rb +++ /dev/null @@ -1,70 +0,0 @@ -# Common methods and setup for Gitlab::Markdown reference filter specs -# -# Must be included into specs manually -module ReferenceFilterSpecHelper - extend ActiveSupport::Concern - - # Shortcut to Rails' auto-generated routes helpers, to avoid including the - # module - def urls - Rails.application.routes.url_helpers - end - - # Modify a String reference to make it invalid - # - # Commit SHAs get reversed, IDs get incremented by 1, all other Strings get - # their word characters reversed. - # - # reference - String reference to modify - # - # Returns a String - def invalidate_reference(reference) - if reference =~ /\A(.+)?.\d+\z/ - # Integer-based reference with optional project prefix - reference.gsub(/\d+\z/) { |i| i.to_i + 1 } - elsif reference =~ /\A(.+@)?(\h{6,40}\z)/ - # SHA-based reference with optional prefix - reference.gsub(/\h{6,40}\z/) { |v| v.reverse } - else - reference.gsub(/\w+\z/) { |v| v.reverse } - end - end - - # Perform `call` on the described class - # - # Automatically passes the current `project` value to the context if none is - # provided. - # - # html - String text to pass to the filter's `call` method. - # contexts - Hash context for the filter. (default: {project: project}) - # - # Returns the String text returned by the filter's `call` method. - def filter(html, contexts = {}) - contexts.reverse_merge!(project: project) - described_class.call(html, contexts) - end - - # Run text through HTML::Pipeline with the current filter and return the - # result Hash - # - # body - String text to run through the pipeline - # contexts - Hash context for the filter. (default: {project: project}) - # - # Returns the Hash of the pipeline result - def pipeline_result(body, contexts = {}) - contexts.reverse_merge!(project: project) - - pipeline = HTML::Pipeline.new([described_class], contexts) - pipeline.call(body) - end - - def allow_cross_reference! - allow_any_instance_of(described_class). - to receive(:user_can_reference_project?).and_return(true) - end - - def disallow_cross_reference! - allow_any_instance_of(described_class). - to receive(:user_can_reference_project?).and_return(false) - end -end -- cgit v1.2.1 From 9e7a9c63a59f4e673271b3600b735e3fa6702432 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 2 Jun 2015 13:41:12 -0400 Subject: Further limit the limited whitelist for project/group descriptions --- lib/gitlab/markdown/sanitization_filter.rb | 1 + spec/lib/gitlab/markdown/sanitization_filter_spec.rb | 19 +++++++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/markdown/sanitization_filter.rb b/lib/gitlab/markdown/sanitization_filter.rb index fc29d09081a..74b3a8d274f 100644 --- a/lib/gitlab/markdown/sanitization_filter.rb +++ b/lib/gitlab/markdown/sanitization_filter.rb @@ -12,6 +12,7 @@ module Gitlab # See http://git.io/vkuAN if pipeline == :description whitelist = LIMITED + whitelist[:elements] -= %w(pre code img ol ul li) else whitelist = super end diff --git a/spec/lib/gitlab/markdown/sanitization_filter_spec.rb b/spec/lib/gitlab/markdown/sanitization_filter_spec.rb index 8627cb288ab..e50c82d0b3c 100644 --- a/spec/lib/gitlab/markdown/sanitization_filter_spec.rb +++ b/spec/lib/gitlab/markdown/sanitization_filter_spec.rb @@ -95,8 +95,23 @@ module Gitlab::Markdown context 'when pipeline is :description' do it 'uses a stricter whitelist' do - doc = filter('

My Project

', pipeline: :description) - expect(doc.to_html.strip).to eq 'My Project' + doc = filter('

Description

', pipeline: :description) + expect(doc.to_html.strip).to eq 'Description' + end + + %w(pre code img ol ul li).each do |elem| + it "removes '#{elem}' elements" do + act = "<#{elem}>Description" + expect(filter(act, pipeline: :description).to_html.strip). + to eq 'Description' + end + end + + %w(b i strong em a ins del sup sub p).each do |elem| + it "still allows '#{elem}' elements" do + exp = act = "<#{elem}>Description" + expect(filter(act, pipeline: :description).to_html).to eq exp + end end end end -- cgit v1.2.1