Commit b8cf360e authored by Francisco Javier López's avatar Francisco Javier López Committed by Douwe Maan

Fixed bug with the content disposition with wiki attachments

parent 11152ddf
...@@ -8,7 +8,7 @@ module SendsBlob ...@@ -8,7 +8,7 @@ module SendsBlob
include SendFileUpload include SendFileUpload
end end
def send_blob(blob, params = {}) def send_blob(repository, blob, params = {})
if blob if blob
headers['X-Content-Type-Options'] = 'nosniff' headers['X-Content-Type-Options'] = 'nosniff'
......
...@@ -8,7 +8,7 @@ class Projects::AvatarsController < Projects::ApplicationController ...@@ -8,7 +8,7 @@ class Projects::AvatarsController < Projects::ApplicationController
def show def show
@blob = @repository.blob_at_branch(@repository.root_ref, @project.avatar_in_git) @blob = @repository.blob_at_branch(@repository.root_ref, @project.avatar_in_git)
send_blob(@blob) send_blob(@repository, @blob)
end end
def destroy def destroy
......
...@@ -12,6 +12,6 @@ class Projects::RawController < Projects::ApplicationController ...@@ -12,6 +12,6 @@ class Projects::RawController < Projects::ApplicationController
def show def show
@blob = @repository.blob_at(@commit.id, @path) @blob = @repository.blob_at(@commit.id, @path)
send_blob(@blob, inline: (params[:inline] != 'false')) send_blob(@repository, @blob, inline: (params[:inline] != 'false'))
end end
end end
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
class Projects::WikisController < Projects::ApplicationController class Projects::WikisController < Projects::ApplicationController
include PreviewMarkdown include PreviewMarkdown
include SendsBlob
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
before_action :authorize_read_wiki! before_action :authorize_read_wiki!
...@@ -26,16 +27,8 @@ class Projects::WikisController < Projects::ApplicationController ...@@ -26,16 +27,8 @@ class Projects::WikisController < Projects::ApplicationController
set_encoding_error unless valid_encoding? set_encoding_error unless valid_encoding?
render 'show' render 'show'
elsif file = @project_wiki.find_file(params[:id], params[:version_id]) elsif file_blob
response.headers['Content-Security-Policy'] = "default-src 'none'" send_blob(@project_wiki.repository, file_blob)
response.headers['X-Content-Security-Policy'] = "default-src 'none'"
send_data(
file.raw_data,
type: file.mime_type,
disposition: 'inline',
filename: file.name
)
elsif can?(current_user, :create_wiki, @project) && view_param == 'create' elsif can?(current_user, :create_wiki, @project) && view_param == 'create'
@page = build_page(title: params[:id]) @page = build_page(title: params[:id])
...@@ -164,4 +157,14 @@ class Projects::WikisController < Projects::ApplicationController ...@@ -164,4 +157,14 @@ class Projects::WikisController < Projects::ApplicationController
def set_encoding_error def set_encoding_error
flash.now[:notice] = "The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository." flash.now[:notice] = "The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository."
end end
def file_blob
strong_memoize(:file_blob) do
commit = @project_wiki.repository.commit(@project_wiki.default_branch)
next unless commit
@project_wiki.repository.blob_at(commit.id, params[:id])
end
end
end end
...@@ -150,7 +150,9 @@ module BlobHelper ...@@ -150,7 +150,9 @@ module BlobHelper
# example of Javascript) we tell the browser of the victim not to # example of Javascript) we tell the browser of the victim not to
# execute untrusted data. # execute untrusted data.
def safe_content_type(blob) def safe_content_type(blob)
if blob.text? if blob.extension == 'svg'
blob.mime_type
elsif blob.text?
'text/plain; charset=utf-8' 'text/plain; charset=utf-8'
elsif blob.image? elsif blob.image?
blob.content_type blob.content_type
...@@ -159,6 +161,12 @@ module BlobHelper ...@@ -159,6 +161,12 @@ module BlobHelper
end end
end end
def content_disposition(blob, inline)
return 'attachment' if blob.extension == 'svg'
inline ? 'inline' : 'attachment'
end
def ref_project def ref_project
@ref_project ||= @target_project || @project @ref_project ||= @target_project || @project
end end
......
...@@ -6,7 +6,7 @@ module WorkhorseHelper ...@@ -6,7 +6,7 @@ module WorkhorseHelper
# Send a Git blob through Workhorse # Send a Git blob through Workhorse
def send_git_blob(repository, blob, inline: true) def send_git_blob(repository, blob, inline: true)
headers.store(*Gitlab::Workhorse.send_git_blob(repository, blob)) headers.store(*Gitlab::Workhorse.send_git_blob(repository, blob))
headers['Content-Disposition'] = inline ? 'inline' : 'attachment' headers['Content-Disposition'] = content_disposition(blob, inline)
headers['Content-Type'] = safe_content_type(blob) headers['Content-Type'] = safe_content_type(blob)
render plain: "" render plain: ""
end end
......
---
title: Fix bug with wiki attachments content disposition
merge_request: 22220
author:
type: fixed
...@@ -22,20 +22,18 @@ describe Projects::WikisController do ...@@ -22,20 +22,18 @@ describe Projects::WikisController do
subject { get :show, namespace_id: project.namespace, project_id: project, id: wiki_title } subject { get :show, namespace_id: project.namespace, project_id: project, id: wiki_title }
context 'when page content encoding is invalid' do it 'limits the retrieved pages for the sidebar' do
it 'limits the retrieved pages for the sidebar' do expect(controller).to receive(:load_wiki).and_return(project_wiki)
expect(controller).to receive(:load_wiki).and_return(project_wiki)
# empty? call # empty? call
expect(project_wiki).to receive(:pages).with(limit: 1).and_call_original expect(project_wiki).to receive(:pages).with(limit: 1).and_call_original
# Sidebar entries # Sidebar entries
expect(project_wiki).to receive(:pages).with(limit: 15).and_call_original expect(project_wiki).to receive(:pages).with(limit: 15).and_call_original
subject subject
expect(response).to have_http_status(:ok) expect(response).to have_http_status(:ok)
expect(response.body).to include(wiki_title) expect(response.body).to include(wiki_title)
end
end end
context 'when page content encoding is invalid' do context 'when page content encoding is invalid' do
...@@ -48,6 +46,42 @@ describe Projects::WikisController do ...@@ -48,6 +46,42 @@ describe Projects::WikisController do
expect(flash[:notice]).to eq 'The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository.' expect(flash[:notice]).to eq 'The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository.'
end end
end end
context 'when page is a file' do
include WikiHelpers
let(:path) { upload_file_to_wiki(project, user, file_name) }
before do
subject
end
subject { get :show, namespace_id: project.namespace, project_id: project, id: path }
context 'when file is an image' do
let(:file_name) { 'dk.png' }
it 'renders the content inline' do
expect(response.headers['Content-Disposition']).to match(/^inline/)
end
context 'when file is a svg' do
let(:file_name) { 'unsanitized.svg' }
it 'renders the content as an attachment' do
expect(response.headers['Content-Disposition']).to match(/^attachment/)
end
end
end
context 'when file is a pdf' do
let(:file_name) { 'git-cheat-sheet.pdf' }
it 'sets the content type to application/octet-stream' do
expect(response.headers['Content-Type']).to eq 'application/octet-stream'
end
end
end
end end
describe 'POST #preview_markdown' do describe 'POST #preview_markdown' do
......
...@@ -54,7 +54,7 @@ describe 'User views empty wiki' do ...@@ -54,7 +54,7 @@ describe 'User views empty wiki' do
it_behaves_like 'empty wiki and non-accessible issues' it_behaves_like 'empty wiki and non-accessible issues'
end end
context 'when user is logged in and a memeber' do context 'when user is logged in and a member' do
let(:project) { create(:project, :public, :wiki_repo) } let(:project) { create(:project, :public, :wiki_repo) }
before do before do
......
...@@ -2,12 +2,15 @@ require 'spec_helper' ...@@ -2,12 +2,15 @@ require 'spec_helper'
describe 'User views a wiki page' do describe 'User views a wiki page' do
shared_examples 'wiki page user view' do shared_examples 'wiki page user view' do
include WikiHelpers
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { create(:project, :wiki_repo, namespace: user.namespace) } let(:project) { create(:project, :wiki_repo, namespace: user.namespace) }
let(:path) { 'image.png' }
let(:wiki_page) do let(:wiki_page) do
create(:wiki_page, create(:wiki_page,
wiki: project.wiki, wiki: project.wiki,
attrs: { title: 'home', content: 'Look at this [image](image.jpg)\n\n ![alt text](image.jpg)' }) attrs: { title: 'home', content: "Look at this [image](#{path})\n\n ![alt text](#{path})" })
end end
before do before do
...@@ -82,33 +85,26 @@ describe 'User views a wiki page' do ...@@ -82,33 +85,26 @@ describe 'User views a wiki page' do
expect(find('.wiki-pages')).to have_content(wiki_page.title.capitalize) expect(find('.wiki-pages')).to have_content(wiki_page.title.capitalize)
end end
it 'shows a file stored in a page' do context 'shows a file stored in a page' do
raw_file = Gitlab::GitalyClient::WikiFile.new( let(:path) { upload_file_to_wiki(project, user, 'dk.png') }
mime_type: 'image/jpeg',
name: 'images/image.jpg',
path: 'images/image.jpg',
raw_data: ''
)
wiki_file = Gitlab::Git::WikiFile.new(raw_file)
allow(wiki_file).to receive(:mime_type).and_return('image/jpeg')
allow_any_instance_of(ProjectWiki).to receive(:find_file).with('image.jpg', nil).and_return(wiki_file)
expect(page).to have_xpath("//img[@data-src='#{project.wiki.wiki_base_path}/image.jpg']") it do
expect(page).to have_link('image', href: "#{project.wiki.wiki_base_path}/image.jpg") expect(page).to have_xpath("//img[@data-src='#{project.wiki.wiki_base_path}/#{path}']")
expect(page).to have_link('image', href: "#{project.wiki.wiki_base_path}/#{path}")
click_on('image') click_on('image')
expect(current_path).to match('wikis/image.jpg') expect(current_path).to match("wikis/#{path}")
expect(page).not_to have_xpath('/html') # Page should render the image which means there is no html involved expect(page).not_to have_xpath('/html') # Page should render the image which means there is no html involved
end
end end
it 'shows the creation page if file does not exist' do it 'shows the creation page if file does not exist' do
expect(page).to have_link('image', href: "#{project.wiki.wiki_base_path}/image.jpg") expect(page).to have_link('image', href: "#{project.wiki.wiki_base_path}/#{path}")
click_on('image') click_on('image')
expect(current_path).to match('wikis/image.jpg') expect(current_path).to match("wikis/#{path}")
expect(page).to have_content('New Wiki Page') expect(page).to have_content('New Wiki Page')
expect(page).to have_content('Create page') expect(page).to have_content('Create page')
end end
......
This diff is collapsed.
module WikiHelpers
extend self
def upload_file_to_wiki(project, user, file_name)
opts = {
file_name: file_name,
file_content: File.read(expand_fixture_path(file_name))
}
::Wikis::CreateAttachmentService.new(project, user, opts)
.execute[:result][:file_path]
end
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