Fix bug with Wiki pages encoding

app/controllers/projects/wikis_controller.rb

@@ -1,10 +1,16 @@
 class Projects::WikisController < Projects::ApplicationController
   include PreviewMarkdown
+  include Gitlab::Utils::StrongMemoize
 
   before_action :authorize_read_wiki!
   before_action :authorize_create_wiki!, only: [:edit, :create, :history]
   before_action :authorize_admin_wiki!, only: :destroy
   before_action :load_project_wiki
+  before_action :load_page, only: [:show, :edit, :update, :history, :destroy]
+  before_action :valid_encoding?, only: [:show, :edit, :update], if: :load_page
+  before_action only: [:edit, :update], unless: :valid_encoding? do
+    redirect_to(project_wiki_path(@project, @page))
+  end
 
   def pages
     @wiki_pages = Kaminari.paginate_array(@project_wiki.pages).page(params[:page])
@@ -12,11 +18,11 @@ class Projects::WikisController < Projects::ApplicationController
   end
 
   def show
-    @page = @project_wiki.find_page(params[:id], params[:version_id])
-
     view_param = @project_wiki.empty? ? params[:view] : 'create'
 
     if @page
+      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'"
@@ -38,13 +44,11 @@ class Projects::WikisController < Projects::ApplicationController
   end
 
   def edit
-    @page = @project_wiki.find_page(params[:id])
   end
 
   def update
     return render('empty') unless can?(current_user, :create_wiki, @project)
 
-    @page = @project_wiki.find_page(params[:id])
     @page = WikiPages::UpdateService.new(@project, current_user, wiki_params).execute(@page)
 
     if @page.valid?
@@ -79,8 +83,6 @@ class Projects::WikisController < Projects::ApplicationController
   end
 
   def history
-    @page = @project_wiki.find_page(params[:id])
-
     if @page
       @page_versions = Kaminari.paginate_array(@page.versions(page: params[:page].to_i),
                                                total_count: @page.count_versions)
@@ -94,8 +96,6 @@ class Projects::WikisController < Projects::ApplicationController
   end
 
   def destroy
-    @page = @project_wiki.find_page(params[:id])
-
     WikiPages::DestroyService.new(@project, current_user).execute(@page)
 
     redirect_to project_wiki_path(@project, :home),
@@ -141,4 +141,25 @@ class Projects::WikisController < Projects::ApplicationController
       page.update_attributes(args) # rubocop:disable Rails/ActiveRecordAliases
     end
   end
+
+  def load_page
+    @page ||= @project_wiki.find_page(*page_params)
+  end
+
+  def page_params
+    keys = [:id]
+    keys << :version_id if params[:action] == 'show'
+
+    params.values_at(*keys)
+  end
+
+  def valid_encoding?
+    strong_memoize(:valid_encoding) do
+      @page.content.encoding == Encoding::UTF_8
+    end
+  end
+
+  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
 end

app/views/projects/wikis/_main_links.html.haml

@@ -4,6 +4,6 @@
       = s_("Wiki|New page")
   = link_to project_wiki_history_path(@project, @page), class: "btn" do
     = s_("Wiki|Page history")
-  - if can?(current_user, :create_wiki, @project) && @page.latest?
+  - if can?(current_user, :create_wiki, @project) && @page.latest? && @valid_encoding
     = link_to project_wiki_edit_path(@project, @page), class: "btn js-wiki-edit" do
       = _("Edit")

changelogs/unreleased/fj-49512-fix-gitlab-git-pages-encoding.yml

@@ -0,0 +1,5 @@
+---
+title: Prevent editing and updating wiki pages with non UTF-8 encoding via web interface
+merge_request: 20906
+author:
+type: fixed

spec/controllers/projects/wikis_controller_spec.rb

@@ -2,50 +2,131 @@ require 'spec_helper'
 
 describe Projects::WikisController do
   let(:project) { create(:project, :public, :repository) }
-  let(:user) { create(:user) }
+  let(:user) { project.owner }
-  let(:wiki) { ProjectWiki.new(project, user) }
+  let(:project_wiki) { ProjectWiki.new(project, user) }
+  let(:wiki) { project_wiki.wiki }
+  let(:wiki_title) { 'page-title-test' }
+
+  before do
+    create_page(wiki_title, 'hello world')
+
+    sign_in(user)
+  end
+
+  after do
+    destroy_page(wiki_title)
+  end
 
   describe 'GET #show' do
-    let(:wiki_title) { 'page-title-test' }
-
     render_views
 
-    before do
+    subject { get :show, namespace_id: project.namespace, project_id: project, id: wiki_title }
-      create_page(wiki_title, 'hello world')
+
+    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
     end
 
-    it 'limits the retrieved pages for the sidebar' do
+    context 'when page content encoding is invalid' do
-      sign_in(user)
+      it 'sets flash error' do
+        allow(controller).to receive(:valid_encoding?).and_return(false)
 
-      expect(controller).to receive(:load_wiki).and_return(wiki)
+        subject
 
-      # empty? call
+        expect(response).to have_http_status(:ok)
-      expect(wiki).to receive(:pages).with(limit: 1).and_call_original
+        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.'
-      # Sidebar entries
+      end
-      expect(wiki).to receive(:pages).with(limit: 15).and_call_original
-
-      get :show, namespace_id: project.namespace, project_id: project, id: wiki_title
-
-      expect(response).to have_http_status(:ok)
-      expect(response.body).to include(wiki_title)
     end
   end
 
   describe 'POST #preview_markdown' do
     it 'renders json in a correct format' do
-      sign_in(user)
-
       post :preview_markdown, namespace_id: project.namespace, project_id: project, id: 'page/path', text: '*Markdown* text'
 
       expect(JSON.parse(response.body).keys).to match_array(%w(body references))
     end
   end
 
+  describe 'GET #edit' do
+    subject { get(:edit, namespace_id: project.namespace, project_id: project, id: wiki_title) }
+
+    context 'when page content encoding is invalid' do
+      it 'redirects to show' do
+        allow(controller).to receive(:valid_encoding?).and_return(false)
+
+        subject
+
+        expect(response).to redirect_to(project_wiki_path(project, project_wiki.pages.first))
+      end
+    end
+
+    context 'when page content encoding is valid' do
+      render_views
+
+      it 'shows the edit page' do
+        subject
+
+        expect(response).to have_http_status(:ok)
+        expect(response.body).to include('Edit Page')
+      end
+    end
+  end
+
+  describe 'PATCH #update' do
+    let(:new_title) { 'New title' }
+    let(:new_content) { 'New content' }
+    subject do
+      patch(:update,
+            namespace_id: project.namespace,
+            project_id: project,
+            id: wiki_title,
+            wiki: { title: new_title, content: new_content })
+    end
+
+    context 'when page content encoding is invalid' do
+      it 'redirects to show' do
+        allow(controller).to receive(:valid_encoding?).and_return(false)
+
+        subject
+        expect(response).to redirect_to(project_wiki_path(project, project_wiki.pages.first))
+      end
+    end
+
+    context 'when page content encoding is valid' do
+      render_views
+
+      it 'updates the page' do
+        subject
+
+        wiki_page = project_wiki.pages.first
+
+        expect(wiki_page.title).to eq new_title
+        expect(wiki_page.content).to eq new_content
+      end
+    end
+  end
+
   def create_page(name, content)
-    project.wiki.wiki.write_page(name, :markdown, content, commit_details(name))
+    wiki.write_page(name, :markdown, content, commit_details(name))
   end
 
   def commit_details(name)
     Gitlab::Git::Wiki::CommitDetails.new(user.id, user.username, user.name, user.email, "created page #{name}")
   end
+
+  def destroy_page(title, dir = '')
+    page = wiki.page(title: title, dir: dir)
+    project_wiki.delete_page(page, "test commit")
+  end
 end

spec/features/projects/wiki/user_views_wiki_page_spec.rb

@@ -137,6 +137,26 @@ describe 'User views a wiki page' do
       end
     end
 
+    context 'when page has invalid content encoding' do
+      let(:content) { 'whatever'.force_encoding('ISO-8859-1') }
+
+      before do
+        allow(Gitlab::EncodingHelper).to receive(:encode!).and_return(content)
+
+        visit(project_wiki_path(project, wiki_page))
+      end
+
+      it 'does not show "Edit" button' do
+        expect(page).not_to have_selector('a.btn', text: 'Edit')
+      end
+
+      it 'shows error' do
+        page.within(:css, '.flash-notice') do
+          expect(page).to have_content('The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository.')
+        end
+      end
+    end
+
     it 'opens a default wiki page', :js do
       visit(project_path(project))