Merge branch 'sha-handling' into 'master'

Adds requirements that supports anything in sha params

Closes #26561 and #2709

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

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 62565e2d522..2685dc27252 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 ceee3226732..7887b886c03 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 bd5c6b34d12..ed206a6def0 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::Commit
         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 41a7c6b83ae..f9a47101e27 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