Skip to content

Commit 20c20b2

Browse files
authored
fix(google-apis-core): ignore non-2xx status codes in streaming downloads (googleapis#27256)
* fix(google-apis-core): ignore non-2xx status codes in streaming downloads Prevents error response bodies (e.g. 503 Service Unavailable or 4xx error payloads) from being written into destination file streams during streaming downloads. Restores original pre-v1.0.0 allowlist filtering behavior (`!(200..299).include?(status)`) to prevent corrupted download offsets on retryable errors. Fixes googleapis/google-cloud-ruby#34750 * test(google-apis-core): disable retries in non-2xx streaming unit tests Explicitly sets retries = 0 in non-2xx streaming specs to prevent Retriable from looping.
1 parent f6b0af0 commit 20c20b2

4 files changed

Lines changed: 42 additions & 4 deletions

File tree

google-apis-core/lib/google/apis/core/download.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,12 @@ def execute_once(client, &block)
8181

8282
http_res = client.get(url.to_s, query, request_header) do |request|
8383
request.options.on_data = proc do |chunk, _size, res|
84-
# The on_data callback is only invoked on a successful response.
8584
# Some Faraday adapters (e.g. Typhoeus) may not provide a response
8685
# object in the callback, so we default to a 200 OK status.
86+
# Note: Ignore non-2xx responses (redirects and 4xx/5xx error payloads)
87+
# so error messages are not written to the download stream.
8788
status = res ? res.status.to_i : 200
88-
next if chunk.nil? || (status >= 300 && status < 400)
89+
next if chunk.nil? || !(200..299).include?(status)
8990

9091
# HTTP 206 is Partial Content
9192
download_offset ||= (status == 206 ? @offset : 0)

google-apis-core/lib/google/apis/core/storage_download.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,12 @@ def execute_once(client, &block)
4848

4949
http_res = client.get(url.to_s, query, request_header) do |request|
5050
request.options.on_data = proc do |chunk, _size, res|
51-
# The on_data callback is only invoked on a successful response.
5251
# Some Faraday adapters (e.g. Typhoeus) may not provide a response
5352
# object in the callback, so we default to a 200 OK status.
53+
# Note: Ignore non-2xx responses (redirects and 4xx/5xx error payloads)
54+
# so error messages are not written to the download stream.
5455
status = res ? res.status.to_i : 200
55-
next if chunk.nil? || (status >= 300 && status < 400)
56+
next if chunk.nil? || !(200..299).include?(status)
5657

5758
download_offset ||= (status == 206 ? @offset : 0)
5859
download_offset += chunk.bytesize

google-apis-core/spec/google/apis/core/download_spec.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,24 @@ def write(data)
157157
end
158158
end
159159

160+
context 'when server responds with non-2xx status and error body' do
161+
let(:dest) { StringIO.new }
162+
163+
it 'does not write error body to destination IO' do
164+
command.options.retries = 0
165+
response = Faraday::Response.new(status: 503, body: 'Service Unavailable')
166+
expect(client).to receive(:get) do |_url, _params, _headers, &block|
167+
request = Faraday::Request.new
168+
request.options = Faraday::RequestOptions.new
169+
block.call(request)
170+
request.options.on_data.call('Service Unavailable', 19, response)
171+
response
172+
end
173+
expect { command.execute(client) }.to raise_error(Google::Apis::ServerError)
174+
expect(dest.string).to be_empty
175+
end
176+
end
177+
160178
context 'with pathname destination' do
161179
let(:dest) { Pathname.new(File.join(Dir.mktmpdir, 'test-path.txt')) }
162180
let(:received) do

google-apis-core/spec/google/apis/core/storage_download_spec.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,4 +162,22 @@ def write(data)
162162
expect(received).to eql 'Hello world'
163163
end
164164
end
165+
166+
context 'when server responds with non-2xx status and error body' do
167+
let(:dest) { StringIO.new }
168+
169+
it 'does not write error body to destination IO' do
170+
command.options.retries = 0
171+
response = Faraday::Response.new(status: 503, body: 'Service Unavailable')
172+
expect(client).to receive(:get) do |_url, _params, _headers, &block|
173+
request = Faraday::Request.new
174+
request.options = Faraday::RequestOptions.new
175+
block.call(request)
176+
request.options.on_data.call('Service Unavailable', 19, response)
177+
response
178+
end
179+
expect { command.execute(client) }.to raise_error(Google::Apis::ServerError)
180+
expect(dest.string).to be_empty
181+
end
182+
end
165183
end

0 commit comments

Comments
 (0)