diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb index b7c656246ef..da7aeb26a75 100644 --- a/app/controllers/projects/wikis_controller.rb +++ b/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 diff --git a/app/views/projects/wikis/_main_links.html.haml b/app/views/projects/wikis/_main_links.html.haml index cadda0a33c2..8d91f411f89 100644 --- a/app/views/projects/wikis/_main_links.html.haml +++ b/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") diff --git a/changelogs/unreleased/fj-49512-fix-gitlab-git-pages-encoding.yml b/changelogs/unreleased/fj-49512-fix-gitlab-git-pages-encoding.yml new file mode 100644 index 00000000000..3af90fff3f6 --- /dev/null +++ b/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 diff --git a/spec/controllers/projects/wikis_controller_spec.rb b/spec/controllers/projects/wikis_controller_spec.rb index fed6677935e..30623b6cb3d 100644 --- a/spec/controllers/projects/wikis_controller_spec.rb +++ b/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(:wiki) { ProjectWiki.new(project, user) } + let(:user) { project.owner } + 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 - create_page(wiki_title, 'hello world') + 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 end - it 'limits the retrieved pages for the sidebar' do - sign_in(user) + context 'when page content encoding is invalid' do + 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(wiki).to receive(:pages).with(limit: 1).and_call_original - # Sidebar entries - 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) + expect(response).to have_http_status(:ok) + 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 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 diff --git a/spec/features/projects/wiki/user_views_wiki_page_spec.rb b/spec/features/projects/wiki/user_views_wiki_page_spec.rb index 0ef7f35f64a..760324adacc 100644 --- a/spec/features/projects/wiki/user_views_wiki_page_spec.rb +++ b/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))