Merge branch 'sh-fix-lfs-download-errors' into 'master'

Properly handle LFS Batch API response in project import

Closes #61624

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

app/services/projects/lfs_pointers/lfs_download_link_list_service.rb

@@ -37,7 +37,17 @@ module Projects
 
         raise DownloadLinksError, response.message unless response.success?
 
-        parse_response_links(response['objects'])
+        # Since the LFS Batch API may return a Content-Ttpe of
+        # application/vnd.git-lfs+json
+        # (https://github.com/git-lfs/git-lfs/blob/master/docs/api/batch.md#requests),
+        # HTTParty does not know this is actually JSON.
+        data = JSON.parse(response.body)
+
+        raise DownloadLinksError, "LFS Batch API did return any objects" unless data.is_a?(Hash) && data.key?('objects')
+
+        parse_response_links(data['objects'])
+      rescue JSON::ParserError
+        raise DownloadLinksError, "LFS Batch API response is not JSON"
       end
 
       def parse_response_links(objects_response)

changelogs/unreleased/sh-fix-lfs-download-errors.yml

+---
+title: Properly handle LFS Batch API response in project import
+merge_request: 28223
+author:
+type: fixed

spec/services/projects/lfs_pointers/lfs_download_link_list_service_spec.rb

@@ -33,7 +33,10 @@ describe Projects::LfsPointers::LfsDownloadLinkListService do
 
   before do
     allow(project).to receive(:lfs_enabled?).and_return(true)
-    allow(Gitlab::HTTP).to receive(:post).and_return(objects_response)
+    response = instance_double(HTTParty::Response)
+    allow(response).to receive(:body).and_return(objects_response.to_json)
+    allow(response).to receive(:success?).and_return(true)
+    allow(Gitlab::HTTP).to receive(:post).and_return(response)
   end
 
   describe '#execute' do
@@ -89,6 +92,21 @@ describe Projects::LfsPointers::LfsDownloadLinkListService do
 
       expect { subject.send(:get_download_links, new_oids) }.to raise_error(described_class::DownloadLinksError)
     end
+
+    shared_examples 'JSON parse errors' do |body|
+      it 'raises error' do
+        response = instance_double(HTTParty::Response)
+        allow(response).to receive(:body).and_return(body)
+        allow(response).to receive(:success?).and_return(true)
+        allow(Gitlab::HTTP).to receive(:post).and_return(response)
+
+        expect { subject.send(:get_download_links, new_oids) }.to raise_error(described_class::DownloadLinksError)
+      end
+    end
+
+    it_behaves_like 'JSON parse errors', '{'
+    it_behaves_like 'JSON parse errors', '{}'
+    it_behaves_like 'JSON parse errors', '{ foo: 123 }'
   end
 
   describe '#parse_response_links' do