Fixed problem with the content disposition with wiki attachments

Before https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/21362
all the uploads to the wiki where handle by the UploadsController. Now
that they are stored inside the wiki repo we are not using it any more.
Instead, we're using the old code in the WikisController#show.

By default, this code set the content disposition to 'inline',
that's why files like pdf were failing to display.

We're moving from that approach to benefit from the blob logic
and caching them and also mitigate the former error.

app/controllers/concerns/sends_blob.rb

@@ -8,7 +8,7 @@ module SendsBlob
     include SendFileUpload
   end
-  def send_blob(blob, params = {})
+  def send_blob(repository, blob, params = {})
     if blob
       headers['X-Content-Type-Options'] = 'nosniff'

app/controllers/projects/avatars_controller.rb

@@ -8,7 +8,7 @@ class Projects::AvatarsController < Projects::ApplicationController
   def show
     @blob = @repository.blob_at_branch(@repository.root_ref, @project.avatar_in_git)
-    send_blob(@blob)
+    send_blob(@repository, @blob)
   end
   def destroy

app/controllers/projects/raw_controller.rb

@@ -12,6 +12,6 @@ class Projects::RawController < Projects::ApplicationController
   def show
     @blob = @repository.blob_at(@commit.id, @path)
-    send_blob(@blob, inline: (params[:inline] != 'false'))
+    send_blob(@repository, @blob, inline: (params[:inline] != 'false'))
   end
 end

app/controllers/projects/wikis_controller.rb

@@ -2,6 +2,7 @@
 class Projects::WikisController < Projects::ApplicationController
   include PreviewMarkdown
+  include SendsBlob
   include Gitlab::Utils::StrongMemoize
   before_action :authorize_read_wiki!
@@ -26,16 +27,8 @@ class Projects::WikisController < Projects::ApplicationController
       set_encoding_error unless valid_encoding?
       render 'show'
-    elsif file = @project_wiki.find_file(params[:id], params[:version_id])
-      response.headers['Content-Security-Policy'] = "default-src 'none'"
-      response.headers['X-Content-Security-Policy'] = "default-src 'none'"
-
-      send_data(
-        file.raw_data,
-        type: file.mime_type,
-        disposition: 'inline',
-        filename: file.name
-      )
+    elsif file_blob
+      send_blob(@project_wiki.repository, file_blob)
     elsif can?(current_user, :create_wiki, @project) && view_param == 'create'
       @page = build_page(title: params[:id])
@@ -164,4 +157,14 @@ class Projects::WikisController < Projects::ApplicationController
   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."
   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

app/helpers/blob_helper.rb

@@ -150,7 +150,9 @@ module BlobHelper
   # example of Javascript) we tell the browser of the victim not to
   # execute untrusted data.
   def safe_content_type(blob)
-    if blob.text?
+    if blob.extension == 'svg'
+      blob.mime_type
+    elsif blob.text?
       'text/plain; charset=utf-8'
     elsif blob.image?
       blob.content_type
@@ -159,6 +161,12 @@ module BlobHelper
     end
   end
+  def content_disposition(blob, inline)
+    return 'attachment' if blob.extension == 'svg'
+
+    inline ? 'inline' : 'attachment'
+  end
+
   def ref_project
     @ref_project ||= @target_project || @project
   end

app/helpers/workhorse_helper.rb

@@ -6,7 +6,7 @@ module WorkhorseHelper
   # Send a Git blob through Workhorse
   def send_git_blob(repository, blob, inline: true)
     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)
     render plain: ""
   end

changelogs/unreleased/fj-52406-wiki-file-content-disposition.yml

+---
+title: Fix bug with wiki attachments content disposition
+merge_request: 22220
+author:
+type: fixed

spec/controllers/projects/wikis_controller_spec.rb

@@ -22,20 +22,18 @@ describe Projects::WikisController do
     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
-        expect(controller).to receive(:load_wiki).and_return(project_wiki)
-        # empty? call
-        expect(project_wiki).to receive(:pages).with(limit: 1).and_call_original
-        # Sidebar entries
-        expect(project_wiki).to receive(:pages).with(limit: 15).and_call_original
-        subject
-        expect(response).to have_http_status(:ok)
-        expect(response.body).to include(wiki_title)
-      end
+    it 'limits the retrieved pages for the sidebar' do
+      expect(controller).to receive(:load_wiki).and_return(project_wiki)
+      # empty? call
+      expect(project_wiki).to receive(:pages).with(limit: 1).and_call_original
+      # Sidebar entries
+      expect(project_wiki).to receive(:pages).with(limit: 15).and_call_original
+      subject
+      expect(response).to have_http_status(:ok)
+      expect(response.body).to include(wiki_title)
     end
     context 'when page content encoding is invalid' do
@@ -48,6 +46,34 @@ 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.'
       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
+    end
   end
   describe 'POST #preview_markdown' do

spec/features/projects/wiki/user_views_wiki_empty_spec.rb

@@ -54,7 +54,7 @@ describe 'User views empty wiki' do
     it_behaves_like 'empty wiki and non-accessible issues'
   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) }
     before do

spec/features/projects/wiki/user_views_wiki_page_spec.rb

@@ -2,12 +2,15 @@ require 'spec_helper'
 describe 'User views a wiki page' do
   shared_examples 'wiki page user view' do
+    include WikiHelpers
+
     let(:user) { create(:user) }
     let(:project) { create(:project, :wiki_repo, namespace: user.namespace) }
+    let(:path) { 'image.png' }
     let(:wiki_page) do
       create(:wiki_page,
         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
     before do
@@ -82,33 +85,26 @@ describe 'User views a wiki page' do
         expect(find('.wiki-pages')).to have_content(wiki_page.title.capitalize)
       end
-      it 'shows a file stored in a page' do
-        raw_file = Gitlab::GitalyClient::WikiFile.new(
-          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']")
-        expect(page).to have_link('image', href: "#{project.wiki.wiki_base_path}/image.jpg")
-        click_on('image')
-        expect(current_path).to match('wikis/image.jpg')
-        expect(page).not_to have_xpath('/html') # Page should render the image which means there is no html involved
+      context 'shows a file stored in a page' do
+        let(:path) { upload_file_to_wiki(project, user, 'dk.png') }
+        it do
+          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')
+          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
+        end
       end
       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')
-        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('Create page')
       end

spec/support/helpers/wiki_helpers.rb

+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