From e65bc0f175c54d9df66fd4950972c0b0b08d448e Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 24 Aug 2016 16:02:56 +0800 Subject: [PATCH] Path could also have slashes! Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5142#note_14347729 --- .../projects/artifacts_controller.rb | 14 +++++++++--- app/helpers/gitlab_routing_helper.rb | 5 +++-- config/routes.rb | 5 ++--- .../projects/artifacts_controller_spec.rb | 22 +++++++++++++++++-- 4 files changed, 36 insertions(+), 10 deletions(-) diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb index 60e432d68d8..17c6d56c8b9 100644 --- a/app/controllers/projects/artifacts_controller.rb +++ b/app/controllers/projects/artifacts_controller.rb @@ -1,4 +1,6 @@ class Projects::ArtifactsController < Projects::ApplicationController + include ExtractsPath + layout 'project' before_action :authorize_read_build! before_action :authorize_update_build!, only: [:keep] @@ -35,7 +37,8 @@ class Projects::ArtifactsController < Projects::ApplicationController end def latest_succeeded - target_path = artifacts_action_path(params[:path], project, build) + path = ref_name_and_path.last + target_path = artifacts_action_path(path, project, build) if target_path redirect_to(target_path) @@ -59,13 +62,18 @@ class Projects::ArtifactsController < Projects::ApplicationController end def build_from_ref - if params[:ref_name] - builds = project.latest_successful_builds_for(params[:ref_name]) + if params[:ref_name_and_path] + ref_name = ref_name_and_path.first + builds = project.latest_successful_builds_for(ref_name) builds.find_by(name: params[:job]) end end + def ref_name_and_path + @ref_name_and_path ||= extract_ref(params[:ref_name_and_path]) + end + def artifacts_file @artifacts_file ||= build.artifacts_file end diff --git a/app/helpers/gitlab_routing_helper.rb b/app/helpers/gitlab_routing_helper.rb index cd526f17b99..a322a90cc4e 100644 --- a/app/helpers/gitlab_routing_helper.rb +++ b/app/helpers/gitlab_routing_helper.rb @@ -153,9 +153,10 @@ module GitlabRoutingHelper # Artifacts def artifacts_action_path(path, project, build) - args = [project.namespace, project, build] + action, path_params = path.split('/', 2) + args = [project.namespace, project, build, path_params] - case path + case action when 'download' download_namespace_project_build_artifacts_path(*args) when 'browse' diff --git a/config/routes.rb b/config/routes.rb index 606181ff837..879cd61a02f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -787,9 +787,8 @@ Rails.application.routes.draw do resources :artifacts, only: [] do collection do get :latest_succeeded, - path: ':ref_name/*path', - format: false, - constraints: { ref_name: /.+/ } # could have / + path: '*ref_name_and_path', + format: false end end end diff --git a/spec/requests/projects/artifacts_controller_spec.rb b/spec/requests/projects/artifacts_controller_spec.rb index 3ba6725efc3..e02f0eacc93 100644 --- a/spec/requests/projects/artifacts_controller_spec.rb +++ b/spec/requests/projects/artifacts_controller_spec.rb @@ -26,8 +26,7 @@ describe Projects::ArtifactsController do latest_succeeded_namespace_project_artifacts_path( project.namespace, project, - ref, - path, + [ref, path].join('/'), job: job) end @@ -94,6 +93,25 @@ describe Projects::ArtifactsController do it_behaves_like 'redirect to the build' end + + context 'with branch name and path containing slashes' do + before do + pipeline.update(ref: 'improve/awesome', + sha: project.commit('improve/awesome').sha) + + get path_from_ref('improve/awesome', build.name, 'file/README.md') + end + + it 'redirects' do + path = file_namespace_project_build_artifacts_path( + project.namespace, + project, + build, + 'README.md') + + expect(response).to redirect_to(path) + end + end end end end