From d8cc1d6252c1dceafc0d6172764bb601238b43b2 Mon Sep 17 00:00:00 2001 From: Alec Thomas Date: Wed, 24 Jun 2026 12:27:58 +1000 Subject: [PATCH] fix(client): don't validate no-ETag full read against stale total In the no-ETag fallback, ParallelGet did a fresh full Open but validated its body length against the total from the earlier discovery request. Since the full read is a separate request that may observe a different revision, that check could spuriously fail a valid download. Pass -1 so the completeness check is skipped, matching the existing full-body path and relying on transport-level EOF detection. Co-authored-by: Codex --- client/parallel_get.go | 5 ++++- client/parallel_get_test.go | 40 +++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/client/parallel_get.go b/client/parallel_get.go index dad25c27..531e2e51 100644 --- a/client/parallel_get.go +++ b/client/parallel_get.go @@ -81,7 +81,10 @@ func ParallelGet(ctx context.Context, c RangeReader, key Key, dst io.WriterAt, c if err != nil { return errors.Wrap(err, "parallel get: full read") } - return errors.Wrap(writeChunkAt(dst, 0, total, full), "parallel get") + // The full read is a fresh request whose body may be a different + // revision than discovery, so the discovery `total` cannot validate its + // length; -1 skips the check and relies on transport-level EOF detection. + return errors.Wrap(writeChunkAt(dst, 0, -1, full), "parallel get") } // Multiple chunks: copy the already-open first chunk concurrently with the diff --git a/client/parallel_get_test.go b/client/parallel_get_test.go index df17c3a3..791e1125 100644 --- a/client/parallel_get_test.go +++ b/client/parallel_get_test.go @@ -118,3 +118,43 @@ func TestParallelGetNoETagSingleChunk(t *testing.T) { assert.NoError(t, err) assert.Equal(t, data, dst.buf) } + +// changingSizeReader serves a multi-chunk body with no ETag on the ranged +// discovery request, then a differently sized body on the subsequent full +// (non-range) read, modelling an object rewritten between the two requests. +type changingSizeReader struct { + discovery []byte + rewritten []byte +} + +func (c *changingSizeReader) Open(_ context.Context, _ client.Key, opts ...client.RequestOption) (io.ReadCloser, http.Header, error) { + o := client.NewRequestOptions(opts...) + headers := http.Header{} + if o.Range == "" { + headers.Set("Content-Length", strconv.FormatInt(int64(len(c.rewritten)), 10)) + return io.NopCloser(bytes.NewReader(c.rewritten)), headers, nil + } + size := int64(len(c.discovery)) + start, length, outcome := o.ResolveRange(size, "") + if outcome == client.RangeNotSatisfiable { + headers.Set("Content-Range", fmt.Sprintf("bytes */%d", size)) + return nil, headers, client.ErrRangeNotSatisfiable + } + headers.Set("Content-Length", strconv.FormatInt(length, 10)) + if outcome == client.RangePartial { + headers.Set("Content-Range", fmt.Sprintf("bytes %d-%d/%d", start, start+length-1, size)) + } + return io.NopCloser(bytes.NewReader(c.discovery[start : start+length])), headers, nil +} + +func TestParallelGetNoETagSizeChangedBetweenRequests(t *testing.T) { + // A no-ETag multi-chunk object falls back to a single full read. If it is + // rewritten to a different size between discovery and that read, the + // discovery total must not be used to validate the full body: the full read + // is itself a consistent revision and should be accepted in its entirety. + c := &changingSizeReader{discovery: make([]byte, 1000), rewritten: []byte("changed")} + var dst bufferAt + err := client.ParallelGet(context.Background(), c, client.NewKey("k"), &dst, 100, 4) + assert.NoError(t, err) + assert.Equal(t, c.rewritten, dst.buf) +}