diff --git a/google-apis-core/google-apis-core.gemspec b/google-apis-core/google-apis-core.gemspec index 828f506410c..15648a8d71c 100644 --- a/google-apis-core/google-apis-core.gemspec +++ b/google-apis-core/google-apis-core.gemspec @@ -20,7 +20,7 @@ Gem::Specification.new do |gem| gem.require_paths = ['lib'] gem.required_ruby_version = '>= 3.2' - gem.add_runtime_dependency 'addressable', '~> 2.8', '>= 2.8.7' + gem.add_runtime_dependency 'addressable', '~> 2.9' gem.add_runtime_dependency 'faraday', '~> 2.13' gem.add_runtime_dependency 'faraday-follow_redirects', '~> 0.3' gem.add_runtime_dependency 'googleauth', '~> 1.14' diff --git a/google-apis-core/lib/google/apis/core/download.rb b/google-apis-core/lib/google/apis/core/download.rb index 282ea15c905..1d2b77bd998 100644 --- a/google-apis-core/lib/google/apis/core/download.rb +++ b/google-apis-core/lib/google/apis/core/download.rb @@ -81,11 +81,12 @@ def execute_once(client, &block) http_res = client.get(url.to_s, query, request_header) do |request| request.options.on_data = proc do |chunk, _size, res| - # The on_data callback is only invoked on a successful response. # Some Faraday adapters (e.g. Typhoeus) may not provide a response # object in the callback, so we default to a 200 OK status. + # Note: Ignore non-2xx responses (redirects and 4xx/5xx error payloads) + # so error messages are not written to the download stream. status = res ? res.status.to_i : 200 - next if chunk.nil? || (status >= 300 && status < 400) + next if chunk.nil? || !(200..299).include?(status) # HTTP 206 is Partial Content download_offset ||= (status == 206 ? @offset : 0) diff --git a/google-apis-core/lib/google/apis/core/storage_download.rb b/google-apis-core/lib/google/apis/core/storage_download.rb index 476999c4073..709c6c68940 100644 --- a/google-apis-core/lib/google/apis/core/storage_download.rb +++ b/google-apis-core/lib/google/apis/core/storage_download.rb @@ -48,11 +48,12 @@ def execute_once(client, &block) http_res = client.get(url.to_s, query, request_header) do |request| request.options.on_data = proc do |chunk, _size, res| - # The on_data callback is only invoked on a successful response. # Some Faraday adapters (e.g. Typhoeus) may not provide a response # object in the callback, so we default to a 200 OK status. + # Note: Ignore non-2xx responses (redirects and 4xx/5xx error payloads) + # so error messages are not written to the download stream. status = res ? res.status.to_i : 200 - next if chunk.nil? || (status >= 300 && status < 400) + next if chunk.nil? || !(200..299).include?(status) download_offset ||= (status == 206 ? @offset : 0) download_offset += chunk.bytesize diff --git a/google-apis-core/spec/google/apis/core/download_spec.rb b/google-apis-core/spec/google/apis/core/download_spec.rb index dc8b5044146..3bc9a170237 100644 --- a/google-apis-core/spec/google/apis/core/download_spec.rb +++ b/google-apis-core/spec/google/apis/core/download_spec.rb @@ -157,6 +157,24 @@ def write(data) end end + context 'when server responds with non-2xx status and error body' do + let(:dest) { StringIO.new } + + it 'does not write error body to destination IO' do + command.options.retries = 0 + response = Faraday::Response.new(status: 503, body: 'Service Unavailable') + expect(client).to receive(:get) do |_url, _params, _headers, &block| + request = Faraday::Request.new + request.options = Faraday::RequestOptions.new + block.call(request) + request.options.on_data.call('Service Unavailable', 19, response) + response + end + expect { command.execute(client) }.to raise_error(Google::Apis::ServerError) + expect(dest.string).to be_empty + end + end + context 'with pathname destination' do let(:dest) { Pathname.new(File.join(Dir.mktmpdir, 'test-path.txt')) } let(:received) do diff --git a/google-apis-core/spec/google/apis/core/storage_download_spec.rb b/google-apis-core/spec/google/apis/core/storage_download_spec.rb index c60f17da16d..a135aec37c9 100644 --- a/google-apis-core/spec/google/apis/core/storage_download_spec.rb +++ b/google-apis-core/spec/google/apis/core/storage_download_spec.rb @@ -162,4 +162,22 @@ def write(data) expect(received).to eql 'Hello world' end end + + context 'when server responds with non-2xx status and error body' do + let(:dest) { StringIO.new } + + it 'does not write error body to destination IO' do + command.options.retries = 0 + response = Faraday::Response.new(status: 503, body: 'Service Unavailable') + expect(client).to receive(:get) do |_url, _params, _headers, &block| + request = Faraday::Request.new + request.options = Faraday::RequestOptions.new + block.call(request) + request.options.on_data.call('Service Unavailable', 19, response) + response + end + expect { command.execute(client) }.to raise_error(Google::Apis::ServerError) + expect(dest.string).to be_empty + end + end end