Commit e67481e0 authored by Stan Hu's avatar Stan Hu

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
parent 712dccd4
...@@ -37,7 +37,17 @@ module Projects ...@@ -37,7 +37,17 @@ module Projects
raise DownloadLinksError, response.message unless response.success? 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 end
def parse_response_links(objects_response) def parse_response_links(objects_response)
......
---
title: Properly handle LFS Batch API response in project import
merge_request: 28223
author:
type: fixed
...@@ -33,7 +33,10 @@ describe Projects::LfsPointers::LfsDownloadLinkListService do ...@@ -33,7 +33,10 @@ describe Projects::LfsPointers::LfsDownloadLinkListService do
before do before do
allow(project).to receive(:lfs_enabled?).and_return(true) 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 end
describe '#execute' do describe '#execute' do
...@@ -89,6 +92,21 @@ describe Projects::LfsPointers::LfsDownloadLinkListService do ...@@ -89,6 +92,21 @@ describe Projects::LfsPointers::LfsDownloadLinkListService do
expect { subject.send(:get_download_links, new_oids) }.to raise_error(described_class::DownloadLinksError) expect { subject.send(:get_download_links, new_oids) }.to raise_error(described_class::DownloadLinksError)
end 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 end
describe '#parse_response_links' do describe '#parse_response_links' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment