Skip to content

Commit abef389

Browse files
AkihiroSudaAkihiroSuda
committed
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 <[email protected]>
1 parent 21e89eb commit abef389

3 files changed

Lines changed: 87 additions & 5 deletions

File tree

‎content/testsuite/provideringester.go‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/*
2+
Copyright The containerd Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package testsuite
18+
19+
import (
20+
"context"
21+
"io"
22+
"io/ioutil"
23+
"testing"
24+
25+
"github.com/containerd/containerd/content"
26+
digest "github.com/opencontainers/go-digest"
27+
)
28+
29+
// ProviderIngester consists of provider and ingester.
30+
// This interface is used for test cases that does not require
31+
// complete content.Store implementation.
32+
type ProviderIngester interface {
33+
content.Provider
34+
content.Ingester
35+
}
36+
37+
// TestSmallBlob tests reading a blob which is smaller than the read size.
38+
// This test is exposed so that it can be used for incomplete content.Store implementation.
39+
func TestSmallBlob(ctx context.Context, t *testing.T, store ProviderIngester) {
40+
blob := []byte(`foobar`)
41+
blobSize := int64(len(blob))
42+
blobDigest := digest.FromBytes(blob)
43+
// test write
44+
w, err := store.Writer(ctx, t.Name(), blobSize, blobDigest)
45+
if err != nil {
46+
t.Fatal(err)
47+
}
48+
if _, err := w.Write(blob); err != nil {
49+
t.Fatal(err)
50+
}
51+
if err := w.Commit(ctx, blobSize, blobDigest); err != nil {
52+
t.Fatal(err)
53+
}
54+
if err := w.Close(); err != nil {
55+
t.Fatal(err)
56+
}
57+
// test read.
58+
readSize := blobSize + 1
59+
ra, err := store.ReaderAt(ctx, blobDigest)
60+
if err != nil {
61+
t.Fatal(err)
62+
}
63+
r := io.NewSectionReader(ra, 0, readSize)
64+
b, err := ioutil.ReadAll(r)
65+
if err != nil {
66+
t.Fatal(err)
67+
}
68+
if err := ra.Close(); err != nil {
69+
t.Fatal(err)
70+
}
71+
d := digest.FromBytes(b)
72+
if blobDigest != d {
73+
t.Fatalf("expected %s (%q), got %s (%q)", blobDigest, string(blob),
74+
d, string(b))
75+
}
76+
}

‎content/testsuite/testsuite.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ func ContentSuite(t *testing.T, name string, storeFn func(ctx context.Context, r
3232
t.Run("ResumeCopySeeker", makeTest(t, name, storeFn, checkResume(resumeCopySeeker)))
3333
t.Run("ResumeCopyReaderAt", makeTest(t, name, storeFn, checkResume(resumeCopyReaderAt)))
3434
t.Run("Labels", makeTest(t, name, storeFn, checkLabels))
35+
36+
t.Run("SmallBlob", makeTest(t, name, storeFn, func(ctx context.Context, t *testing.T, cs content.Store) {
37+
TestSmallBlob(ctx, t, cs)
38+
}))
3539
}
3640

3741
func makeTest(t *testing.T, name string, storeFn func(ctx context.Context, root string) (context.Context, content.Store, func() error, error), fn func(ctx context.Context, t *testing.T, cs content.Store)) func(t *testing.T) {

‎services/content/service.go‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,9 @@ func (s *service) Read(req *api.ReadContentRequest, session api.Content_ReadServ
177177

178178
var (
179179
offset = req.Offset
180-
size = req.Size_
180+
// size is read size, not the expected size of the blob (oi.Size), which the caller might not be aware of.
181+
// offset+size can be larger than oi.Size.
182+
size = req.Size_
181183

182184
// TODO(stevvooe): Using the global buffer pool. At 32KB, it is probably
183185
// 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
189191
offset = 0
190192
}
191193

192-
if size <= 0 {
193-
size = oi.Size - offset
194+
if offset > oi.Size {
195+
return status.Errorf(codes.OutOfRange, "read past object length %v bytes", oi.Size)
194196
}
195197

196-
if offset+size > oi.Size {
197-
return status.Errorf(codes.OutOfRange, "read past object length %v bytes", oi.Size)
198+
if size <= 0 || offset+size > oi.Size {
199+
size = oi.Size - offset
198200
}
199201

200202
_, err = io.CopyBuffer(

0 commit comments

Comments
 (0)