Fix corrupt parallel cache and artifact downloads when a server ignores Range

What does this MR do?

Makes the two chunk fetchers of the parallel cache and artifact downloads accept only a 206 Partial Content whose Content-Range is the range they asked for: presignedRangeFetchChunk in commands/helpers/cache_extractor.go and fetchChunk in tryArtifactParallelDownload in network/gitlab.go. A chunk answered with 200 OK, with a 206 for another range or with a 206 without Content-Range now fails the parallel download instead of being written at the chunk's offset. On the presigned cache path a chunk answered with 200 then makes the extractor fall back to a sequential GET, as the artifact download already falls back to a single download stream.

Why was this MR needed?

With FF_USE_PARALLEL_CACHE_TRANSFER or FF_USE_PARALLEL_ARTIFACT_TRANSFER enabled, both fetchers accepted 200 and 206 for every chunk Range GET (cache_extractor.go#L350, gitlab.go#L1224). A 200 means the server ignored Range and sent the whole object from byte 0. transfer.ParallelRangeDownload reads the first chunk length bytes of that body and writes them with WriteAt at the chunk's offset (parallel_download.go#L52-71), so every chunk answered with 200 holds the opening bytes of the object. A 206 for another range, or one without Content-Range, passed the same check and was written at the chunk's offset too. The cache archive or the artifact on disk is corrupted and the download reports success. On the cache path the corrupt archive also keeps the object's Last-Modified time, so a later cache extractor run that finds it on disk logs that it is up to date and does not download it again.

The !6401 review, which added both fetchers, asked on the single-byte probe whether a server without Range support would ignore the header and send the full body. The probe does treat that 200 as "Range not honored" (cache_extractor.go#L273, gitlab.go#L1342), but it only sees the first request. A CDN, a proxy or a load-balanced object store can still answer a later chunk request with 200, for example when only some of its nodes or cache tiers support ranges.

Behavior change: such a chunk now fails the parallel download. For artifacts, DownloadArtifacts already falls back to a single download stream when tryArtifactParallelDownload returns false (gitlab.go#L1356-1359). For the cache, tryPresignedParallelDownload now returns done=false when a chunk is answered with 200, so handlePresignedURL makes the sequential GET instead of retrying the probe and the whole parallel download against a server that answers the same way each time. A 206 for another range or without Content-Range fails the cache download and is retried, like a chunk answered with an error status. The sequential GET would not help there: a caching proxy whose cache key leaves out Range answers it with the same partial 206, and the sequential download would keep that as the archive.

Not covered here, each with a different cause:

• The GoCloud parallel path for S3 and Azure: s3blob and azblob hand back a 200 body without checking it, and the driver hides the response from this code (#40108).
• Chunks of an object replaced during the download: no chunk request sends If-Match, so the archive can mix two versions (#40109).
• After a failed parallel attempt, the artifact single-stream fallback writes over the file without truncating it (#40110).
• The sequential cache download keeps a 206 answer to its GET without Range as the archive, which is why a chunk makes the cache download fall back to it only on a 200 (#40111).

What's the best way to test this MR?

Unless a case says otherwise, the tests use an httptest server that answers every Range request starting at byte 0 (the cache probe and the first chunk) as requested:

• TestCacheExtractorPresignedParallelTransferFallsBackOnFullBodyChunk in commands/helpers: every chunk after the first is answered with 200 and the whole object. The extractor makes one sequential GET and the archive equals the object.
• TestCacheExtractorPresignedParallelTransferRejectsChunkOutsideRange in commands/helpers: every chunk after the first is answered with a 206 for another range or with a 206 without Content-Range, and in a third case every request, a GET without Range included, gets the one-byte 206 a caching proxy whose cache key leaves out Range stores for the probe. The download fails with a retryable error, makes no GET without Range and writes no archive.
• TestCacheExtractorPresignedParallelTransferFailsOnChunkError in commands/helpers: a chunk answered with 503 still fails with a retryable error and no sequential GET.
• TestTryArtifactParallelDownload in network: every chunk is answered as requested, the parallel download succeeds and the file equals the object.
• TestTryArtifactParallelDownloadRejectsChunkOutsideRange in network: the same three answers for the chunk after the first make the parallel download fail.
• TestDownloadArtifactsParallelFallsBackOnChunkOutsideRange in network: through DownloadArtifacts with FF_USE_PARALLEL_ARTIFACT_TRANSFER, the middle chunk of three is answered with 200. The result is DownloadSucceeded, one GET without Range, and a file equal to the object.

go test -race -count=1 -run 'TestCacheExtractorPresignedParallelTransfer|TestTryArtifactParallelDownload|TestDownloadArtifactsParallel' ./commands/helpers/ ./network/

With the fetchers from main, both new cache tests fail in every case, TestTryArtifactParallelDownloadRejectsChunkOutsideRange fails in all three, and the DownloadArtifacts test fails (no GET without Range, file differs from the object). With the Content-Range check alone, the three network tests and the 206 cache tests pass, and the cache fallback test still fails with range request failed: 200 OK. I also checked that each test catches a change it is there for: rejecting every chunk response fails TestTryArtifactParallelDownload, sending no Range fails all three network tests, and going back to accepting 200 or any 206 fails the other two. On the cache path, falling back on a 206 as well fails all three cases of TestCacheExtractorPresignedParallelTransferRejectsChunkOutsideRange, and treating every chunk failure as a Range problem also fails TestCacheExtractorPresignedParallelTransferFailsOnChunkError. I ran go test -race -count=1 ./commands/helpers/ ./network/ ./helpers/transfer/, GOOS=windows and GOOS=darwin go vet on those packages, and make lint in the runner-linters:2.12.2-go1.26.5 image the go:lint job uses (0 issues., test directives included).

Test output and what lands on disk without the fix

With the fetchers from main at 3c07c5b0:

--- FAIL: TestCacheExtractorPresignedParallelTransferFallsBackOnFullBodyChunk
        Messages: the extractor should fall back to one sequential GET
--- FAIL: TestCacheExtractorPresignedParallelTransferRejectsChunkOutsideRange
    --- FAIL: .../206_for_another_range
    --- FAIL: .../206_without_Content-Range
        Messages: a chunk outside its range should fail with a retryable error
    --- FAIL: .../206_a_caching_proxy_replays_to_every_request
        Error: "chunk read at offset 21: unexpected EOF" does not contain "range request failed: 206 Partial Content"
--- PASS: TestCacheExtractorPresignedParallelTransferFailsOnChunkError
--- PASS: TestTryArtifactParallelDownload
--- FAIL: TestTryArtifactParallelDownloadRejectsChunkOutsideRange
    --- FAIL: .../200_with_the_whole_object
    --- FAIL: .../206_for_another_range
    --- FAIL: .../206_without_Content-Range
        Messages: a chunk response outside the requested range must fail the parallel download
--- FAIL: TestDownloadArtifactsParallelFallsBackOnChunkOutsideRange
        Messages: DownloadArtifacts should fall back to one GET without Range
        Messages: the file differs from the object (33555456 bytes, want 33555456)

A throwaway test that reads the file back with a server answering later chunks with 200 shows what each path leaves behind on main: the cache archive has 144 of its 159 bytes wrong and zip.NewReader refuses it (zip: not a valid zip file), and the artifact (16 MiB + 1 KiB) ends with the object's first 1024 bytes, while both downloads report success. Running the cache download a second time against the corrupt archive on main sends only the probe and logs archive.zip is up to date.

With this MR:

ok  gitlab.com/gitlab-org/gitlab-runner/commands/helpers
ok  gitlab.com/gitlab-org/gitlab-runner/network
ok  gitlab.com/gitlab-org/gitlab-runner/helpers/transfer

The tests that call tryArtifactParallelDownload directly call ensureTLSConfig on the client first, as DownloadArtifacts does before its probe on the same cached client. Without that the chunk goroutines create the transport concurrently and the race detector reports it in client.ensureTLSConfig. That is the unsynchronized client state already noted in the data race tracker, so I kept the tests to the order DownloadArtifacts uses and left the client alone. The DownloadArtifacts test needs no such step.

Environment: Linux amd64, Go 1.26.5 (from go.mod).

What are the relevant issue numbers?

Related to #40102 (closed)