Fix/download non 2xx status

[torreypayne]

Historical Context & Root Cause

Prior to google-apis-core v1.0.0 (when httpclient was used as the backend), DownloadCommand safeguarded download streams by strictly checking HTTP status codes against an allowlist of successful download statuses:

# Before v1.0.0 (httpclient backend)
status = res.http_header.status_code.to_i
next unless OK_STATUS.include?(status) # where OK_STATUS = [200, 201, 206]

On July 31, 2025, in commit cdf6281c733 (PR #23524, migrating the underlying HTTP library from httpclient to Faraday in v1.0.0), OK_STATUS was deprecated (# @deprecated No longer used), and the condition was replaced with:

# After v1.0.0 (Faraday backend)
status = res.status.to_i
next if chunk.nil? || (status >= 300 && status < 400)

When using Faraday's redirect middleware, Faraday invokes on_data for intermediate 301/302 redirect responses before following the redirect to the final URL. The check status >= 300 && status < 400 was introduced to prevent redirect HTML pages from being written to @download_io.

However, in commit 69490b7b0f9 (Sep 9, 2025), an explanatory comment was added above this line:

# 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.

This comment reveals the assumption that led to this bug: maintainers assumed Faraday only invokes on_data for successful 2xx responses. In reality, standard Faraday adapters (such as Net::HTTP) stream response bodies for all HTTP status codes—including 4xx client errors and 5xx server error payloads—directly through on_data.

Because status >= 300 && status < 400 only filters out 3xx redirects, any streaming download that encounters an HTTP error (such as 503 Service Unavailable) writes the error response body directly into @download_io and advances @offset by the length of the error message payload.

Fixes googleapis/google-cloud-ruby#34750

---

Fix Implementation

In both DownloadCommand (lib/google/apis/core/download.rb) and StorageDownloadCommand (lib/google/apis/core/storage_download.rb), modify the on_data callback filter to skip any non-success status codes (!(200..299).include?(status)):

- next if chunk.nil? || (status >= 300 && status < 400)
+ next if chunk.nil? || !(200..299).include?(status)

This change directly implements the resolution proposed in issue #34750 and restores the original pre-v1.0.0 allowlist logic (OK_STATUS) that protected download streams from error payload corruption.

---

Downstream Impact & Execution Flow (Issue #34750)

In downstream clients like google-cloud-storage, writing error payloads and advancing @offset caused consecutive retryable server errors (503 Service Unavailable) to request out-of-bounds byte ranges, resulting in fatal 416 Range Not Satisfiable errors or CRC32C checksum verification failures.

Specifically, this caused consistent presubmit conformance test failures for storage.objects.get:

• ConformanceTest#test_1-return-503_return-503-storage.objects.get-0
• ConformanceTest#test_1-return-reset-connection_return-503-storage.objects.get-0

Step-by-Step Execution Flow of the Bug

When downloading an object (object.download / :download_file):

1. First Request: The client requests the object. The server returns 503 Service Unavailable with a response body describing the error (e.g., 84 bytes).
2. on_data Callback: Because 503 is not in 300...400, on_data writes the 84-byte error message into @download_io and increments @offset from 0 to 84.
3. Status Verification: After the request completes, check_status inspects 503 and raises Google::Apis::ServerError (retryable).
4. Second Request (Retry 1): Retriable catches the error and retries. Because @offset is now 84, the client assumes it is resuming a partially completed download and adds: Range: bytes=84-
5. Server Error: The server returns 503 again (84 bytes). on_data writes the second error body and increments @offset by another 84 bytes (168).
6. Third Request (Retry 2): The client retries again, sending: Range: bytes=168-
7. Fatal 416 Range Not Satisfiable: Because the test file is 12 bytes ("My test file"), requesting a range starting at 168 is out of bounds. The server responds with 416 Requested Range Not Satisfiable.
8. Failure: 416 maps to Google::Apis::ClientError (400 "Invalid request"), which is non-retryable—aborting execution and failing the test.

---

Risk Assessment

In HTTP/1.1 and HTTP/2 media downloads, there is zero valid use case where a client should write the response body of a non-2xx status code into a destination file stream (@download_io):

• 1xx (Informational): No message bodies exist.
• 2xx (Success - 200, 201, 203, 206): This is the only range where actual file content is transferred.
• 3xx (Redirection - 301, 302, 304, 307): The response body is either empty (304 Not Modified) or redirect HTML (<a href="...">Moved</a>).
• 4xx (Client Errors - 400, 403, 404, 416, 429): The response body is an error message payload (JSON, XML, or HTML).
• 5xx (Server Errors - 500, 502, 503, 504): The response body is a server error payload.

Restricting chunk writing strictly to 200..299 carries zero regression risk for upstream gems or services.