Commit 8fd2c084 authored by Markus Koller's avatar Markus Koller

Make checks for continue_params more robust

The check for continue_params&.key?(:to) in Projects::ImportsController
caused an exception in redirect_to if this key contained a nil value.

Since url_for won't add any params for an empty hash, we can just return
that in continue_params if params[:continue] isn't present, and simplify
the code in the controllers to check for the values we actually want to
use.
parent e8aff835
...@@ -6,7 +6,7 @@ module ContinueParams ...@@ -6,7 +6,7 @@ module ContinueParams
def continue_params def continue_params
continue_params = params[:continue] continue_params = params[:continue]
return unless continue_params return {} unless continue_params
continue_params = continue_params.permit(:to, :notice, :notice_now) continue_params = continue_params.permit(:to, :notice, :notice_now)
continue_params[:to] = safe_redirect_path(continue_params[:to]) continue_params[:to] = safe_redirect_path(continue_params[:to])
......
...@@ -46,18 +46,14 @@ class Projects::ForksController < Projects::ApplicationController ...@@ -46,18 +46,14 @@ class Projects::ForksController < Projects::ApplicationController
@forked_project ||= ::Projects::ForkService.new(project, current_user, namespace: namespace).execute @forked_project ||= ::Projects::ForkService.new(project, current_user, namespace: namespace).execute
if @forked_project.saved? && @forked_project.forked? if !@forked_project.saved? || !@forked_project.forked?
if @forked_project.import_in_progress?
redirect_to project_import_path(@forked_project, continue: continue_params)
else
if continue_params
redirect_to continue_params[:to], notice: continue_params[:notice]
else
redirect_to project_path(@forked_project), notice: "The project '#{@forked_project.name}' was successfully forked."
end
end
else
render :error render :error
elsif @forked_project.import_in_progress?
redirect_to project_import_path(@forked_project, continue: continue_params)
elsif continue_params[:to]
redirect_to continue_params[:to], notice: continue_params[:notice]
else
redirect_to project_path(@forked_project), notice: "The project '#{@forked_project.name}' was successfully forked."
end end
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
......
...@@ -23,7 +23,7 @@ class Projects::ImportsController < Projects::ApplicationController ...@@ -23,7 +23,7 @@ class Projects::ImportsController < Projects::ApplicationController
def show def show
if @project.import_finished? if @project.import_finished?
if continue_params&.key?(:to) if continue_params[:to]
redirect_to continue_params[:to], notice: continue_params[:notice] redirect_to continue_params[:to], notice: continue_params[:notice]
else else
redirect_to project_path(@project), notice: finished_notice redirect_to project_path(@project), notice: finished_notice
...@@ -31,11 +31,7 @@ class Projects::ImportsController < Projects::ApplicationController ...@@ -31,11 +31,7 @@ class Projects::ImportsController < Projects::ApplicationController
elsif @project.import_failed? elsif @project.import_failed?
redirect_to new_project_import_path(@project) redirect_to new_project_import_path(@project)
else else
if continue_params && continue_params[:notice_now] flash.now[:notice] = continue_params[:notice_now]
flash.now[:notice] = continue_params[:notice_now]
end
# Render
end end
end end
......
...@@ -103,7 +103,7 @@ class Projects::JobsController < Projects::ApplicationController ...@@ -103,7 +103,7 @@ class Projects::JobsController < Projects::ApplicationController
@build.cancel @build.cancel
if continue_params if continue_params[:to]
redirect_to continue_params[:to] redirect_to continue_params[:to]
else else
redirect_to builds_project_pipeline_path(@project, @build.pipeline.id) redirect_to builds_project_pipeline_path(@project, @build.pipeline.id)
......
...@@ -18,6 +18,14 @@ describe ContinueParams do ...@@ -18,6 +18,14 @@ describe ContinueParams do
ActionController::Parameters.new(continue: params) ActionController::Parameters.new(continue: params)
end end
it 'returns an empty hash if params are not present' do
allow(controller).to receive(:params) do
ActionController::Parameters.new
end
expect(controller.continue_params).to eq({})
end
it 'cleans up any params that are not allowed' do it 'cleans up any params that are not allowed' do
allow(controller).to receive(:params) do allow(controller).to receive(:params) do
strong_continue_params(to: '/hello', strong_continue_params(to: '/hello',
......
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