Commit 90aa870c authored by Christian Walther's avatar Christian Walther

Fix invalid Atom feeds when using emoji, horizontal rules, or images.

Fixes issues #880, #723, #1113: Markdown must be rendered to XHTML, not HTML, when generating summary content for Atom feeds. Otherwise, content-less tags like <img> and <hr>, generated when issue descriptions, merge request descriptions, comments, or commit messages use emoji, horizontal rules, or images, are not terminated and make the Atom XML invalid.
parent 71e14699
......@@ -2,6 +2,7 @@ v 7.9.0 (unreleased)
- Move labels/milestones tabs to sidebar
- Improve UI for commits, issues and merge request lists
- Fix commit comments on first line of diff not rendering in Merge Request Discussion view.
- 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.
......
%div{xmlns: "http://www.w3.org/1999/xhtml"}
- if issue.description.present?
= markdown issue.description
= markdown(issue.description, xhtml: true)
%div{xmlns: "http://www.w3.org/1999/xhtml"}
- if merge_request.description.present?
= markdown merge_request.description
= markdown(merge_request.description, xhtml: true)
%div{xmlns: "http://www.w3.org/1999/xhtml"}
= markdown note.note
= markdown(note.note, xhtml: true)
......@@ -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
......
......@@ -33,17 +33,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)
# 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(text, project = @project, html_options = {})
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
......@@ -86,14 +92,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
......
......@@ -58,10 +58,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
......@@ -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<hr ?\/>\n\n<p>I guess/
end
it 'should have XHTML summaries in notes' do
expect(body).to match /Bug confirmed <img[^>]*\/>/
end
it 'should have XHTML summaries in merge request descriptions' do
expect(body).to match /Here is the fix: <img[^>]*\/>/
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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment