blob: Discard objects in case sending blob tree entries fails

We do not currently discard the objects read by `sendBlobTreeEntry()` in
case the function fails. As a result, the object reader would be dirty
after the function has failed and wouldn't be returned to the catfile
cache.

Discard the object's contents in case the function fails. While this
hasn't caused any known issues, it's still the right thing to do.

1 files changed, 11 insertions, 5 deletions

diff --git a/internal/gitaly/service/blob/get_blobs.go b/internal/gitaly/service/blob/get_blobs.go
index b7e83bbe2..69632ec29 100644
--- a/internal/gitaly/service/blob/get_blobs.go
+++ b/internal/gitaly/service/blob/get_blobs.go
@@ -95,7 +95,12 @@ func sendGetBlobsResponse(
 	return nil
 }
 
-func sendBlobTreeEntry(response *gitalypb.GetBlobsResponse, stream gitalypb.BlobService_GetBlobsServer, objectReader catfile.ObjectReader, limit int64) error {
+func sendBlobTreeEntry(
+	response *gitalypb.GetBlobsResponse,
+	stream gitalypb.BlobService_GetBlobsServer,
+	objectReader catfile.ObjectReader,
+	limit int64,
+) (returnedErr error) {
 	ctx := stream.Context()
 
 	var readLimit int64
@@ -119,6 +124,11 @@ func sendBlobTreeEntry(response *gitalypb.GetBlobsResponse, stream gitalypb.Blob
 	if err != nil {
 		return status.Errorf(codes.Internal, "GetBlobs: %v", err)
 	}
+	defer func() {
+		if _, err := io.Copy(io.Discard, blobObj.Reader); err != nil && returnedErr == nil {
+			returnedErr = status.Errorf(codes.Internal, "GetBlobs: discarding data: %v", err)
+		}
+	}()
 	if blobObj.Type != "blob" {
 		return status.Errorf(codes.Internal, "blob got unexpected type %q", blobObj.Type)
 	}
@@ -140,10 +150,6 @@ func sendBlobTreeEntry(response *gitalypb.GetBlobsResponse, stream gitalypb.Blob
 		return status.Errorf(codes.Unavailable, "GetBlobs: send: %v", err)
 	}
 
-	if _, err := io.Copy(io.Discard, blobObj.Reader); err != nil {
-		return status.Errorf(codes.Unavailable, "GetBlobs: discarding data: %v", err)
-	}
-
 	return nil
 }