Merge pull request #2234 from AkihiroSuda/cherry-pick-2221

stevvooe · web-flow · commit bde0036230a9 · 2018-03-27T09:34:56.000-07:00
[release/1.0] services/content: fix reading a blob which is smaller than the read buffer
diff --git a/content/testsuite/testsuite.go b/content/testsuite/testsuite.go
@@ -31,6 +31,7 @@ func ContentSuite(t *testing.T, name string, storeFn func(ctx context.Context, r
 	t.Run("ResumeCopy", makeTest(t, name, storeFn, checkResume(resumeCopy)))
 	t.Run("ResumeCopySeeker", makeTest(t, name, storeFn, checkResume(resumeCopySeeker)))
 	t.Run("ResumeCopyReaderAt", makeTest(t, name, storeFn, checkResume(resumeCopyReaderAt)))
+	t.Run("SmallBlob", makeTest(t, name, storeFn, checkSmallBlob))
 	t.Run("Labels", makeTest(t, name, storeFn, checkLabels))
 }
 
@@ -467,6 +468,45 @@ func resumeCopyReaderAt(ctx context.Context, w content.Writer, b []byte, _, size
 	return errors.Wrap(content.Copy(ctx, w, r, size, dgst), "copy failed")
 }
 
+// checkSmallBlob tests reading a blob which is smaller than the read size.
+func checkSmallBlob(ctx context.Context, t *testing.T, store content.Store) {
+	blob := []byte(`foobar`)
+	blobSize := int64(len(blob))
+	blobDigest := digest.FromBytes(blob)
+	// test write
+	w, err := store.Writer(ctx, t.Name(), blobSize, blobDigest)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := w.Write(blob); err != nil {
+		t.Fatal(err)
+	}
+	if err := w.Commit(ctx, blobSize, blobDigest); err != nil {
+		t.Fatal(err)
+	}
+	if err := w.Close(); err != nil {
+		t.Fatal(err)
+	}
+	// test read.
+	readSize := blobSize + 1
+	ra, err := store.ReaderAt(ctx, blobDigest)
+	if err != nil {
+		t.Fatal(err)
+	}
+	r := io.NewSectionReader(ra, 0, readSize)
+	b, err := ioutil.ReadAll(r)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := ra.Close(); err != nil {
+		t.Fatal(err)
+	}
+	d := digest.FromBytes(b)
+	if blobDigest != d {
+		t.Fatalf("expected %s (%q), got %s (%q)", blobDigest, string(blob),
+			d, string(b))
+	}
+}
 func checkStatus(t *testing.T, w content.Writer, expected content.Status, d digest.Digest, preStart, postStart, preUpdate, postUpdate time.Time) {
 	t.Helper()
 	st, err := w.Status()
diff --git a/services/content/service.go b/services/content/service.go
@@ -177,7 +177,9 @@ func (s *service) Read(req *api.ReadContentRequest, session api.Content_ReadServ
 
 	var (
 		offset = req.Offset
-		size   = req.Size_
+		// size is read size, not the expected size of the blob (oi.Size), which the caller might not be aware of.
+		// offset+size can be larger than oi.Size.
+		size = req.Size_
 
 		// TODO(stevvooe): Using the global buffer pool. At 32KB, it is probably
 		// little inefficient for work over a fast network. We can tune this later.
@@ -189,12 +191,12 @@ func (s *service) Read(req *api.ReadContentRequest, session api.Content_ReadServ
 		offset = 0
 	}
 
-	if size <= 0 {
-		size = oi.Size - offset
+	if offset > oi.Size {
+		return status.Errorf(codes.OutOfRange, "read past object length %v bytes", oi.Size)
 	}
 
-	if offset+size > oi.Size {
-		return status.Errorf(codes.OutOfRange, "read past object length %v bytes", oi.Size)
+	if size <= 0 || offset+size > oi.Size {
+		size = oi.Size - offset
 	}
 
 	_, err = io.CopyBuffer(
