services/content: fix reading a blob which is smaller than the read buffer.

The newly added test fails without this fix in services/content/service.go:

    $ go test -c . && sudo ./containerd.test -test.v -test.root -test.run TestContentClient
    ...
        --- FAIL: TestContentClient/SmallBlob (0.02s)
            provideringester.go:62: rpc error: code = OutOfRange desc = read
    past object length 6 bytes
            helpers.go:67: drwx------       4096
    /tmp/content-suite-ContentClient-286788688
    FAIL

Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
This commit is contained in:
Akihiro Suda 2018-03-22 15:57:39 +09:00
parent 9304193b8c
commit 7b323b1402
3 changed files with 87 additions and 5 deletions

View File

@ -0,0 +1,76 @@
/*
Copyright The containerd Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package testsuite
import (
"context"
"io"
"io/ioutil"
"testing"
"github.com/containerd/containerd/content"
digest "github.com/opencontainers/go-digest"
)
// ProviderIngester consists of provider and ingester.
// This interface is used for test cases that does not require
// complete content.Store implementation.
type ProviderIngester interface {
content.Provider
content.Ingester
}
// TestSmallBlob tests reading a blob which is smaller than the read size.
// This test is exposed so that it can be used for incomplete content.Store implementation.
func TestSmallBlob(ctx context.Context, t *testing.T, store ProviderIngester) {
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))
}
}

View File

@ -50,6 +50,10 @@ func ContentSuite(t *testing.T, name string, storeFn func(ctx context.Context, r
t.Run("CrossNamespaceAppend", makeTest(t, name, storeFn, checkCrossNSAppend))
t.Run("CrossNamespaceShare", makeTest(t, name, storeFn, checkCrossNSShare))
t.Run("SmallBlob", makeTest(t, name, storeFn, func(ctx context.Context, t *testing.T, cs content.Store) {
TestSmallBlob(ctx, t, cs)
}))
}
// ContextWrapper is used to decorate new context used inside the test

View File

@ -187,7 +187,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.
@ -199,12 +201,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(