fix: attempt range query only if server explicitly supports (#369)

Fixes #368 Signed-off-by: Shiwei Zhang <shizh@microsoft.com>
oras-project · Dec 2, 2022 · 7dcd097 · 7dcd097
1 parent 511e4d4
commit 7dcd097
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 24 deletions.
diff --git a/registry/remote/example_test.go b/registry/remote/example_test.go
@@ -185,6 +185,7 @@ func TestMain(m *testing.M) {
 		case p == fmt.Sprintf("/v2/%s/blobs/%s", exampleRepositoryName, exampleLayerDigest):
 			w.Header().Set("Content-Type", ocispec.MediaTypeImageLayer)
 			w.Header().Set("Docker-Content-Digest", string(exampleLayerDigest))
+			w.Header().Set("Accept-Ranges", "bytes")
 			var start, end = 0, len(exampleLayer) - 1
 			if h := r.Header.Get("Range"); h != "" {
 				w.WriteHeader(http.StatusPartialContent)

diff --git a/registry/remote/repository.go b/registry/remote/repository.go
@@ -623,14 +623,6 @@ func (s *blobStore) Fetch(ctx context.Context, target ocispec.Descriptor) (rc io
 		return nil, err
 	}
 
-	// probe server range request ability.
-	// Docker spec allows range header form of "Range: bytes=<start>-<end>".
-	// However, the remote server may still not RFC 7233 compliant.
-	// Reference: https://docs.docker.com/registry/spec/api/#blob
-	if target.Size > 0 {
-		req.Header.Set("Range", fmt.Sprintf("bytes=0-%d", target.Size-1))
-	}
-
 	resp, err := s.repo.client().Do(req)
 	if err != nil {
 		return nil, err
@@ -646,9 +638,15 @@ func (s *blobStore) Fetch(ctx context.Context, target ocispec.Descriptor) (rc io
 		if size := resp.ContentLength; size != -1 && size != target.Size {
 			return nil, fmt.Errorf("%s %q: mismatch Content-Length", resp.Request.Method, resp.Request.URL)
 		}
+
+		// check server range request capability.
+		// Docker spec allows range header form of "Range: bytes=<start>-<end>".
+		// However, the remote server may still not RFC 7233 compliant.
+		// Reference: https://docs.docker.com/registry/spec/api/#blob
+		if rangeUnit := resp.Header.Get("Accept-Ranges"); rangeUnit == "bytes" {
+			return httputil.NewReadSeekCloser(s.repo.client(), req, resp.Body, target.Size), nil
+		}
 		return resp.Body, nil
-	case http.StatusPartialContent:
-		return httputil.NewReadSeekCloser(s.repo.client(), req, resp.Body, target.Size), nil
 	case http.StatusNotFound:
 		return nil, fmt.Errorf("%s: %w", target.Digest, errdef.ErrNotFound)
 	default:
@@ -808,13 +806,6 @@ func (s *blobStore) FetchReference(ctx context.Context, reference string) (desc
 		return ocispec.Descriptor{}, nil, err
 	}
 
-	// probe server range request ability.
-	// Docker spec allows range header form of "Range: bytes=<start>-<end>".
-	// The form of "Range: bytes=<start>-" is also acceptable.
-	// However, the remote server may still not RFC 7233 compliant.
-	// Reference: https://docs.docker.com/registry/spec/api/#blob
-	req.Header.Set("Range", "bytes=0-")
-
 	resp, err := s.repo.client().Do(req)
 	if err != nil {
 		return ocispec.Descriptor{}, nil, err
@@ -831,13 +822,15 @@ func (s *blobStore) FetchReference(ctx context.Context, reference string) (desc
 		if err != nil {
 			return ocispec.Descriptor{}, nil, err
 		}
-		return desc, resp.Body, nil
-	case http.StatusPartialContent:
-		desc, err = generateBlobDescriptor(resp, refDigest)
-		if err != nil {
-			return ocispec.Descriptor{}, nil, err
+
+		// check server range request capability.
+		// Docker spec allows range header form of "Range: bytes=<start>-<end>".
+		// However, the remote server may still not RFC 7233 compliant.
+		// Reference: https://docs.docker.com/registry/spec/api/#blob
+		if rangeUnit := resp.Header.Get("Accept-Ranges"); rangeUnit == "bytes" {
+			return desc, httputil.NewReadSeekCloser(s.repo.client(), req, resp.Body, desc.Size), nil
 		}
-		return desc, httputil.NewReadSeekCloser(s.repo.client(), req, resp.Body, desc.Size), nil
+		return desc, resp.Body, nil
 	case http.StatusNotFound:
 		return ocispec.Descriptor{}, nil, fmt.Errorf("%s: %w", ref, errdef.ErrNotFound)
 	default:

diff --git a/registry/remote/repository_test.go b/registry/remote/repository_test.go
@@ -45,7 +45,6 @@ import (
 )
 
 type testIOStruct struct {
-	name                    string
 	isTag                   bool
 	clientSuppliedReference string
 	serverCalculatedDigest  digest.Digest // for non-HEAD (body-containing) requests only
@@ -1967,6 +1966,9 @@ func Test_BlobStore_Fetch_Seek(t *testing.T) {
 		case "/v2/test/blobs/" + blobDesc.Digest.String():
 			w.Header().Set("Content-Type", "application/octet-stream")
 			w.Header().Set("Docker-Content-Digest", blobDesc.Digest.String())
+			if seekable {
+				w.Header().Set("Accept-Ranges", "bytes")
+			}
 			rangeHeader := r.Header.Get("Range")
 			if !seekable || rangeHeader == "" {
 				w.WriteHeader(http.StatusOK)
@@ -2487,6 +2489,9 @@ func Test_BlobStore_FetchReference_Seek(t *testing.T) {
 		case "/v2/test/blobs/" + blobDesc.Digest.String():
 			w.Header().Set("Content-Type", "application/octet-stream")
 			w.Header().Set("Docker-Content-Digest", blobDesc.Digest.String())
+			if seekable {
+				w.Header().Set("Accept-Ranges", "bytes")
+			}
 			rangeHeader := r.Header.Get("Range")
 			if !seekable || rangeHeader == "" {
 				w.WriteHeader(http.StatusOK)