diff --git a/CHANGELOG b/CHANGELOG index bd66a92933d..ec30b09b90b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -101,6 +101,9 @@ v 7.8.1 - Fix urls for the issues when relative url was enabled v 7.8.0 + - Fix invalid Atom feeds when using emoji, horizontal rules, or images (Christian Walther) + +v 7.8.0 (unreleased) - Fix access control and protection against XSS for note attachments and other uploads. - Replace highlight.js with rouge-fork rugments (Stefan Tatschner) - Make project search case insensitive (Hannes Rosenögger) diff --git a/app/views/events/_event_issue.atom.haml b/app/views/events/_event_issue.atom.haml index eba2b63797a..0edb61ea246 100644 --- a/app/views/events/_event_issue.atom.haml +++ b/app/views/events/_event_issue.atom.haml @@ -1,3 +1,3 @@ %div{xmlns: "http://www.w3.org/1999/xhtml"} - if issue.description.present? - = markdown issue.description + = markdown(issue.description, xhtml: true) diff --git a/app/views/events/_event_merge_request.atom.haml b/app/views/events/_event_merge_request.atom.haml index 0aea2d17d65..1a8b62abeab 100644 --- a/app/views/events/_event_merge_request.atom.haml +++ b/app/views/events/_event_merge_request.atom.haml @@ -1,3 +1,3 @@ %div{xmlns: "http://www.w3.org/1999/xhtml"} - if merge_request.description.present? - = markdown merge_request.description + = markdown(merge_request.description, xhtml: true) diff --git a/app/views/events/_event_note.atom.haml b/app/views/events/_event_note.atom.haml index be0e05481ed..b49c331ccf2 100644 --- a/app/views/events/_event_note.atom.haml +++ b/app/views/events/_event_note.atom.haml @@ -1,2 +1,2 @@ %div{xmlns: "http://www.w3.org/1999/xhtml"} - = markdown note.note + = markdown(note.note, xhtml: true) diff --git a/app/views/events/_event_push.atom.haml b/app/views/events/_event_push.atom.haml index 0ffd2aa0b98..5d14def8f75 100644 --- a/app/views/events/_event_push.atom.haml +++ b/app/views/events/_event_push.atom.haml @@ -6,7 +6,7 @@ %i at = commit[:timestamp].to_time.to_s(:short) - %blockquote= markdown(escape_once(commit[:message])) + %blockquote= markdown(escape_once(commit[:message]), xhtml: true) - if event.commits_count > 15 %p %i diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb index c3a8d90ef54..e02e5b9fc3d 100644 --- a/lib/gitlab/markdown.rb +++ b/lib/gitlab/markdown.rb @@ -34,17 +34,23 @@ module Gitlab attr_reader :html_options - def gfm_with_tasks(text, project = @project, html_options = {}) - text = gfm(text, project, html_options) - parse_tasks(text) - end - # Public: Parse the provided text with GitLab-Flavored Markdown # # text - the source text # project - extra options for the reference links as given to link_to # html_options - extra options for the reference links as given to link_to def gfm(text, project = @project, html_options = {}) + gfm_with_options(text, {}, project, html_options) + end + + # Public: Parse the provided text with GitLab-Flavored Markdown + # + # text - the source text + # options - parse_tasks: true - render tasks + # - xhtml: true - output XHTML instead of HTML + # project - extra options for the reference links as given to link_to + # html_options - extra options for the reference links as given to link_to + def gfm_with_options(text, options = {}, project = @project, html_options = {}) return text if text.nil? # Duplicate the string so we don't alter the original, then call to_str @@ -87,14 +93,22 @@ module Gitlab markdown_pipeline = HTML::Pipeline::Gitlab.new(filters).pipeline result = markdown_pipeline.call(text, markdown_context) - text = result[:output].to_html(save_with: 0) + saveoptions = 0 + if options[:xhtml] + saveoptions |= Nokogiri::XML::Node::SaveOptions::AS_XHTML + end + text = result[:output].to_html(save_with: saveoptions) allowed_attributes = ActionView::Base.sanitized_allowed_attributes allowed_tags = ActionView::Base.sanitized_allowed_tags - sanitize text.html_safe, - attributes: allowed_attributes + %w(id class style), - tags: allowed_tags + %w(table tr td th) + text = sanitize text.html_safe, + attributes: allowed_attributes + %w(id class style), + tags: allowed_tags + %w(table tr td th) + if options[:parse_tasks] + text = parse_tasks(text) + end + text end private diff --git a/lib/redcarpet/render/gitlab_html.rb b/lib/redcarpet/render/gitlab_html.rb index 1cd3933e4b7..10efff2ae9f 100644 --- a/lib/redcarpet/render/gitlab_html.rb +++ b/lib/redcarpet/render/gitlab_html.rb @@ -67,10 +67,6 @@ class Redcarpet::Render::GitlabHTML < Redcarpet::Render::HTML unless @template.instance_variable_get("@project_wiki") || @project.nil? full_document = h.create_relative_links(full_document) end - if @options[:parse_tasks] - h.gfm_with_tasks(full_document) - else - h.gfm(full_document) - end + h.gfm_with_options(full_document, @options) end end diff --git a/spec/features/atom/users_spec.rb b/spec/features/atom/users_spec.rb index c0316b073ad..770ac04c2c5 100644 --- a/spec/features/atom/users_spec.rb +++ b/spec/features/atom/users_spec.rb @@ -15,17 +15,24 @@ describe "User Feed", feature: true do let(:project) { create(:project) } let(:issue) do create(:issue, project: project, - author: user, description: '') + author: user, description: "Houston, we have a bug!\n\n***\n\nI guess.") end let(:note) do create(:note, noteable: issue, author: user, - note: 'Bug confirmed', project: project) + note: 'Bug confirmed :+1:', project: project) + end + let(:merge_request) do + create(:merge_request, + title: 'Fix bug', author: user, + source_project: project, target_project: project, + description: "Here is the fix: ![an image](image.png)") end before do project.team << [user, :master] issue_event(issue, user) note_event(note, user) + merge_request_event(merge_request, user) visit user_path(user, :atom, private_token: user.private_token) end @@ -37,6 +44,18 @@ describe "User Feed", feature: true do expect(body). to have_content("#{safe_name} commented on issue ##{issue.iid}") end + + it 'should have XHTML summaries in issue descriptions' do + expect(body).to match /we have a bug!<\/p>\n\n
\n\n

I guess/ + end + + it 'should have XHTML summaries in notes' do + expect(body).to match /Bug confirmed ]*\/>/ + end + + it 'should have XHTML summaries in merge request descriptions' do + expect(body).to match /Here is the fix: ]*\/>/ + end end end @@ -48,6 +67,10 @@ describe "User Feed", feature: true do EventCreateService.new.leave_note(note, user) end + def merge_request_event(request, user) + EventCreateService.new.open_mr(request, user) + end + def safe_name html_escape(user.name) end