diff --git a/pkg/fetch/http_fetcher.go b/pkg/fetch/http_fetcher.go index 3e5fc60..4567648 100644 --- a/pkg/fetch/http_fetcher.go +++ b/pkg/fetch/http_fetcher.go @@ -146,7 +146,9 @@ func (hf *httpFetcher) downloadBlob(ctx context.Context, uri string, digestFunct log.Printf("Error downloading blob with URI %s: %v", uri, err) return buffer.NewBufferFromError(util.StatusWrapWithCode(err, codes.Internal, "HTTP request failed")), bb_digest.BadDigest, "" } - if resp.StatusCode != http.StatusOK { + // A Range header makes 206 the success status for the requested segment. + if resp.StatusCode != http.StatusOK && + !(resp.StatusCode == http.StatusPartialContent && req.Header.Get("Range") != "") { log.Printf("Error downloading blob with URI %s: %v", uri, resp.StatusCode) return buffer.NewBufferFromError(status.Errorf(codes.Internal, "HTTP request failed with status %#v", resp.Status)), bb_digest.BadDigest, "" } diff --git a/pkg/fetch/http_fetcher_test.go b/pkg/fetch/http_fetcher_test.go index f20e062..212e25f 100644 --- a/pkg/fetch/http_fetcher_test.go +++ b/pkg/fetch/http_fetcher_test.go @@ -531,6 +531,81 @@ func TestHTTPFetcherFetchBlob(t *testing.T) { }) } +func TestHTTPFetcherFetchBlobPartialContent(t *testing.T) { + ctrl, ctx := gomock.WithContext(context.Background(), t) + + instance := util.Must(digest.NewInstanceName(InstanceName)) + digestFunction, err := instance.GetDigestFunction(remoteexecution.DigestFunction_SHA256, 0) + require.NoError(t, err) + digestGenerator := digestFunction.NewGenerator(int64(len(TestData))) + digestGenerator.Write([]byte(TestData)) + helloDigest := digestGenerator.Sum() + + uri := "www.example.com" + checksum := &remoteasset.Qualifier{ + Name: "checksum.sri", + Value: digestToChecksumSri(remoteexecution.DigestFunction_SHA256, helloDigest), + } + + casBlobAccess := mock.NewMockBlobAccess(ctrl) + roundTripper := mock.NewMockRoundTripper(ctrl) + HTTPFetcher := fetch.NewHTTPFetcher(&http.Client{Transport: roundTripper}, casBlobAccess) + + t.Run("AcceptedWithRangeHeader", func(t *testing.T) { + tempDir := t.TempDir() + t.Setenv("TMPDIR", tempDir) + request := &remoteasset.FetchBlobRequest{ + InstanceName: InstanceName, + Uris: []string{uri}, + Qualifiers: []*remoteasset.Qualifier{ + checksum, + {Name: "http_header:Range", Value: "bytes=0-4"}, + }, + } + body := io.NopCloser(bytes.NewBuffer([]byte(TestData))) + httpDoCall := roundTripper.EXPECT().RoundTrip(&headerMatcher{ + headers: map[string]string{"Range": "bytes=0-4"}, + }).Return(&http.Response{ + Status: "206 Partial Content", + StatusCode: 206, + Body: body, + ContentLength: 5, + }, nil) + expectBlobPut(t, casBlobAccess, ctx, helloDigest).After(httpDoCall) + + response, err := HTTPFetcher.FetchBlob(ctx, request) + require.NoError(t, err) + require.True(t, proto.Equal(response.BlobDigest, helloDigest.GetProto())) + require.Equal(t, response.Status.Code, int32(codes.OK)) + requireNoTemporaryFiles(t, tempDir) + }) + + // Without a Range header on the request, 206 means the server sent + // something other than what was asked for. + t.Run("RejectedWithoutRangeHeader", func(t *testing.T) { + tempDir := t.TempDir() + t.Setenv("TMPDIR", tempDir) + request := &remoteasset.FetchBlobRequest{ + InstanceName: InstanceName, + Uris: []string{uri}, + Qualifiers: []*remoteasset.Qualifier{checksum}, + } + body := io.NopCloser(bytes.NewBuffer([]byte(TestData))) + roundTripper.EXPECT().RoundTrip(gomock.Any()).Return(&http.Response{ + Status: "206 Partial Content", + StatusCode: 206, + Body: body, + ContentLength: 5, + }, nil) + + response, err := HTTPFetcher.FetchBlob(ctx, request) + require.Nil(t, response) + testutil.RequireEqualStatus(t, status.Error(codes.NotFound, + `Unable to download blob from any provided URI: HTTP request failed with status "206 Partial Content"`), err) + requireNoTemporaryFiles(t, tempDir) + }) +} + func TestHTTPFetcherFetchDirectory(t *testing.T) { ctrl, ctx := gomock.WithContext(context.Background(), t) diff --git a/pkg/fetch/logging_fetcher.go b/pkg/fetch/logging_fetcher.go index 50dce92..8b44d01 100644 --- a/pkg/fetch/logging_fetcher.go +++ b/pkg/fetch/logging_fetcher.go @@ -20,8 +20,18 @@ func NewLoggingFetcher(fetcher Fetcher) Fetcher { } } +// Qualifier values are client supplied and can carry credentials, so only +// names are logged. +func qualifierNames(qualifiers []*remoteasset.Qualifier) []string { + names := make([]string, 0, len(qualifiers)) + for _, q := range qualifiers { + names = append(names, q.Name) + } + return names +} + func (lf *loggingFetcher) FetchBlob(ctx context.Context, req *remoteasset.FetchBlobRequest) (*remoteasset.FetchBlobResponse, error) { - log.Printf("Fetching Blob %s with qualifiers %s", req.Uris, req.Qualifiers) + log.Printf("Fetching Blob %s with qualifiers %s", req.Uris, qualifierNames(req.Qualifiers)) resp, err := lf.fetcher.FetchBlob(ctx, req) if err == nil { log.Printf("FetchBlob completed for %s with status code %d", req.Uris, resp.Status.GetCode()) @@ -32,7 +42,7 @@ func (lf *loggingFetcher) FetchBlob(ctx context.Context, req *remoteasset.FetchB } func (lf *loggingFetcher) FetchDirectory(ctx context.Context, req *remoteasset.FetchDirectoryRequest) (*remoteasset.FetchDirectoryResponse, error) { - log.Printf("Fetching Directory %s with qualifiers %s", req.Uris, req.Qualifiers) + log.Printf("Fetching Directory %s with qualifiers %s", req.Uris, qualifierNames(req.Qualifiers)) resp, err := lf.fetcher.FetchDirectory(ctx, req) if err == nil { log.Printf("FetchBlob completed for %s with status code %d", req.Uris, resp.Status.GetCode())