diff options
| -rw-r--r-- | changelogs/unreleased/sha-handling.yml | 5 | ||||
| -rw-r--r-- | doc/api/repositories.md | 2 | ||||
| -rw-r--r-- | lib/api/api.rb | 7 | ||||
| -rw-r--r-- | lib/api/commits.rb | 12 | ||||
| -rw-r--r-- | lib/api/repositories.rb | 4 | ||||
| -rw-r--r-- | lib/api/v3/builds.rb | 2 | ||||
| -rw-r--r-- | lib/api/v3/commits.rb | 12 | ||||
| -rw-r--r-- | lib/api/v3/repositories.rb | 6 | ||||
| -rw-r--r-- | spec/requests/api/v3/repositories_spec.rb | 11 | 
9 files changed, 33 insertions, 28 deletions
diff --git a/changelogs/unreleased/sha-handling.yml b/changelogs/unreleased/sha-handling.yml new file mode 100644 index 00000000000..d776edafef5 --- /dev/null +++ b/changelogs/unreleased/sha-handling.yml @@ -0,0 +1,5 @@ +--- +title: Fix 404 errors in API caused when the branch name had a dot +merge_request: 14462 +author: gvieira37 +type: fixed diff --git a/doc/api/repositories.md b/doc/api/repositories.md index bccef924375..594babc74be 100644 --- a/doc/api/repositories.md +++ b/doc/api/repositories.md @@ -85,7 +85,7 @@ GET /projects/:id/repository/blobs/:sha  Parameters:  - `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user -- `sha` (required) - The commit or branch name +- `sha` (required) - The blob SHA  ## Raw blob content diff --git a/lib/api/api.rb b/lib/api/api.rb index 79e55a2f4f7..99fcc59ba04 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -4,6 +4,10 @@ module API      LOG_FILENAME = Rails.root.join("log", "api_json.log") +    NO_SLASH_URL_PART_REGEX = %r{[^/]+} +    PROJECT_ENDPOINT_REQUIREMENTS = { id: NO_SLASH_URL_PART_REGEX }.freeze +    COMMIT_ENDPOINT_REQUIREMENTS = PROJECT_ENDPOINT_REQUIREMENTS.merge(sha: NO_SLASH_URL_PART_REGEX).freeze +      use GrapeLogging::Middleware::RequestLogger,          logger: Logger.new(LOG_FILENAME),          formatter: Gitlab::GrapeLogging::Formatters::LogrageWithTimestamp.new, @@ -96,9 +100,6 @@ module API      helpers ::API::Helpers      helpers ::API::Helpers::CommonHelpers -    NO_SLASH_URL_PART_REGEX = %r{[^/]+} -    PROJECT_ENDPOINT_REQUIREMENTS = { id: NO_SLASH_URL_PART_REGEX }.freeze -      # Keep in alphabetical order      mount ::API::AccessRequests      mount ::API::AwardEmoji diff --git a/lib/api/commits.rb b/lib/api/commits.rb index 4b8d248f5f7..67a5264adc8 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -4,8 +4,6 @@ module API    class Commits < Grape::API      include PaginationParams -    COMMIT_ENDPOINT_REQUIREMENTS = API::PROJECT_ENDPOINT_REQUIREMENTS.merge(sha: API::NO_SLASH_URL_PART_REGEX) -      before { authorize! :download_code, user_project }      params do @@ -85,7 +83,7 @@ module API        params do          requires :sha, type: String, desc: 'A commit sha, or the name of a branch or tag'        end -      get ':id/repository/commits/:sha', requirements: COMMIT_ENDPOINT_REQUIREMENTS do +      get ':id/repository/commits/:sha', requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do          commit = user_project.commit(params[:sha])          not_found! 'Commit' unless commit @@ -99,7 +97,7 @@ module API        params do          requires :sha, type: String, desc: 'A commit sha, or the name of a branch or tag'        end -      get ':id/repository/commits/:sha/diff', requirements: COMMIT_ENDPOINT_REQUIREMENTS do +      get ':id/repository/commits/:sha/diff', requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do          commit = user_project.commit(params[:sha])          not_found! 'Commit' unless commit @@ -115,7 +113,7 @@ module API          use :pagination          requires :sha, type: String, desc: 'A commit sha, or the name of a branch or tag'        end -      get ':id/repository/commits/:sha/comments', requirements: COMMIT_ENDPOINT_REQUIREMENTS do +      get ':id/repository/commits/:sha/comments', requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do          commit = user_project.commit(params[:sha])          not_found! 'Commit' unless commit @@ -132,7 +130,7 @@ module API          requires :sha, type: String, desc: 'A commit sha, or the name of a branch or tag to be cherry picked'          requires :branch, type: String, desc: 'The name of the branch'        end -      post ':id/repository/commits/:sha/cherry_pick', requirements: COMMIT_ENDPOINT_REQUIREMENTS do +      post ':id/repository/commits/:sha/cherry_pick', requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do          authorize! :push_code, user_project          commit = user_project.commit(params[:sha]) @@ -169,7 +167,7 @@ module API            requires :line_type, type: String, values: %w(new old), default: 'new', desc: 'The type of the line'          end        end -      post ':id/repository/commits/:sha/comments', requirements: COMMIT_ENDPOINT_REQUIREMENTS do +      post ':id/repository/commits/:sha/comments', requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do          commit = user_project.commit(params[:sha])          not_found! 'Commit' unless commit diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index 2255fb1b70d..70f7bf32a71 100644 --- a/lib/api/repositories.rb +++ b/lib/api/repositories.rb @@ -57,7 +57,7 @@ module API        desc 'Get raw blob contents from the repository'        params do -        requires :sha, type: String, desc: 'The commit, branch name, or tag name' +        requires :sha, type: String, desc: 'The commit hash'        end        get ':id/repository/blobs/:sha/raw' do          assign_blob_vars! @@ -67,7 +67,7 @@ module API        desc 'Get a blob from the repository'        params do -        requires :sha, type: String, desc: 'The commit, branch name, or tag name' +        requires :sha, type: String, desc: 'The commit hash'        end        get ':id/repository/blobs/:sha' do          assign_blob_vars! diff --git a/lib/api/v3/builds.rb b/lib/api/v3/builds.rb index c189d486f50..f493fd7c7ec 100644 --- a/lib/api/v3/builds.rb +++ b/lib/api/v3/builds.rb @@ -8,7 +8,7 @@ module API        params do          requires :id, type: String, desc: 'The ID of a project'        end -      resource :projects do +      resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do          helpers do            params :optional_scope do              optional :scope, types: [String, Array[String]], desc: 'The scope of builds to show', diff --git a/lib/api/v3/commits.rb b/lib/api/v3/commits.rb index 5936f4700aa..759bb998c14 100644 --- a/lib/api/v3/commits.rb +++ b/lib/api/v3/commits.rb @@ -11,7 +11,7 @@ module API        params do          requires :id, type: String, desc: 'The ID of a project'        end -      resource :projects, requirements: { id: %r{[^/]+} } do +      resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do          desc 'Get a project repository commits' do            success ::API::Entities::RepoCommit          end @@ -72,7 +72,7 @@ module API          params do            requires :sha, type: String, desc: 'A commit sha, or the name of a branch or tag'          end -        get ":id/repository/commits/:sha" do +        get ":id/repository/commits/:sha", requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do            commit = user_project.commit(params[:sha])            not_found! "Commit" unless commit @@ -86,7 +86,7 @@ module API          params do            requires :sha, type: String, desc: 'A commit sha, or the name of a branch or tag'          end -        get ":id/repository/commits/:sha/diff" do +        get ":id/repository/commits/:sha/diff", requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do            commit = user_project.commit(params[:sha])            not_found! "Commit" unless commit @@ -102,7 +102,7 @@ module API            use :pagination            requires :sha, type: String, desc: 'A commit sha, or the name of a branch or tag'          end -        get ':id/repository/commits/:sha/comments' do +        get ':id/repository/commits/:sha/comments', requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do            commit = user_project.commit(params[:sha])            not_found! 'Commit' unless commit @@ -119,7 +119,7 @@ module API            requires :sha, type: String, desc: 'A commit sha to be cherry picked'            requires :branch, type: String, desc: 'The name of the branch'          end -        post ':id/repository/commits/:sha/cherry_pick' do +        post ':id/repository/commits/:sha/cherry_pick', requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do            authorize! :push_code, user_project            commit = user_project.commit(params[:sha]) @@ -156,7 +156,7 @@ module API              requires :line_type, type: String, values: %w(new old), default: 'new', desc: 'The type of the line'            end          end -        post ':id/repository/commits/:sha/comments' do +        post ':id/repository/commits/:sha/comments', requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do            commit = user_project.commit(params[:sha])            not_found! 'Commit' unless commit diff --git a/lib/api/v3/repositories.rb b/lib/api/v3/repositories.rb index 0eaa0de2eef..b9bc9a99093 100644 --- a/lib/api/v3/repositories.rb +++ b/lib/api/v3/repositories.rb @@ -8,7 +8,7 @@ module API        params do          requires :id, type: String, desc: 'The ID of a project'        end -      resource :projects, requirements: { id: %r{[^/]+} } do +      resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do          helpers do            def handle_project_member_errors(errors)              if errors[:project_access].any? @@ -43,7 +43,7 @@ module API            requires :sha, type: String, desc: 'The commit, branch name, or tag name'            requires :filepath, type: String, desc: 'The path to the file to display'          end -        get [":id/repository/blobs/:sha", ":id/repository/commits/:sha/blob"] do +        get [":id/repository/blobs/:sha", ":id/repository/commits/:sha/blob"], requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do            repo = user_project.repository            commit = repo.commit(params[:sha])            not_found! "Commit" unless commit @@ -56,7 +56,7 @@ module API          params do            requires :sha, type: String, desc: 'The commit, branch name, or tag name'          end -        get ':id/repository/raw_blobs/:sha' do +        get ':id/repository/raw_blobs/:sha', requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do            repo = user_project.repository            begin              blob = Gitlab::Git::Blob.raw(repo, params[:sha]) diff --git a/spec/requests/api/v3/repositories_spec.rb b/spec/requests/api/v3/repositories_spec.rb index 1a55e2a71cd..67624a0bbea 100644 --- a/spec/requests/api/v3/repositories_spec.rb +++ b/spec/requests/api/v3/repositories_spec.rb @@ -97,10 +97,11 @@ describe API::V3::Repositories do      end    end -  { -    'blobs/:sha' => 'blobs/master', -    'commits/:sha/blob' => 'commits/master/blob' -  }.each do |desc_path, example_path| +  [ +    ['blobs/:sha', 'blobs/master'], +    ['blobs/:sha', 'blobs/v1.1.0'], +    ['commits/:sha/blob', 'commits/master/blob'] +  ].each do |desc_path, example_path|      describe "GET /projects/:id/repository/#{desc_path}" do        let(:route) { "/projects/#{project.id}/repository/#{example_path}?filepath=README.md" }        shared_examples_for 'repository blob' do @@ -110,7 +111,7 @@ describe API::V3::Repositories do          end          context 'when sha does not exist' do            it_behaves_like '404 response' do -            let(:request) { get v3_api(route.sub('master', 'invalid_branch_name'), current_user) } +            let(:request) { get v3_api("/projects/#{project.id}/repository/#{desc_path.sub(':sha', 'invalid_branch_name')}?filepath=README.md", current_user) }              let(:message) { '404 Commit Not Found' }            end          end  | 
