Skip to content
Snippets Groups Projects
Commit 6cbdc90c authored by Luke Duncalfe's avatar Luke Duncalfe
Browse files

Pass all wiki markup formats through pipelines

Previously, when the wiki page format was anything other than `markdown`
or `asciidoc` the formatted content would be returned though a Gitaly
call. Gitaly in turn would delegate formatting to the gitlab-gollum-lib
gem, which in turn would delegate that to various gems (like RDoc for
`rdoc`) and then apply some very liberal sanitization.

It was too liberal!

This change brings our wiki content formatting in line with how we
format other markdown at GitLab, so we have a SSOT for sanitization.

https://gitlab.com/gitlab-org/gitlab/issues/30540
parent 635e1578
No related branches found
No related tags found
No related merge requests found
Loading
Loading
@@ -133,15 +133,7 @@ module MarkupHelper
issuable_state_filter_enabled: true
)
 
html =
case wiki_page.format
when :markdown
markdown_unsafe(text, context)
when :asciidoc
asciidoc_unsafe(text)
else
wiki_page.formatted_content.html_safe
end
html = markup_unsafe(wiki_page.path, text, context)
 
prepare_for_rendering(html, context)
end
Loading
Loading
Loading
Loading
@@ -138,6 +138,12 @@ class WikiPage
@version ||= @page.version
end
 
def path
return unless persisted?
@path ||= @page.path
end
def versions(options = {})
return [] unless persisted?
 
Loading
Loading
---
title: Sanitize all wiki markup formats with GitLab sanitization pipelines
merge_request:
author:
type: security
Loading
Loading
@@ -10,7 +10,7 @@ module Gitlab
def self.render(file_name, input, context)
html = GitHub::Markup.render(file_name, input)
.force_encoding(input.encoding)
context[:pipeline] = :markup
context[:pipeline] ||= :markup
 
html = Banzai.render(html, context)
 
Loading
Loading
require 'spec_helper'
 
describe MarkupHelper do
let!(:project) { create(:project, :repository) }
let(:user) { create(:user, username: 'gfm') }
let(:commit) { project.commit }
let(:issue) { create(:issue, project: project) }
let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
let(:snippet) { create(:project_snippet, project: project) }
before do
# Ensure the generated reference links aren't redacted
set(:project) { create(:project, :repository) }
set(:user) do
user = create(:user, username: 'gfm')
project.add_maintainer(user)
user
end
set(:issue) { create(:issue, project: project) }
set(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
set(:snippet) { create(:project_snippet, project: project) }
let(:commit) { project.commit }
 
before do
# Helper expects a @project instance variable
helper.instance_variable_set(:@project, project)
 
Loading
Loading
@@ -42,8 +42,8 @@ describe MarkupHelper do
 
describe "override default project" do
let(:actual) { issue.to_reference }
let(:second_project) { create(:project, :public) }
let(:second_issue) { create(:issue, project: second_project) }
set(:second_project) { create(:project, :public) }
set(:second_issue) { create(:issue, project: second_project) }
 
it 'links to the issue' do
expected = urls.project_issue_path(second_project, second_issue)
Loading
Loading
@@ -53,7 +53,7 @@ describe MarkupHelper do
 
describe 'uploads' do
let(:text) { "![ImageTest](/uploads/test.png)" }
let(:group) { create(:group) }
set(:group) { create(:group) }
 
subject { helper.markdown(text) }
 
Loading
Loading
@@ -75,7 +75,7 @@ describe MarkupHelper do
end
 
describe "with a group in the context" do
let(:project_in_group) { create(:project, group: group) }
set(:project_in_group) { create(:project, group: group) }
 
before do
helper.instance_variable_set(:@group, group)
Loading
Loading
@@ -233,38 +233,48 @@ describe MarkupHelper do
end
 
describe '#render_wiki_content' do
let(:wiki) { double('WikiPage', path: "file.#{extension}") }
let(:context) do
{
pipeline: :wiki, project: project, project_wiki: wiki,
page_slug: 'nested/page', issuable_state_filter_enabled: true
}
end
before do
@wiki = double('WikiPage')
allow(@wiki).to receive(:content).and_return('wiki content')
allow(@wiki).to receive(:slug).and_return('nested/page')
helper.instance_variable_set(:@project_wiki, @wiki)
expect(wiki).to receive(:content).and_return('wiki content')
expect(wiki).to receive(:slug).and_return('nested/page')
helper.instance_variable_set(:@project_wiki, wiki)
end
 
it "uses Wiki pipeline for markdown files" do
allow(@wiki).to receive(:format).and_return(:markdown)
context 'when file is Markdown' do
let(:extension) { 'md' }
 
expect(helper).to receive(:markdown_unsafe).with('wiki content',
pipeline: :wiki, project: project, project_wiki: @wiki, page_slug: "nested/page",
issuable_state_filter_enabled: true)
it 'renders using #markdown_unsafe helper method' do
expect(helper).to receive(:markdown_unsafe).with('wiki content', context)
 
helper.render_wiki_content(@wiki)
helper.render_wiki_content(wiki)
end
end
 
it "uses Asciidoctor for asciidoc files" do
allow(@wiki).to receive(:format).and_return(:asciidoc)
context 'when file is Asciidoc' do
let(:extension) { 'adoc' }
 
expect(helper).to receive(:asciidoc_unsafe).with('wiki content')
it 'renders using Gitlab::Asciidoc' do
expect(Gitlab::Asciidoc).to receive(:render)
 
helper.render_wiki_content(@wiki)
helper.render_wiki_content(wiki)
end
end
 
it "uses the Gollum renderer for all other file types" do
allow(@wiki).to receive(:format).and_return(:rdoc)
formatted_content_stub = double('formatted_content')
expect(formatted_content_stub).to receive(:html_safe)
allow(@wiki).to receive(:formatted_content).and_return(formatted_content_stub)
context 'any other format' do
let(:extension) { 'foo' }
 
helper.render_wiki_content(@wiki)
it 'renders all other formats using Gitlab::OtherMarkup' do
expect(Gitlab::OtherMarkup).to receive(:render)
helper.render_wiki_content(wiki)
end
end
end
 
Loading
Loading
Loading
Loading
@@ -439,6 +439,23 @@ describe WikiPage do
end
end
 
describe '#path' do
let(:path) { 'mypath.md' }
let(:wiki_page) { instance_double('Gitlab::Git::WikiPage', path: path).as_null_object }
it 'returns the path when persisted' do
page = described_class.new(wiki, wiki_page, true)
expect(page.path).to eq(path)
end
it 'returns nil when not persisted' do
page = described_class.new(wiki, wiki_page, false)
expect(page.path).to be_nil
end
end
describe '#directory' do
context 'when the page is at the root directory' do
it 'returns an empty string' do
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment