From e9880b01d5d4f64784bd737cb00d5b4e816ed469 Mon Sep 17 00:00:00 2001 From: Paul Gower Date: Fri, 24 Apr 2026 20:09:12 -0500 Subject: [PATCH] Fix artifact download for non-binary bodies (#621) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit job_artifacts_download, download_job_artifact_file, and download_branch_artifact_file raise Gitlab::Error::Parsing on any artifact whose response body is not tagged ASCII_8BIT — YAML, plain text, CSV, JUnit XML, etc. The else-branch of each parser proc routes non-binary bodies through Gitlab::Request.parse, which calls JSON.load and re-raises non-JSON input as Gitlab::Error::Parsing. The non-binary path is reached for two cases: error responses with a JSON envelope (where Gitlab::Error::ResponseError#initialize relies on parsed_response to build the error message — Request.parse is correct here) and successful text artifacts (where it isn't). Keep the JSON parse and fall back to FileResponse on Gitlab::Error::Parsing. Add spec coverage for the text-response path on each method. --- lib/gitlab/client/jobs.rb | 24 +++++++++--- spec/gitlab/client/jobs_spec.rb | 69 +++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 6 deletions(-) diff --git a/lib/gitlab/client/jobs.rb b/lib/gitlab/client/jobs.rb index a961cc8a2..fbcf95e34 100644 --- a/lib/gitlab/client/jobs.rb +++ b/lib/gitlab/client/jobs.rb @@ -97,8 +97,12 @@ def job_artifacts_download(project_id, ref_name, job_name) parser: proc { |body, _| if body.encoding == Encoding::ASCII_8BIT # binary response ::Gitlab::FileResponse.new StringIO.new(body, 'rb+') - else # error with json response - ::Gitlab::Request.parse(body) + else + begin + ::Gitlab::Request.parse(body) # JSON error envelope + rescue ::Gitlab::Error::Parsing + ::Gitlab::FileResponse.new StringIO.new(body) # text artifact + end end }) end @@ -119,8 +123,12 @@ def download_job_artifact_file(project_id, job_id, artifact_path) parser: proc { |body, _| if body.encoding == Encoding::ASCII_8BIT # binary response ::Gitlab::FileResponse.new StringIO.new(body, 'rb+') - else # error with json response - ::Gitlab::Request.parse(body) + else + begin + ::Gitlab::Request.parse(body) # JSON error envelope + rescue ::Gitlab::Error::Parsing + ::Gitlab::FileResponse.new StringIO.new(body) # text artifact + end end }) end @@ -143,8 +151,12 @@ def download_branch_artifact_file(project_id, ref_name, artifact_path, job) parser: proc { |body, _| if body.encoding == Encoding::ASCII_8BIT # binary response ::Gitlab::FileResponse.new StringIO.new(body, 'rb+') - else # error with json response - ::Gitlab::Request.parse(body) + else + begin + ::Gitlab::Request.parse(body) # JSON error envelope + rescue ::Gitlab::Error::Parsing + ::Gitlab::FileResponse.new StringIO.new(body) # text artifact + end end }) end diff --git a/spec/gitlab/client/jobs_spec.rb b/spec/gitlab/client/jobs_spec.rb index 914d74b1b..9c44779fa 100644 --- a/spec/gitlab/client/jobs_spec.rb +++ b/spec/gitlab/client/jobs_spec.rb @@ -116,6 +116,29 @@ end end + context 'when successful text response' do + before do + fixture = load_fixture('raw_file.txt') + @body = fixture.read + stub_request(:get, "#{Gitlab.endpoint}/projects/3/jobs/artifacts/master/download") + .with(query: { job: 'test' }, headers: { 'PRIVATE-TOKEN' => Gitlab.private_token }) + .to_return(body: @body, headers: { 'Content-Disposition' => 'attachment; filename=raw_file.txt' }) + @job_artifacts = Gitlab.job_artifacts_download(3, 'master', 'test') + end + + it 'returns a FileResponse' do + expect(@job_artifacts).to be_a Gitlab::FileResponse + end + + it 'returns a file with filename' do + expect(@job_artifacts.filename).to eq 'raw_file.txt' + end + + it 'returns the body content' do + expect(@job_artifacts.read).to eq @body + end + end + context 'when bad request' do it 'throws an exception' do stub_get('/projects/3/jobs/artifacts/master/download', 'error_project_not_found', 404) @@ -149,6 +172,29 @@ end end + context 'when successful text response' do + before do + fixture = load_fixture('raw_file.txt') + @body = fixture.read + stub_request(:get, "#{Gitlab.endpoint}/projects/3/jobs/5/artifacts/raw_file.txt") + .with(headers: { 'PRIVATE-TOKEN' => Gitlab.private_token }) + .to_return(body: @body, headers: { 'Content-Disposition' => 'attachment; filename=raw_file.txt' }) + @job_artifact_file = Gitlab.download_job_artifact_file(3, 5, 'raw_file.txt') + end + + it 'returns a FileResponse' do + expect(@job_artifact_file).to be_a Gitlab::FileResponse + end + + it 'returns a file with filename' do + expect(@job_artifact_file.filename).to eq 'raw_file.txt' + end + + it 'returns the body content' do + expect(@job_artifact_file.read).to eq @body + end + end + context 'when bad request' do it 'throws an exception' do stub_get('/projects/3/jobs/5/artifacts/raw_file.txt', 'error_project_not_found', 404) @@ -182,6 +228,29 @@ end end + context 'when successful text response' do + before do + fixture = load_fixture('raw_file.txt') + @body = fixture.read + stub_request(:get, "#{Gitlab.endpoint}/projects/1/jobs/artifacts/master/raw/raw_file.txt") + .with(query: { job: 'txt' }, headers: { 'PRIVATE-TOKEN' => Gitlab.private_token }) + .to_return(body: @body, headers: { 'Content-Disposition' => 'attachment; filename=raw_file.txt' }) + @branch_artifact_file = Gitlab.download_branch_artifact_file(1, 'master', 'raw_file.txt', 'txt') + end + + it 'returns a FileResponse' do + expect(@branch_artifact_file).to be_a Gitlab::FileResponse + end + + it 'returns a file with filename' do + expect(@branch_artifact_file.filename).to eq 'raw_file.txt' + end + + it 'returns the body content' do + expect(@branch_artifact_file.read).to eq @body + end + end + context 'when bad request' do it 'throws an exception' do stub_get('/projects/1/jobs/artifacts/master/raw/raw_file.txt', 'error_project_not_found', 404)