Merge branch '51952-forking-via-webide' into 'master'

Resolve "500 error when forking via the web IDE button"

See merge request gitlab-org/gitlab-ce!29909

5 files changed, 13 insertions, 21 deletions

diff --git a/app/controllers/concerns/continue_params.rb b/app/controllers/concerns/continue_params.rb
index 54c0510497f..d5830f6648c 100644
--- a/app/controllers/concerns/continue_params.rb
+++ b/app/controllers/concerns/continue_params.rb
@@ -6,7 +6,7 @@ module ContinueParams
 
   def continue_params
     continue_params = params[:continue]
-    return unless continue_params
+    return {} unless continue_params
 
     continue_params = continue_params.permit(:to, :notice, :notice_now)
     continue_params[:to] = safe_redirect_path(continue_params[:to])
diff --git a/app/controllers/concerns/internal_redirect.rb b/app/controllers/concerns/internal_redirect.rb
index 6785e6972d0..fa3716502a0 100644
--- a/app/controllers/concerns/internal_redirect.rb
+++ b/app/controllers/concerns/internal_redirect.rb
@@ -5,8 +5,8 @@ module InternalRedirect
 
   def safe_redirect_path(path)
     return unless path
-    # Verify that the string starts with a `/` but not a double `/`.
-    return unless path =~ %r{^/\w.*$}
+    # Verify that the string starts with a `/` and a known route character.
+    return unless path =~ %r{^/[-\w].*$}
 
     uri = URI(path)
     # Ignore anything path of the redirect except for the path, querystring and,
diff --git a/app/controllers/projects/forks_controller.rb b/app/controllers/projects/forks_controller.rb
index 7a1700a206a..ac1c4bc7fd3 100644
--- a/app/controllers/projects/forks_controller.rb
+++ b/app/controllers/projects/forks_controller.rb
@@ -46,18 +46,14 @@ class Projects::ForksController < Projects::ApplicationController
 
     @forked_project ||= ::Projects::ForkService.new(project, current_user, namespace: namespace).execute
 
-    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
+    if !@forked_project.saved? || !@forked_project.forked?
       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
   # rubocop: enable CodeReuse/ActiveRecord
diff --git a/app/controllers/projects/imports_controller.rb b/app/controllers/projects/imports_controller.rb
index afbf9fd7720..da32ab9e2e0 100644
--- a/app/controllers/projects/imports_controller.rb
+++ b/app/controllers/projects/imports_controller.rb
@@ -23,7 +23,7 @@ class Projects::ImportsController < Projects::ApplicationController
 
   def show
     if @project.import_finished?
-      if continue_params&.key?(:to)
+      if continue_params[:to]
         redirect_to continue_params[:to], notice: continue_params[:notice]
       else
         redirect_to project_path(@project), notice: finished_notice
@@ -31,11 +31,7 @@ class Projects::ImportsController < Projects::ApplicationController
     elsif @project.import_failed?
       redirect_to new_project_import_path(@project)
     else
-      if continue_params && continue_params[:notice_now]
-        flash.now[:notice] = continue_params[:notice_now]
-      end
-
-      # Render
+      flash.now[:notice] = continue_params[:notice_now]
     end
   end
 
diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb
index d7c0039b234..02ff6e872c9 100644
--- a/app/controllers/projects/jobs_controller.rb
+++ b/app/controllers/projects/jobs_controller.rb
@@ -103,7 +103,7 @@ class Projects::JobsController < Projects::ApplicationController
 
     @build.cancel
 
-    if continue_params
+    if continue_params[:to]
       redirect_to continue_params[:to]
     else
       redirect_to builds_project_pipeline_path(@project, @build.pipeline.id)