Repository: buchgr/bazel-remote
Pull request #266 (2020-05-07 15:45:30)

Consistent handling of empty files in CAS
Hi,

There were special handling of empty files in server/grpc_cas.go and
server/http.go. It assumed that empty files are not stored in CAS.
The reason was to handle clients (like bazel?) which in some cases
don't upload the empty blob, but do reference it. It is kind of OK
not to upload the empty blob, since it can be recongnised by hash
and trivally regenerated.

**Problem was** that server/grpc_bytestream.go,
GetValidatedActionResult in disk.go and perhaps more places, still
expected the empty blobs to be available on disk.

**This commit moves the logic into the disk.go to assure the same
logic is used consistenly everywhere.**

Would you accept this pull request, if I add and update unit tests?

BR
Ulrik Falklöf

ulrfa: > Do you know of existing clients that are broken with the current setup?

I don't know  (see my comment in #267) 

> Maybe we could add a size argument to disk.Get and disk.Contains and have the HTTP code specify -1 if the size is unkown? ... Then we can use a simple less than or equal to zero check before doing a string comparison with the empty hash.

Yes, good idea!

> Re the bytestream API, it is only intended to be used for large files (which are necessarily non-empty). It wouldn't hurt us to have a sanity check here too, but if you have clients doing this it might be worth reporting issues against them to switch to the ContentAddressableStorage APIs for small files, which I think are more efficient.

Good point! I don't know if there are such clients. Do you prefer grpc_bytstream.go to reject empty file requests with a clear error message? Or accept them and let disk.go handle also empty ones transparently?

> Re GetValidatedActionResult, I thought I covered all the corner cases where empty CAS blobs are valid, but it would not surprise me if I missed something.

Could the following be empty?

https://github.com/buchgr/bazel-remote/blob/69bce621e7ab073e92c7ec479e0708eb315d5b6a/cache/disk/disk.go#L663

https://github.com/buchgr/bazel-remote/blob/69bce621e7ab073e92c7ec479e0708eb315d5b6a/cache/disk/disk.go#L674

mostynb: I think you're right about the two extra cases in GetValidatedActionResult. Here's a quickfix for those, and for the bytestream API (let's just handle this corner case, even though clients shouldn't use it): https://github.com/buchgr/bazel-remote/pull/270

Would you like to redo this PR, adding a size parameter to Get and Contains as described above (to be completed after I tag the next release)?
ulrfa: Yes, I would like to redo this PR, but I wait until after your release. I will then add a size parameter to Get and Contains.
ulrfa: The PR is now updated. Is it as you suggested @mostynb, with the added size parameter to Get and Contains?
ulrfa: Thanks for the review! Something more?

ulrfa: Thanks! I have now pushed a squashed commit, including the comment.

Patch:
diff --git a/cache/disk/disk.go b/cache/disk/disk.go
index c30aab6..8976f6d 100644
--- a/cache/disk/disk.go
+++ b/cache/disk/disk.go
@@ -1,6 +1,7 @@
 package disk
 
 import (
+	"bytes"
 	"crypto/sha256"
 	"encoding/hex"
 	"fmt"
@@ -59,6 +60,7 @@ type nameAndInfo struct {
 }
 
 const sha256HashStrSize = sha256.Size * 2 // Two hex characters per byte.
+const emptySha256 = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
 
 // New returns a new instance of a filesystem-based cache rooted at `dir`,
 // with a maximum size of `maxSizeBytes` bytes and an optional backend `proxy`.
@@ -257,6 +259,11 @@ func (c *Cache) Put(kind cache.EntryKind, hash string, expectedSize int64, r io.
 			len(hash), sha256.Size)
 	}
 
+	if kind == cache.CAS && expectedSize == 0 && hash == emptySha256 {
+		io.Copy(ioutil.Discard, r)
+		return nil
+	}
+
 	key := cacheKey(kind, hash)
 
 	c.mu.Lock()
@@ -403,8 +410,9 @@ func (c *Cache) availableOrTryProxy(key string) (available bool, tryProxy bool)
 // Get returns an io.ReadCloser with the content of the cache item stored under `hash`
 // and the number of bytes that can be read from it. If the item is not found, the
 // io.ReadCloser will be nil. If some error occurred when processing the request, then
-// it is returned.
-func (c *Cache) Get(kind cache.EntryKind, hash string) (io.ReadCloser, int64, error) {
+// it is returned. The 'size' of the content to be retreived shall be provided when
+// known, or as -1 when unknown.
+func (c *Cache) Get(kind cache.EntryKind, hash string, size int64) (io.ReadCloser, int64, error) {
 
 	// The hash format is checked properly in the http/grpc code.
 	// Just perform a simple/fast check here, to catch bad tests.
@@ -413,6 +421,11 @@ func (c *Cache) Get(kind cache.EntryKind, hash string) (io.ReadCloser, int64, er
 			len(hash), sha256.Size)
 	}
 
+	if kind == cache.CAS && size <= 0 && hash == emptySha256 {
+		cacheHits.Inc()
+		return ioutil.NopCloser(bytes.NewReader([]byte{})), 0, nil
+	}
+
 	var err error
 	key := cacheKey(kind, hash)
 
@@ -423,11 +436,16 @@ func (c *Cache) Get(kind cache.EntryKind, hash string) (io.ReadCloser, int64, er
 		var fileInfo os.FileInfo
 		fileInfo, err = os.Stat(blobPath)
 		if err == nil {
+			foundSize := fileInfo.Size()
+			if isSizeMismatch(size, foundSize) {
+				cacheMisses.Inc()
+				return nil, -1, nil
+			}
 			var f *os.File
 			f, err = os.Open(blobPath)
 			if err == nil {
 				cacheHits.Inc()
-				return f, fileInfo.Size(), nil
+				return f, foundSize, nil
 			}
 		}
 
@@ -483,6 +501,9 @@ func (c *Cache) Get(kind cache.EntryKind, hash string) (io.ReadCloser, int64, er
 	if err != nil || r == nil {
 		return nil, -1, err
 	}
+	if isSizeMismatch(size, foundSize) {
+		return nil, -1, nil
+	}
 
 	f, err = os.Create(tmpFilePath)
 	if err != nil {
@@ -529,7 +550,10 @@ func (c *Cache) Get(kind cache.EntryKind, hash string) (io.ReadCloser, int64, er
 //
 // If there is a local cache miss, the proxy backend (if there is
 // one) will be checked.
-func (c *Cache) Contains(kind cache.EntryKind, hash string) (bool, int64) {
+//
+// The 'size' of the potential content, shall be provided when known,
+// or as -1 when unknown.
+func (c *Cache) Contains(kind cache.EntryKind, hash string, size int64) (bool, int64) {
 
 	// The hash format is checked properly in the http/grpc code.
 	// Just perform a simple/fast check here, to catch bad tests.
@@ -537,26 +561,37 @@ func (c *Cache) Contains(kind cache.EntryKind, hash string) (bool, int64) {
 		return false, int64(-1)
 	}
 
-	var foundLocally bool
-	size := int64(-1)
+	if kind == cache.CAS && size <= 0 && hash == emptySha256 {
+		return true, 0
+	}
+
+	var found bool
+	foundSize := int64(-1)
 	key := cacheKey(kind, hash)
 
 	c.mu.Lock()
-	val, found := c.lru.Get(key)
+	val, isInLru := c.lru.Get(key)
 	// Uncommitted (i.e. uploading items) should be reported as not ok
-	if found {
+	if isInLru {
 		item := val.(*lruItem)
-		foundLocally = item.committed
-		size = item.size
+		found = item.committed
+		foundSize = item.size
 	}
 	c.mu.Unlock()
 
-	if foundLocally {
-		return true, size
+	if found {
+		if isSizeMismatch(size, foundSize) {
+			return false, int64(-1)
+		}
+		return true, foundSize
 	}
 
 	if c.proxy != nil {
-		return c.proxy.Contains(kind, hash)
+		found, foundSize = c.proxy.Contains(kind, hash)
+		if isSizeMismatch(size, foundSize) {
+			return false, int64(-1)
+		}
+		return found, foundSize
 	}
 
 	return false, int64(-1)
@@ -577,6 +612,10 @@ func (c *Cache) Stats() (currentSize int64, numItems int) {
 	return c.lru.CurrentSize(), c.lru.Len()
 }
 
+func isSizeMismatch(requestedSize int64, foundSize int64) bool {
+	return requestedSize > -1 && foundSize > -1 && requestedSize != foundSize
+}
+
 func ensureDirExists(path string) {
 	if _, err := os.Stat(path); os.IsNotExist(err) {
 		err = os.MkdirAll(path, os.ModePerm)
@@ -599,7 +638,8 @@ func cacheFilePath(kind cache.EntryKind, cacheDir string, hash string) string {
 // not, nil values are returned. If something unexpected went wrong, return
 // an error.
 func (c *Cache) GetValidatedActionResult(hash string) (*pb.ActionResult, []byte, error) {
-	rdr, sizeBytes, err := c.Get(cache.AC, hash)
+
+	rdr, sizeBytes, err := c.Get(cache.AC, hash, -1)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -620,8 +660,8 @@ func (c *Cache) GetValidatedActionResult(hash string) (*pb.ActionResult, []byte,
 	}
 
 	for _, f := range result.OutputFiles {
-		if len(f.Contents) == 0 && f.Digest.SizeBytes > 0 {
-			found, _ := c.Contains(cache.CAS, f.Digest.Hash)
+		if len(f.Contents) == 0 {
+			found, _ := c.Contains(cache.CAS, f.Digest.Hash, f.Digest.SizeBytes)
 			if !found {
 				return nil, nil, nil // aka "not found"
 			}
@@ -629,7 +669,7 @@ func (c *Cache) GetValidatedActionResult(hash string) (*pb.ActionResult, []byte,
 	}
 
 	for _, d := range result.OutputDirectories {
-		r, size, err := c.Get(cache.CAS, d.TreeDigest.Hash)
+		r, size, err := c.Get(cache.CAS, d.TreeDigest.Hash, d.TreeDigest.SizeBytes)
 		if r == nil {
 			return nil, nil, err // aka "not found", or an err if non-nil
 		}
@@ -657,10 +697,10 @@ func (c *Cache) GetValidatedActionResult(hash string) (*pb.ActionResult, []byte,
 		}
 
 		for _, f := range tree.Root.GetFiles() {
-			if f.Digest == nil || f.Digest.SizeBytes == 0 {
+			if f.Digest == nil {
 				continue
 			}
-			found, _ := c.Contains(cache.CAS, f.Digest.Hash)
+			found, _ := c.Contains(cache.CAS, f.Digest.Hash, f.Digest.SizeBytes)
 			if !found {
 				return nil, nil, nil // aka "not found"
 			}
@@ -668,10 +708,10 @@ func (c *Cache) GetValidatedActionResult(hash string) (*pb.ActionResult, []byte,
 
 		for _, child := range tree.GetChildren() {
 			for _, f := range child.GetFiles() {
-				if f.Digest == nil || f.Digest.SizeBytes == 0 {
+				if f.Digest == nil {
 					continue
 				}
-				found, _ := c.Contains(cache.CAS, f.Digest.Hash)
+				found, _ := c.Contains(cache.CAS, f.Digest.Hash, f.Digest.SizeBytes)
 				if !found {
 					return nil, nil, nil // aka "not found"
 				}
@@ -679,15 +719,15 @@ func (c *Cache) GetValidatedActionResult(hash string) (*pb.ActionResult, []byte,
 		}
 	}
 
-	if result.StdoutDigest != nil && result.StdoutDigest.SizeBytes > 0 {
-		found, _ := c.Contains(cache.CAS, result.StdoutDigest.Hash)
+	if result.StdoutDigest != nil {
+		found, _ := c.Contains(cache.CAS, result.StdoutDigest.Hash, result.StdoutDigest.SizeBytes)
 		if !found {
 			return nil, nil, nil // aka "not found"
 		}
 	}
 
-	if result.StderrDigest != nil && result.StderrDigest.SizeBytes > 0 {
-		found, _ := c.Contains(cache.CAS, result.StderrDigest.Hash)
+	if result.StderrDigest != nil {
+		found, _ := c.Contains(cache.CAS, result.StderrDigest.Hash, result.StderrDigest.SizeBytes)
 		if !found {
 			return nil, nil, nil // aka "not found"
 		}
diff --git a/server/grpc_ac.go b/server/grpc_ac.go
index 9f7b2d3..09b7892 100644
--- a/server/grpc_ac.go
+++ b/server/grpc_ac.go
@@ -44,10 +44,14 @@ func (s *grpcServer) GetActionResult(ctx context.Context,
 		return nil, err
 	}
 
+	// Clients provides hash and size of the Action, but not size of the ActionResult
+	// checked by the the disk cache.
+	const unknownActionResultSize = -1
+
 	if !s.depsCheck {
 		logPrefix = "GRPC AC GET NODEPSCHECK"
 
-		rdr, sizeBytes, err := s.cache.Get(cache.AC, req.ActionDigest.Hash)
+		rdr, sizeBytes, err := s.cache.Get(cache.AC, req.ActionDigest.Hash, unknownActionResultSize)
 		if err != nil {
 			s.accessLogger.Printf("%s %s %s", logPrefix, req.ActionDigest.Hash, err)
 			return nil, status.Error(codes.Unknown, err.Error())
@@ -145,7 +149,7 @@ func (s *grpcServer) maybeInline(inline bool, slice *[]byte, digest **pb.Digest,
 			}
 		}
 
-		found, _ := s.cache.Contains(cache.CAS, (*digest).Hash)
+		found, _ := s.cache.Contains(cache.CAS, (*digest).Hash, (*digest).SizeBytes)
 		if !found {
 			err := s.cache.Put(cache.CAS, (*digest).Hash, (*digest).SizeBytes,
 				bytes.NewReader(*slice))
diff --git a/server/grpc_asset.go b/server/grpc_asset.go
index 6cc2f8e..1a8a493 100644
--- a/server/grpc_asset.go
+++ b/server/grpc_asset.go
@@ -64,14 +64,14 @@ func (s *grpcServer) FetchBlob(ctx context.Context, req *asset.FetchBlobRequest)
 
 			sha256Str = hex.EncodeToString(decoded)
 
-			found, size := s.cache.Contains(cache.CAS, sha256Str)
+			found, size := s.cache.Contains(cache.CAS, sha256Str, -1)
 			if !found {
 				continue
 			}
 
 			if size < 0 {
 				// We don't know the size yet (bad http backend?).
-				r, size, err := s.cache.Get(cache.CAS, sha256Str)
+				r, size, err := s.cache.Get(cache.CAS, sha256Str, -1)
 				if r != nil {
 					defer r.Close()
 				}
diff --git a/server/grpc_cas.go b/server/grpc_cas.go
index 8f34234..7e9dcc0 100644
--- a/server/grpc_cas.go
+++ b/server/grpc_cas.go
@@ -37,13 +37,7 @@ func (s *grpcServer) FindMissingBlobs(ctx context.Context,
 			return nil, err
 		}
 
-		if digest.SizeBytes == 0 {
-			// The hash was validated, so we know it's OK.
-			s.accessLogger.Printf("GRPC CAS HEAD %s OK", hash)
-			continue
-		}
-
-		found, _ := s.cache.Contains(cache.CAS, hash)
+		found, _ := s.cache.Contains(cache.CAS, hash, digest.GetSizeBytes())
 		if !found {
 			s.accessLogger.Printf("GRPC CAS HEAD %s NOT FOUND", hash)
 			resp.MissingBlobDigests = append(resp.MissingBlobDigests, digest)
@@ -106,7 +100,7 @@ func (s *grpcServer) getBlobData(hash string, size int64) ([]byte, error) {
 		return []byte{}, nil
 	}
 
-	rdr, sizeBytes, err := s.cache.Get(cache.CAS, hash)
+	rdr, sizeBytes, err := s.cache.Get(cache.CAS, hash, size)
 	if err != nil {
 		rdr.Close()
 		return []byte{}, err
diff --git a/server/http.go b/server/http.go
index 5d9a821..6a9c87d 100644
--- a/server/http.go
+++ b/server/http.go
@@ -193,15 +193,7 @@ func (h *httpCache) CacheHandler(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 
-		if kind == cache.CAS && hash == emptySha256 {
-			w.Header().Set("Content-Type", "application/octet-stream")
-			w.Header().Set("Content-Length", "0")
-			w.Write([]byte{})
-			h.logResponse(http.StatusOK, r)
-			return
-		}
-
-		rdr, sizeBytes, err := h.cache.Get(kind, hash)
+		rdr, sizeBytes, err := h.cache.Get(kind, hash, -1)
 		if err != nil {
 			if e, ok := err.(*cache.Error); ok {
 				http.Error(w, e.Error(), e.Code)
@@ -236,13 +228,7 @@ func (h *httpCache) CacheHandler(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 
-		if contentLength == 0 && kind == cache.CAS {
-			if hash == emptySha256 {
-				w.WriteHeader(http.StatusOK)
-				h.logResponse(http.StatusOK, r)
-				return
-			}
-
+		if contentLength == 0 && kind == cache.CAS && hash != emptySha256 {
 			msg := fmt.Sprintf("Invalid empty blob hash: \"%s\"", hash)
 			http.Error(w, msg, http.StatusBadRequest)
 			h.errorLogger.Printf("PUT %s: %s", path(kind, hash), msg)
@@ -303,16 +289,9 @@ func (h *httpCache) CacheHandler(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 
-		if kind == cache.CAS && hash == emptySha256 {
-			w.Header().Set("Content-Length", "0")
-			w.WriteHeader(http.StatusOK)
-			h.logResponse(http.StatusOK, r)
-			return
-		}
-
 		// Unvalidated path:
 
-		ok, size := h.cache.Contains(kind, hash)
+		ok, size := h.cache.Contains(kind, hash, -1)
 		if !ok {
 			http.Error(w, "Not found", http.StatusNotFound)
 			h.logResponse(http.StatusNotFound, r)


Test patch:
diff --git a/cache/disk/disk_test.go b/cache/disk/disk_test.go
index 566050a..7930a11 100644
--- a/cache/disk/disk_test.go
+++ b/cache/disk/disk_test.go
@@ -72,6 +72,7 @@ func checkItems(cache *Cache, expSize int64, expNum int) error {
 const KEY = "a-key"
 const contents = "hello"
 const contentsHash = "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824"
+const contentsLength = int64(len(contents))
 
 func TestCacheBasics(t *testing.T) {
 	cacheDir := tempDir(t)
@@ -84,7 +85,7 @@ func TestCacheBasics(t *testing.T) {
 	}
 
 	// Non-existing item
-	rdr, _, err := testCache.Get(cache.CAS, contentsHash)
+	rdr, _, err := testCache.Get(cache.CAS, contentsHash, contentsLength)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -106,7 +107,7 @@ func TestCacheBasics(t *testing.T) {
 	}
 
 	// Get the item back
-	rdr, sizeBytes, err := testCache.Get(cache.CAS, contentsHash)
+	rdr, sizeBytes, err := testCache.Get(cache.CAS, contentsHash, contentsLength)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -183,6 +184,91 @@ func TestCachePutWrongSize(t *testing.T) {
 	}
 }
 
+func TestCacheGetContainsWrongSize(t *testing.T) {
+
+	cacheDir := tempDir(t)
+	defer os.RemoveAll(cacheDir)
+	testCache := New(cacheDir, 100, nil)
+
+	var found bool
+	var rdr io.ReadCloser
+
+	err := testCache.Put(cache.CAS, contentsHash, contentsLength, strings.NewReader(contents))
+	if err != nil {
+		t.Fatal("Expected success", err)
+	}
+
+	found, _ = testCache.Contains(cache.CAS, contentsHash, contentsLength+1)
+	if found {
+		t.Error("Expected not found, due to size being different")
+	}
+
+	rdr, _, _ = testCache.Get(cache.CAS, contentsHash, contentsLength+1)
+	if rdr != nil {
+		t.Error("Expected not found, due to size being different")
+	}
+
+	found, _ = testCache.Contains(cache.CAS, contentsHash, -1)
+	if !found {
+		t.Error("Expected found, when unknown size")
+	}
+
+	rdr, _, _ = testCache.Get(cache.CAS, contentsHash, -1)
+	if rdr == nil {
+		t.Error("Expected found, when unknown size")
+	}
+}
+
+func TestCacheGetContainsWrongSizeWithProxy(t *testing.T) {
+
+	cacheDir := tempDir(t)
+	defer os.RemoveAll(cacheDir)
+	testCache := New(cacheDir, 100, new(proxyStub))
+
+	var found bool
+	var rdr io.ReadCloser
+
+	// The proxyStub contains the digest {contentsHash, contentsLength}.
+
+	found, _ = testCache.Contains(cache.CAS, contentsHash, contentsLength+1)
+	if found {
+		t.Error("Expected not found, due to size being different")
+	}
+
+	rdr, _, _ = testCache.Get(cache.CAS, contentsHash, contentsLength+1)
+	if rdr != nil {
+		t.Error("Expected not found, due to size being different")
+	}
+	if err := checkItems(testCache, 0, 0); err != nil {
+		t.Fatal(err)
+	}
+
+	found, _ = testCache.Contains(cache.CAS, contentsHash, -1)
+	if !found {
+		t.Error("Expected found, when unknown size")
+	}
+
+	rdr, _, _ = testCache.Get(cache.CAS, contentsHash, -1)
+	if rdr == nil {
+		t.Error("Expected found, when unknown size")
+	}
+	if err := checkItems(testCache, contentsLength, 1); err != nil {
+		t.Fatal(err)
+	}
+}
+
+type proxyStub struct{}
+
+func (d proxyStub) Put(kind cache.EntryKind, hash string, size int64, rdr io.Reader) {}
+
+func (d proxyStub) Get(kind cache.EntryKind, hash string) (io.ReadCloser, int64, error) {
+	return ioutil.NopCloser(strings.NewReader(contents)), contentsLength, nil
+}
+
+func (d proxyStub) Contains(kind cache.EntryKind, hash string) (bool, int64) {
+	return true, contentsLength
+}
+
 func expectContentEquals(rdr io.ReadCloser, sizeBytes int64, expectedContent []byte) error {
 	if rdr == nil {
 		return fmt.Errorf("expected the item to exist")
@@ -216,7 +302,7 @@ func putGetCompareBytes(kind cache.EntryKind, hash string, data []byte, testCach
 		return err
 	}
 
-	rdr, sizeBytes, err := testCache.Get(kind, hash)
+	rdr, sizeBytes, err := testCache.Get(kind, hash, int64(len(data)))
 	if err != nil {
 		return err
 	}
@@ -307,7 +393,7 @@ func TestCacheExistingFiles(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	found, _ := testCache.Contains(cache.CAS, "f53b46209596d170f7659a414c9ff9f6b545cf77ffd6e1cbe9bcc57e1afacfbd")
+	found, _ := testCache.Contains(cache.CAS, "f53b46209596d170f7659a414c9ff9f6b545cf77ffd6e1cbe9bcc57e1afacfbd", contentsLength)
 	if found {
 		t.Fatalf("%s should have been evicted", items[0])
 	}
@@ -390,17 +476,17 @@ func TestMigrateFromOldDirectoryStructure(t *testing.T) {
 	}
 
 	var found bool
-	found, _ = testCache.Contains(cache.AC, acHash)
+	found, _ = testCache.Contains(cache.AC, acHash, 512)
 	if !found {
 		t.Fatalf("Expected cache to contain AC entry '%s'", acHash)
 	}
 
-	found, _ = testCache.Contains(cache.CAS, casHash1)
+	found, _ = testCache.Contains(cache.CAS, casHash1, 1024)
 	if !found {
 		t.Fatalf("Expected cache to contain CAS entry '%s'", casHash1)
 	}
 
-	found, _ = testCache.Contains(cache.CAS, casHash2)
+	found, _ = testCache.Contains(cache.CAS, casHash2, 1024)
 	if !found {
 		t.Fatalf("Expected cache to contain CAS entry '%s'", casHash2)
 	}
@@ -436,17 +522,17 @@ func TestLoadExistingEntries(t *testing.T) {
 
 	var found bool
 
-	found, _ = testCache.Contains(cache.AC, acHash)
+	found, _ = testCache.Contains(cache.AC, acHash, blobSize)
 	if !found {
 		t.Fatalf("Expected cache to contain AC entry '%s'", acHash)
 	}
 
-	found, _ = testCache.Contains(cache.CAS, casHash)
+	found, _ = testCache.Contains(cache.CAS, casHash, blobSize)
 	if !found {
 		t.Fatalf("Expected cache to contain CAS entry '%s'", casHash)
 	}
 
-	found, _ = testCache.Contains(cache.RAW, rawHash)
+	found, _ = testCache.Contains(cache.RAW, rawHash, blobSize)
 	if !found {
 		t.Fatalf("Expected cache to contain RAW entry '%s'", rawHash)
 	}
@@ -580,7 +666,7 @@ func TestHttpProxyBackend(t *testing.T) {
 	blob, casHash := testutils.RandomDataAndHash(blobSize)
 
 	// Non-existing item
-	r, _, err := testCache.Get(cache.CAS, casHash)
+	r, _, err := testCache.Get(cache.CAS, casHash, blobSize)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -614,12 +700,12 @@ func TestHttpProxyBackend(t *testing.T) {
 	// Confirm that it does not contain the item we added to the
 	// first testCache and the proxy backend.
 
-	found, _ := testCache.Contains(cache.CAS, casHash)
+	found, _ := testCache.Contains(cache.CAS, casHash, blobSize)
 	if found {
 		t.Fatalf("Expected the cache not to contain %s", casHash)
 	}
 
-	r, _, err = testCache.Get(cache.CAS, casHash)
+	r, _, err = testCache.Get(cache.CAS, casHash, blobSize)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -630,13 +716,13 @@ func TestHttpProxyBackend(t *testing.T) {
 	// Add the proxy backend and check that we can Get the item.
 	testCache.proxy = proxy
 
-	found, _ = testCache.Contains(cache.CAS, casHash)
+	found, _ = testCache.Contains(cache.CAS, casHash, blobSize)
 	if !found {
 		t.Fatalf("Expected the cache to contain %s (via the proxy)",
 			casHash)
 	}
 
-	r, fetchedSize, err := testCache.Get(cache.CAS, casHash)
+	r, fetchedSize, err := testCache.Get(cache.CAS, casHash, blobSize)
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/cache/httpproxy/httpproxy_test.go b/cache/httpproxy/httpproxy_test.go
index 35073e7..5ffee07 100644
--- a/cache/httpproxy/httpproxy_test.go
+++ b/cache/httpproxy/httpproxy_test.go
@@ -157,7 +157,7 @@ func TestEverything(t *testing.T) {
 	var found bool
 	var size int64
 
-	found, size = diskCache.Contains(cache.AC, hash)
+	found, size = diskCache.Contains(cache.AC, hash, int64(len(acData)))
 	if !found {
 		t.Fatalf("Expected to find AC item %s", hash)
 	}
@@ -166,7 +166,7 @@ func TestEverything(t *testing.T) {
 			len(acData), size)
 	}
 
-	found, size = diskCache.Contains(cache.CAS, hash)
+	found, size = diskCache.Contains(cache.CAS, hash, int64(len(casData)))
 	if !found {
 		t.Fatalf("Expected to find CAS item %s", hash)
 	}
@@ -180,7 +180,7 @@ func TestEverything(t *testing.T) {
 	var data []byte
 	var rc io.ReadCloser
 
-	rc, size, err = diskCache.Get(cache.AC, hash)
+	rc, size, err = diskCache.Get(cache.AC, hash, int64(len(acData)))
 	if err != nil {
 		t.Error(err)
 	}
@@ -200,7 +200,7 @@ func TestEverything(t *testing.T) {
 	}
 	rc.Close()
 
-	rc, size, err = diskCache.Get(cache.CAS, hash)
+	rc, size, err = diskCache.Get(cache.CAS, hash, int64(len(casData)))
 	if err != nil {
 		t.Error(err)
 	}
@@ -235,7 +235,7 @@ func TestEverything(t *testing.T) {
 
 	// Confirm that we can HEAD both values successfully.
 
-	found, size = diskCache.Contains(cache.AC, hash)
+	found, size = diskCache.Contains(cache.AC, hash, int64(len(acData)))
 	if !found {
 		t.Fatalf("Expected to find AC item %s", hash)
 	}
@@ -244,7 +244,7 @@ func TestEverything(t *testing.T) {
 			len(acData), size)
 	}
 
-	found, size = diskCache.Contains(cache.CAS, hash)
+	found, size = diskCache.Contains(cache.CAS, hash, int64(len(casData)))
 	if !found {
 		t.Fatalf("Expected to find CAS item %s", hash)
 	}
@@ -255,7 +255,7 @@ func TestEverything(t *testing.T) {
 
 	// Confirm that we can GET both values successfully.
 
-	rc, size, err = diskCache.Get(cache.AC, hash)
+	rc, size, err = diskCache.Get(cache.AC, hash, int64(len(acData)))
 	if err != nil {
 		t.Error(err)
 	}
@@ -275,7 +275,7 @@ func TestEverything(t *testing.T) {
 	}
 	rc.Close()
 
-	rc, size, err = diskCache.Get(cache.CAS, hash)
+	rc, size, err = diskCache.Get(cache.CAS, hash, int64(len(casData)))
 	if err != nil {
 		t.Error(err)
 	}
diff --git a/server/grpc_bytestream.go b/server/grpc_bytestream.go
index fc3307c..c85582b 100644
--- a/server/grpc_bytestream.go
+++ b/server/grpc_bytestream.go
@@ -87,17 +87,6 @@ func (s *grpcServer) Read(req *bytestream.ReadRequest,
 		return err
 	}
 
-	if size == 0 {
-		// While not technically an error, clients really shouldn't be
-		// asking for empty data with the bytestream API (IMO).
-		err = resp.Send(&bytestream.ReadResponse{Data: []byte{}})
-		if err != nil {
-			s.accessLogger.Printf("GRPC BYTESTREAM READ FAILED TO SEND RESPONSE: %s", err)
-			return status.Error(codes.Unknown, err.Error())
-		}
-		return nil
-	}
-
 	if req.ReadOffset > size {
 		msg := fmt.Sprintf("ReadOffset %d larger than expected data size %d resource: %s",
 			req.ReadOffset, size, req.ResourceName)
@@ -105,7 +94,7 @@ func (s *grpcServer) Read(req *bytestream.ReadRequest,
 		return status.Error(codes.OutOfRange, msg)
 	}
 
-	rdr, sizeBytes, err := s.cache.Get(cache.CAS, hash)
+	rdr, sizeBytes, err := s.cache.Get(cache.CAS, hash, size)
 	if err != nil {
 		msg := fmt.Sprintf("GRPC BYTESTREAM READ FAILED: %v", err)
 		s.accessLogger.Printf(msg)
diff --git a/server/grpc_test.go b/server/grpc_test.go
index f17147c..e2a7f0a 100644
--- a/server/grpc_test.go
+++ b/server/grpc_test.go
@@ -573,7 +573,7 @@ func TestGrpcByteStreamDeadline(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	_, sz, err := diskCache.Get(cache.CAS, testBlobHash)
+	_, sz, err := diskCache.Get(cache.CAS, testBlobHash, testBlobSize)
 	if err != nil {
 		t.Fatalf("get error: %v\n", err)
 	}