diff options
author | Stan Hu <stanhu@gmail.com> | 2019-05-09 15:53:25 +0300 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2019-05-10 02:04:40 +0300 |
commit | e67481e079023e1319e91fabfb90b88c9a2b2655 (patch) | |
tree | 1b8536d97905bec416e18336e5dc439bc589919d | |
parent | 712dccd4c0f37b9fce12f5a945a551f8dca07cc4 (diff) |
Properly handle LFS Batch API response in project import
Project imports were failing with `undefined method each_with_object for
String` because the import was attempting to parse the LFS Batch API and
failing due to the fact that the Content-Type wasn't a supported format
(e.g. application/vnd.git-lfs+json instead of application/json). We now
parse the body as JSON.
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/61624
3 files changed, 35 insertions, 2 deletions
diff --git a/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb b/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb index 05974948505..9b72480d18b 100644 --- a/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb +++ b/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) diff --git a/changelogs/unreleased/sh-fix-lfs-download-errors.yml b/changelogs/unreleased/sh-fix-lfs-download-errors.yml new file mode 100644 index 00000000000..ad67df6bb06 --- /dev/null +++ b/changelogs/unreleased/sh-fix-lfs-download-errors.yml @@ -0,0 +1,5 @@ +--- +title: Properly handle LFS Batch API response in project import +merge_request: 28223 +author: +type: fixed diff --git a/spec/services/projects/lfs_pointers/lfs_download_link_list_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_download_link_list_service_spec.rb index d8427d0bf78..80debcd3a7a 100644 --- a/spec/services/projects/lfs_pointers/lfs_download_link_list_service_spec.rb +++ b/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 |