From 87cca0fb8009178ed81f77ff499e3adb12b40d5b Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Thu, 11 Apr 2019 13:56:43 +0800 Subject: [PATCH] FIX: Return the right response code for invalid theme id. --- app/controllers/admin/themes_controller.rb | 20 +++++++++++++------ spec/requests/admin/themes_controller_spec.rb | 20 +++++++++++++++++++ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/app/controllers/admin/themes_controller.rb b/app/controllers/admin/themes_controller.rb index b2d73a399b6..84ad22f67ac 100644 --- a/app/controllers/admin/themes_controller.rb +++ b/app/controllers/admin/themes_controller.rb @@ -7,8 +7,10 @@ class Admin::ThemesController < Admin::AdminController skip_before_action :check_xhr, only: [:show, :preview, :export] def preview - @theme = Theme.find(params[:id]) - redirect_to path("/?preview_theme_id=#{@theme.id}") + theme = Theme.find_by(id: params[:id]) + raise Discourse::InvalidParameters.new(:id) unless theme + + redirect_to path("/?preview_theme_id=#{theme.id}") end def upload_asset @@ -148,7 +150,8 @@ class Admin::ThemesController < Admin::AdminController end def update - @theme = Theme.find(params[:id]) + @theme = Theme.find_by(id: params[:id]) + raise Discourse::InvalidParameters.new(:id) unless @theme original_json = ThemeSerializer.new(@theme, root: false).to_json @@ -215,7 +218,9 @@ class Admin::ThemesController < Admin::AdminController end def destroy - @theme = Theme.find(params[:id]) + @theme = Theme.find_by(id: params[:id]) + raise Discourse::InvalidParameters.new(:id) unless @theme + StaffActionLogger.new(current_user).log_theme_destroy(@theme) @theme.destroy @@ -225,12 +230,15 @@ class Admin::ThemesController < Admin::AdminController end def show - @theme = Theme.find(params[:id]) + @theme = Theme.find_by(id: params[:id]) + raise Discourse::InvalidParameters.new(:id) unless @theme + render json: ThemeSerializer.new(@theme) end def export - @theme = Theme.find(params[:id]) + @theme = Theme.find_by(id: params[:id]) + raise Discourse::InvalidParameters.new(:id) unless @theme exporter = ThemeStore::TgzExporter.new(@theme) file_path = exporter.package_filename diff --git a/spec/requests/admin/themes_controller_spec.rb b/spec/requests/admin/themes_controller_spec.rb index 539eddbb822..0de5b0f9c95 100644 --- a/spec/requests/admin/themes_controller_spec.rb +++ b/spec/requests/admin/themes_controller_spec.rb @@ -218,6 +218,12 @@ describe Admin::ThemesController do describe '#update' do let(:theme) { Fabricate(:theme) } + it 'returns the right response when an invalid id is given' do + put "/admin/themes/99999.json" + + expect(response.status).to eq(400) + end + it 'can change default theme' do SiteSetting.default_theme_id = -1 @@ -342,6 +348,12 @@ describe Admin::ThemesController do describe '#destroy' do let(:theme) { Fabricate(:theme) } + it 'returns the right response when an invalid id is given' do + delete "/admin/themes/9999.json" + + expect(response.status).to eq(400) + end + it "deletes the field's javascript cache" do theme.set_field(target: :common, name: :header, value: '') theme.save! @@ -356,4 +368,12 @@ describe Admin::ThemesController do expect { javascript_cache.reload }.to raise_error(ActiveRecord::RecordNotFound) end end + + describe '#preview' do + it "should return the right response when an invalid id is given" do + get "/admin/themes/9999/preview.json" + + expect(response.status).to eq(400) + end + end end