Skip to content

Commit ae108b3

Browse files
committed
fix: update retry test expectations and validate Content-Range on resume
Update TestPullMaxRetriesExhausted to expect 5 HTTP calls (1 initial + 4 retries) and the correct error message now that Pull uses maxRetries=4. Also address a bug-risk review comment in rangeTransport.RoundTrip: add Content-Range header validation so we only record a range request as successful when the server returns 206 with a start offset matching the requested offset. This guards against misbehaving servers that return 206 with a different range, which would cause WriteBlob to append at the wrong position and corrupt the blob.
1 parent 5959463 commit ae108b3

2 files changed

Lines changed: 37 additions & 8 deletions

File tree

cmd/cli/desktop/desktop_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -191,13 +191,13 @@ func TestPullMaxRetriesExhausted(t *testing.T) {
191191
mockContext := NewContextForMock(mockClient)
192192
client := New(mockContext)
193193

194-
// All 4 attempts (1 initial + 3 retries) fail with network error
195-
mockClient.EXPECT().Do(gomock.Any()).Return(nil, io.EOF).Times(4)
194+
// All 5 attempts (1 initial + 4 retries) fail with network error
195+
mockClient.EXPECT().Do(gomock.Any()).Return(nil, io.EOF).Times(5)
196196

197197
printer := NewSimplePrinter(func(s string) {})
198198
_, _, err := client.Pull(modelName, printer)
199199
assert.Error(t, err)
200-
assert.Contains(t, err.Error(), "download failed after 3 retries")
200+
assert.Contains(t, err.Error(), "download failed after 4 retries")
201201
}
202202

203203
func TestPushRetryOnNetworkError(t *testing.T) {

pkg/distribution/oci/remote/remote.go

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -198,18 +198,47 @@ func (t *rangeTransport) RoundTrip(req *http.Request) (*http.Response, error) {
198198
}
199199

200200
// If we requested a Range, record success only when the server honoured it
201-
// with 206 Partial Content. A 200 response means the server ignored the Range
202-
// header and is sending the full file from byte 0; appending that stream to
203-
// the existing partial file would produce a corrupt blob.
201+
// with 206 Partial Content and a matching Content-Range start offset. A 200
202+
// response means the server ignored the Range header and is sending the full
203+
// file from byte 0; appending that stream to the existing partial file would
204+
// produce a corrupt blob. We also validate the Content-Range start offset to
205+
// guard against a misbehaving server that returns 206 with a different range.
204206
if requestedOffset > 0 && resp.StatusCode == http.StatusPartialContent {
205-
if rs := GetRangeSuccess(req.Context()); rs != nil {
206-
rs.Add(digest, requestedOffset)
207+
if rangeStartMatchesOffset(resp.Header.Get("Content-Range"), requestedOffset) {
208+
if rs := GetRangeSuccess(req.Context()); rs != nil {
209+
rs.Add(digest, requestedOffset)
210+
}
207211
}
208212
}
209213

210214
return resp, nil
211215
}
212216

217+
// rangeStartMatchesOffset parses the Content-Range response header and reports
218+
// whether its start byte equals the given offset. The format is defined by
219+
// RFC 9110: "bytes START-END/TOTAL" (TOTAL may be "*"). We fail closed: if the
220+
// header is absent or cannot be parsed we return false so that the caller does
221+
// not treat an ambiguous response as a successful range request.
222+
func rangeStartMatchesOffset(contentRange string, offset int64) bool {
223+
if contentRange == "" {
224+
return false
225+
}
226+
// Trim the unit prefix "bytes " and split on "-"
227+
after, ok := strings.CutPrefix(contentRange, "bytes ")
228+
if !ok {
229+
return false
230+
}
231+
dashIdx := strings.Index(after, "-")
232+
if dashIdx < 0 {
233+
return false
234+
}
235+
var start int64
236+
if _, err := fmt.Sscanf(after[:dashIdx], "%d", &start); err != nil {
237+
return false
238+
}
239+
return start == offset
240+
}
241+
213242
// extractDigestAndOffset extracts the blob digest from the request URL and returns
214243
// the corresponding resume offset if one exists.
215244
func (t *rangeTransport) extractDigestAndOffset(req *http.Request, offsets map[string]int64) (string, int64) {

0 commit comments

Comments
 (0)