From 244e22a38d82f65bf3069cd6cdb9bd6f1e1c84c8 Mon Sep 17 00:00:00 2001 From: wangxiaoxuan273 Date: Mon, 19 Sep 2022 08:21:42 +0000 Subject: [PATCH 01/11] feat: add read and verify utility method Signed-off-by: wangxiaoxuan273 --- content/reader.go | 124 ++++++++++++++++++++++ content/{utils_test.go => reader_test.go} | 0 content/utils.go | 64 ----------- 3 files changed, 124 insertions(+), 64 deletions(-) create mode 100644 content/reader.go rename content/{utils_test.go => reader_test.go} (100%) delete mode 100644 content/utils.go diff --git a/content/reader.go b/content/reader.go new file mode 100644 index 00000000..c5eec8cc --- /dev/null +++ b/content/reader.go @@ -0,0 +1,124 @@ +/* +Copyright The ORAS 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 content + +import ( + "errors" + "fmt" + "io" + + "github.com/opencontainers/go-digest" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "oras.land/oras-go/v2/internal/ioutil" +) + +var ( + // ErrInvalidDescriptorSize is returned by ReadAll() when + // the descriptor has an invalid size. + ErrInvalidDescriptorSize = errors.New("invalid descriptor size") + + // ErrMismatchedDigest is returned by ReadAll() when + // the descriptor has an invalid digest. + ErrMismatchedDigest = errors.New("mismatched digest") + + // ErrTrailingData is returned by ReadAll() when + // there exists trailing data unread when the read terminates. + ErrTrailingData = errors.New("trailing data") +) + +// VerifyReader reads the content described by its descriptor and verifies +// against its size and digest. +type VerifyReader struct { + base *io.LimitedReader + verifier digest.Verifier + verified bool + err error +} + +// Read reads byte content into the VerifyReader. +func (vr *VerifyReader) Read(p []byte) (n int, err error) { + if vr.err != nil { + return 0, vr.err + } + + n, err = vr.base.Read(p) + if err != nil { + if err == io.EOF && vr.base.N > 0 { + err = io.ErrUnexpectedEOF + } + vr.err = err + } + return +} + +// Verify verifies the read content against the size and the digest. +func (vr *VerifyReader) Verify() error { + if vr.verified { + return nil + } + if vr.err == nil { + if vr.base.N > 0 { + return errors.New("early verify") + } + } else if vr.err != io.EOF { + return vr.err + } + + if err := ioutil.EnsureEOF(vr.base.R); err != nil { + vr.err = ErrTrailingData + return vr.err + } + if !vr.verifier.Verified() { + vr.err = ErrMismatchedDigest + return vr.err + } + + vr.verified = true + vr.err = io.EOF + return nil +} + +// NewVerifyReader returns a pointer to a new VerifyReader. +func NewVerifyReader(r io.Reader, desc ocispec.Descriptor) *VerifyReader { + verifier := desc.Digest.Verifier() + lr := &io.LimitedReader{ + R: io.TeeReader(r, verifier), + N: desc.Size, + } + return &VerifyReader{ + base: lr, + verifier: verifier, + } +} + +// ReadAll safely reads the content described by the descriptor. +// The read content is verified against the size and the digest +// using a VerifyReader. +func ReadAll(r io.Reader, desc ocispec.Descriptor) ([]byte, error) { + if desc.Size < 0 { + return nil, ErrInvalidDescriptorSize + } + buf := make([]byte, desc.Size) + + vr := NewVerifyReader(r, desc) + if _, err := io.ReadFull(vr, buf); err != nil { + return nil, fmt.Errorf("read failed: %w", err) + } + if err := vr.Verify(); err != nil { + return nil, err + } + return buf, nil +} diff --git a/content/utils_test.go b/content/reader_test.go similarity index 100% rename from content/utils_test.go rename to content/reader_test.go diff --git a/content/utils.go b/content/utils.go deleted file mode 100644 index 193becce..00000000 --- a/content/utils.go +++ /dev/null @@ -1,64 +0,0 @@ -/* -Copyright The ORAS 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 content - -import ( - "errors" - "fmt" - "io" - - ocispec "github.com/opencontainers/image-spec/specs-go/v1" - "oras.land/oras-go/v2/internal/ioutil" -) - -var ( - // ErrInvalidDescriptorSize is returned by ReadAll() when - // the descriptor has an invalid size. - ErrInvalidDescriptorSize = errors.New("invalid descriptor size") - - // ErrMismatchedDigest is returned by ReadAll() when - // the descriptor has an invalid digest. - ErrMismatchedDigest = errors.New("mismatched digest") - - // ErrTrailingData is returned by ReadAll() when - // there exists trailing data unread when the read terminates. - ErrTrailingData = errors.New("trailing data") -) - -// ReadAll safely reads the content described by the descriptor. -// The read content is verified against the size and the digest. -func ReadAll(r io.Reader, desc ocispec.Descriptor) ([]byte, error) { - if desc.Size < 0 { - return nil, ErrInvalidDescriptorSize - } - buf := make([]byte, desc.Size) - - // verify while reading - verifier := desc.Digest.Verifier() - r = io.TeeReader(r, verifier) - // verify the size of the read content - if _, err := io.ReadFull(r, buf); err != nil { - return nil, fmt.Errorf("read failed: %w", err) - } - if err := ioutil.EnsureEOF(r); err != nil { - return nil, ErrTrailingData - } - // verify the digest of the read content - if !verifier.Verified() { - return nil, ErrMismatchedDigest - } - return buf, nil -} From 1613257f178c6251ee31f801546bcdc3649b07bc Mon Sep 17 00:00:00 2001 From: wangxiaoxuan273 Date: Mon, 19 Sep 2022 11:01:51 +0000 Subject: [PATCH 02/11] refined unit test on read Signed-off-by: wangxiaoxuan273 --- content/reader.go | 3 +- content/reader_test.go | 88 +++++++++++++++++++++++++++++++++++++++--- 2 files changed, 84 insertions(+), 7 deletions(-) diff --git a/content/reader.go b/content/reader.go index c5eec8cc..390977c1 100644 --- a/content/reader.go +++ b/content/reader.go @@ -48,7 +48,8 @@ type VerifyReader struct { err error } -// Read reads byte content into the VerifyReader. +// Read reads up to len(p) bytes into p. It returns the number of bytes +// read (0 <= n <= len(p)) and any error encountered. func (vr *VerifyReader) Read(p []byte) (n int, err error) { if vr.err != nil { return 0, vr.err diff --git a/content/reader_test.go b/content/reader_test.go index f77bf555..d4665df5 100644 --- a/content/reader_test.go +++ b/content/reader_test.go @@ -26,7 +26,83 @@ import ( ocispec "github.com/opencontainers/image-spec/specs-go/v1" ) -func TestReadAllCorrectDescriptor(t *testing.T) { +func TestVerifyReader_Read(t *testing.T) { + content := []byte("example content") + desc := ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageLayer, + Digest: digest.FromBytes(content), + Size: int64(len(content))} + + // matched content and descriptor + r := bytes.NewReader(content) + vr := NewVerifyReader(r, desc) + buf := make([]byte, 5) + n, err := vr.Read(buf) + + if !bytes.Equal(buf, []byte("examp")) { + t.Fatalf("incorrect read content: %s", buf) + } + if n != 5 { + t.Fatalf("incorrect number of bytes read: %d", n) + } + if err != nil { + t.Fatal("Read() error = ", err) + } + + // mismatched content and descriptor + r = bytes.NewReader([]byte("foo")) + vr = NewVerifyReader(r, desc) + buf = make([]byte, 5) + n, err = vr.Read(buf) + if n != 3 { + t.Fatalf("incorrect number of bytes read: %d", n) + } + if err != nil { + t.Fatal("Read() error = ", err) + } +} + +func TestVerifyReader_Verify(t *testing.T) { + content := []byte("example content") + desc := ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageLayer, + Digest: digest.FromBytes(content), + Size: int64(len(content))} + + // matched content and descriptor + r := bytes.NewReader(content) + vr := NewVerifyReader(r, desc) + buf := make([]byte, len(content)) + if _, err := vr.Read(buf); err != nil { + t.Fatal("Read() error = ", err) + } + if err := vr.Verify(); err != nil { + t.Fatal("Verify() error = ", err) + } + + // mismatched content and descriptor, read size larger than descriptor size + content = []byte("foo") + r = bytes.NewReader(content) + desc = ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageLayer, + Digest: digest.FromBytes(content), + Size: int64(len(content)) - 1} + vr = NewVerifyReader(r, desc) + buf = make([]byte, 5) + if _, err := vr.Read(buf); err != nil { + t.Fatal("Read() error = ", err) + } + if err := vr.Verify(); !errors.Is(err, ErrTrailingData) { + t.Fatalf("Verify() error = %v, want %v", err, ErrTrailingData) + } + + // call vr.Verify again, the result should be the same + if err := vr.Verify(); !errors.Is(err, ErrTrailingData) { + t.Fatalf("2nd Verify() error = %v, want %v", err, ErrTrailingData) + } +} + +func TestReadAll_CorrectDescriptor(t *testing.T) { content := []byte("example content") desc := ocispec.Descriptor{ MediaType: ocispec.MediaTypeImageLayer, @@ -42,7 +118,7 @@ func TestReadAllCorrectDescriptor(t *testing.T) { } } -func TestReadAllReadSizeSmallerThanDescriptorSize(t *testing.T) { +func TestReadAll_ReadSizeSmallerThanDescriptorSize(t *testing.T) { content := []byte("example content") desc := ocispec.Descriptor{ MediaType: ocispec.MediaTypeImageLayer, @@ -55,7 +131,7 @@ func TestReadAllReadSizeSmallerThanDescriptorSize(t *testing.T) { } } -func TestReadAllReadSizeLargerThanDescriptorSize(t *testing.T) { +func TestReadAll_ReadSizeLargerThanDescriptorSize(t *testing.T) { content := []byte("example content") desc := ocispec.Descriptor{ MediaType: ocispec.MediaTypeImageLayer, @@ -68,7 +144,7 @@ func TestReadAllReadSizeLargerThanDescriptorSize(t *testing.T) { } } -func TestReadAllInvalidDigest(t *testing.T) { +func TestReadAll_InvalidDigest(t *testing.T) { content := []byte("example content") desc := ocispec.Descriptor{ MediaType: ocispec.MediaTypeImageLayer, @@ -81,7 +157,7 @@ func TestReadAllInvalidDigest(t *testing.T) { } } -func TestReadAllEmptyContent(t *testing.T) { +func TestReadAll_EmptyContent(t *testing.T) { content := []byte("") desc := ocispec.Descriptor{ MediaType: ocispec.MediaTypeImageLayer, @@ -98,7 +174,7 @@ func TestReadAllEmptyContent(t *testing.T) { } } -func TestReadAllInvalidDescriptorSize(t *testing.T) { +func TestReadAll_InvalidDescriptorSize(t *testing.T) { content := []byte("example content") desc := ocispec.Descriptor{ MediaType: ocispec.MediaTypeImageLayer, From 467514007edec639c251b6fa2fe348ff9405e123 Mon Sep 17 00:00:00 2001 From: wangxiaoxuan273 Date: Mon, 19 Sep 2022 11:21:20 +0000 Subject: [PATCH 03/11] refactored copy buffer Signed-off-by: wangxiaoxuan273 --- content/reader.go | 15 +++++++++++++-- internal/ioutil/io.go | 29 ++++++----------------------- 2 files changed, 19 insertions(+), 25 deletions(-) diff --git a/content/reader.go b/content/reader.go index 390977c1..326c3f2f 100644 --- a/content/reader.go +++ b/content/reader.go @@ -22,7 +22,6 @@ import ( "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" - "oras.land/oras-go/v2/internal/ioutil" ) var ( @@ -78,7 +77,7 @@ func (vr *VerifyReader) Verify() error { return vr.err } - if err := ioutil.EnsureEOF(vr.base.R); err != nil { + if err := EnsureEOF(vr.base.R); err != nil { vr.err = ErrTrailingData return vr.err } @@ -123,3 +122,15 @@ func ReadAll(r io.Reader, desc ocispec.Descriptor) ([]byte, error) { } return buf, nil } + +// EnsureEOF ensures the read operation ends with an EOF and no +// trailing data is present. +func EnsureEOF(r io.Reader) error { + var peek [1]byte + _, err := io.ReadFull(r, peek[:]) + if err != io.EOF { + return errors.New("trailing data") + } + + return nil +} diff --git a/internal/ioutil/io.go b/internal/ioutil/io.go index 431fdc9e..b889cf67 100644 --- a/internal/ioutil/io.go +++ b/internal/ioutil/io.go @@ -16,12 +16,12 @@ limitations under the License. package ioutil import ( - "errors" "fmt" "io" "reflect" ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "oras.land/oras-go/v2/content" ) // CloserFunc is the basic Close method defined in io.Closer. @@ -37,31 +37,14 @@ func (fn CloserFunc) Close() error { // The copied content is verified against the size and the digest. func CopyBuffer(dst io.Writer, src io.Reader, buf []byte, desc ocispec.Descriptor) error { // verify while copying - verifier := desc.Digest.Verifier() - lr := io.LimitReader(src, desc.Size) - mw := io.MultiWriter(verifier, dst) - - if _, err := io.CopyBuffer(mw, lr, buf); err != nil { + vr := content.NewVerifyReader(src, desc) + if _, err := io.CopyBuffer(dst, vr, buf); err != nil { return fmt.Errorf("copy failed: %w", err) } - - if !verifier.Verified() { - return errors.New("digest verification failed") + if err := vr.Verify(); err != nil { + return err } - - return EnsureEOF(lr) -} - -// EnsureEOF ensures the read operation ends with an EOF and no -// trailing data is present. -func EnsureEOF(r io.Reader) error { - var peek [1]byte - _, err := io.ReadFull(r, peek[:]) - if err != io.EOF { - return errors.New("trailing data") - } - - return nil + return content.EnsureEOF(vr) } // nopCloserType is the type of `io.NopCloser()`. From cad49b66749f1421a42e70b0776329ef26538a6e Mon Sep 17 00:00:00 2001 From: wangxiaoxuan273 Date: Tue, 20 Sep 2022 04:24:34 +0000 Subject: [PATCH 04/11] resolved comments Signed-off-by: wangxiaoxuan273 --- content/reader.go | 11 +++++------ content/reader_test.go | 12 ++++++------ internal/ioutil/io.go | 2 +- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/content/reader.go b/content/reader.go index 326c3f2f..16f758c4 100644 --- a/content/reader.go +++ b/content/reader.go @@ -77,8 +77,8 @@ func (vr *VerifyReader) Verify() error { return vr.err } - if err := EnsureEOF(vr.base.R); err != nil { - vr.err = ErrTrailingData + if err := ensureEOF(vr.base.R); err != nil { + vr.err = err return vr.err } if !vr.verifier.Verified() { @@ -123,14 +123,13 @@ func ReadAll(r io.Reader, desc ocispec.Descriptor) ([]byte, error) { return buf, nil } -// EnsureEOF ensures the read operation ends with an EOF and no +// ensureEOF ensures the read operation ends with an EOF and no // trailing data is present. -func EnsureEOF(r io.Reader) error { +func ensureEOF(r io.Reader) error { var peek [1]byte _, err := io.ReadFull(r, peek[:]) if err != io.EOF { - return errors.New("trailing data") + return ErrTrailingData } - return nil } diff --git a/content/reader_test.go b/content/reader_test.go index d4665df5..5427bf55 100644 --- a/content/reader_test.go +++ b/content/reader_test.go @@ -39,27 +39,27 @@ func TestVerifyReader_Read(t *testing.T) { buf := make([]byte, 5) n, err := vr.Read(buf) + if err != nil { + t.Fatal("Read() error = ", err) + } if !bytes.Equal(buf, []byte("examp")) { t.Fatalf("incorrect read content: %s", buf) } if n != 5 { t.Fatalf("incorrect number of bytes read: %d", n) } - if err != nil { - t.Fatal("Read() error = ", err) - } // mismatched content and descriptor r = bytes.NewReader([]byte("foo")) vr = NewVerifyReader(r, desc) buf = make([]byte, 5) n, err = vr.Read(buf) - if n != 3 { - t.Fatalf("incorrect number of bytes read: %d", n) - } if err != nil { t.Fatal("Read() error = ", err) } + if n != 3 { + t.Fatalf("incorrect number of bytes read: %d", n) + } } func TestVerifyReader_Verify(t *testing.T) { diff --git a/internal/ioutil/io.go b/internal/ioutil/io.go index b889cf67..010e988a 100644 --- a/internal/ioutil/io.go +++ b/internal/ioutil/io.go @@ -44,7 +44,7 @@ func CopyBuffer(dst io.Writer, src io.Reader, buf []byte, desc ocispec.Descripto if err := vr.Verify(); err != nil { return err } - return content.EnsureEOF(vr) + return nil } // nopCloserType is the type of `io.NopCloser()`. From c58ea508079dbc875354c7272dd30e90947a1f6f Mon Sep 17 00:00:00 2001 From: wangxiaoxuan273 Date: Tue, 20 Sep 2022 05:09:42 +0000 Subject: [PATCH 05/11] added more unit test of reader Signed-off-by: wangxiaoxuan273 --- content/reader_test.go | 50 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 7 deletions(-) diff --git a/content/reader_test.go b/content/reader_test.go index 5427bf55..63e71df5 100644 --- a/content/reader_test.go +++ b/content/reader_test.go @@ -27,13 +27,12 @@ import ( ) func TestVerifyReader_Read(t *testing.T) { + // matched content and descriptor with small buffer content := []byte("example content") desc := ocispec.Descriptor{ MediaType: ocispec.MediaTypeImageLayer, Digest: digest.FromBytes(content), Size: int64(len(content))} - - // matched content and descriptor r := bytes.NewReader(content) vr := NewVerifyReader(r, desc) buf := make([]byte, 5) @@ -49,8 +48,25 @@ func TestVerifyReader_Read(t *testing.T) { t.Fatalf("incorrect number of bytes read: %d", n) } - // mismatched content and descriptor - r = bytes.NewReader([]byte("foo")) + // matched content and descriptor with sufficient buffer + content = []byte("foo foo") + desc = ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageLayer, + Digest: digest.FromBytes(content), + Size: int64(len(content))} + r = bytes.NewReader(content) + vr = NewVerifyReader(r, desc) + buf = make([]byte, len(content)) + n, err = vr.Read(buf) + if err != nil { + t.Fatal("Read() error = ", err) + } + if n != len(content) { + t.Fatalf("incorrect number of bytes read: %d", n) + } + + // mismatched content and descriptor with sufficient buffer + r = bytes.NewReader([]byte("bar")) vr = NewVerifyReader(r, desc) buf = make([]byte, 5) n, err = vr.Read(buf) @@ -63,13 +79,12 @@ func TestVerifyReader_Read(t *testing.T) { } func TestVerifyReader_Verify(t *testing.T) { + // matched content and descriptor content := []byte("example content") desc := ocispec.Descriptor{ MediaType: ocispec.MediaTypeImageLayer, Digest: digest.FromBytes(content), Size: int64(len(content))} - - // matched content and descriptor r := bytes.NewReader(content) vr := NewVerifyReader(r, desc) buf := make([]byte, len(content)) @@ -88,7 +103,7 @@ func TestVerifyReader_Verify(t *testing.T) { Digest: digest.FromBytes(content), Size: int64(len(content)) - 1} vr = NewVerifyReader(r, desc) - buf = make([]byte, 5) + buf = make([]byte, len(content)) if _, err := vr.Read(buf); err != nil { t.Fatal("Read() error = ", err) } @@ -100,6 +115,27 @@ func TestVerifyReader_Verify(t *testing.T) { if err := vr.Verify(); !errors.Is(err, ErrTrailingData) { t.Fatalf("2nd Verify() error = %v, want %v", err, ErrTrailingData) } + + // mismatched content and descriptor, wrong digest + content = []byte("bar") + r = bytes.NewReader(content) + desc = ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageLayer, + Digest: digest.FromBytes([]byte("foo")), + Size: int64(len(content))} + vr = NewVerifyReader(r, desc) + buf = make([]byte, len(content)) + if _, err := vr.Read(buf); err != nil { + t.Fatal("Read() error = ", err) + } + if err := vr.Verify(); !errors.Is(err, ErrMismatchedDigest) { + t.Fatalf("Verify() error = %v, want %v", err, ErrMismatchedDigest) + } + + // call vr.Verify again, the result should be the same + if err := vr.Verify(); !errors.Is(err, ErrMismatchedDigest) { + t.Fatalf("2nd Verify() error = %v, want %v", err, ErrMismatchedDigest) + } } func TestReadAll_CorrectDescriptor(t *testing.T) { From 4002cb2f684ad55f4f62a7140904373ae917fc43 Mon Sep 17 00:00:00 2001 From: wangxiaoxuan273 Date: Tue, 20 Sep 2022 05:29:58 +0000 Subject: [PATCH 06/11] resolved comments Signed-off-by: wangxiaoxuan273 --- content/example_test.go | 49 +++++++++++++++++++++++++++++++++++++++++ internal/ioutil/io.go | 5 +---- 2 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 content/example_test.go diff --git a/content/example_test.go b/content/example_test.go new file mode 100644 index 00000000..82d02abe --- /dev/null +++ b/content/example_test.go @@ -0,0 +1,49 @@ +/* +Copyright The ORAS 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 content_test + +import ( + "bytes" + "fmt" + + "github.com/opencontainers/go-digest" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "oras.land/oras-go/v2/content" +) + +// ExampleVerifyReader gives an example of creating and using VerifyReader. +func ExampleVerifyReader() { + blob := []byte("hello world") + desc := ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageLayer, + Digest: digest.FromBytes(blob), + Size: int64(len(blob))} + r := bytes.NewReader(blob) + vr := content.NewVerifyReader(r, desc) + buf := make([]byte, len(blob)) + + if _, err := vr.Read(buf); err != nil { + panic(err) + } + if err := vr.Verify(); err != nil { + panic(err) + } + + fmt.Println(string(buf)) + + // Output: + // hello world +} diff --git a/internal/ioutil/io.go b/internal/ioutil/io.go index 010e988a..888d26f8 100644 --- a/internal/ioutil/io.go +++ b/internal/ioutil/io.go @@ -41,10 +41,7 @@ func CopyBuffer(dst io.Writer, src io.Reader, buf []byte, desc ocispec.Descripto if _, err := io.CopyBuffer(dst, vr, buf); err != nil { return fmt.Errorf("copy failed: %w", err) } - if err := vr.Verify(); err != nil { - return err - } - return nil + return vr.Verify() } // nopCloserType is the type of `io.NopCloser()`. From d4494440fb2d73a14d562ec7d1c8c457fee5befc Mon Sep 17 00:00:00 2001 From: wangxiaoxuan273 Date: Tue, 20 Sep 2022 06:56:08 +0000 Subject: [PATCH 07/11] added unit tests for CopyBuffer Signed-off-by: wangxiaoxuan273 --- internal/ioutil/io_test.go | 63 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/internal/ioutil/io_test.go b/internal/ioutil/io_test.go index fe69d6cb..392e0955 100644 --- a/internal/ioutil/io_test.go +++ b/internal/ioutil/io_test.go @@ -16,10 +16,14 @@ limitations under the License. package ioutil import ( + "bytes" "io" "os" "reflect" "testing" + + ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "oras.land/oras-go/v2/content" ) func TestUnwrapNopCloser(t *testing.T) { @@ -50,3 +54,62 @@ func TestUnwrapNopCloser(t *testing.T) { }) } } + +func TestCopyBuffer(t *testing.T) { + type args struct { + src io.Reader + buf []byte + desc ocispec.Descriptor + } + tests := []struct { + name string + args args + wantDst string + wantErr error + }{ + { + name: "exact buffer size, no errors", + args: args{bytes.NewReader([]byte("foo")), make([]byte, 3), content.NewDescriptorFromBytes("test", []byte("foo"))}, + wantDst: "foo", + wantErr: nil, + }, + { + name: "small buffer size, no errors", + args: args{bytes.NewReader([]byte("foo")), make([]byte, 1), content.NewDescriptorFromBytes("test", []byte("foo"))}, + wantDst: "foo", + wantErr: nil, + }, + { + name: "big buffer size, no errors", + args: args{bytes.NewReader([]byte("foo")), make([]byte, 5), content.NewDescriptorFromBytes("test", []byte("foo"))}, + wantDst: "foo", + wantErr: nil, + }, + { + name: "wrong digest", + args: args{bytes.NewReader([]byte("foo")), make([]byte, 3), content.NewDescriptorFromBytes("test", []byte("bar"))}, + wantDst: "foo", + wantErr: content.ErrMismatchedDigest, + }, + { + name: "wrong size", + args: args{bytes.NewReader([]byte("foo")), make([]byte, 3), content.NewDescriptorFromBytes("test", []byte("fo"))}, + wantDst: "foo", + wantErr: content.ErrTrailingData, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dst := &bytes.Buffer{} + err := CopyBuffer(dst, tt.args.src, tt.args.buf, tt.args.desc) + if err != tt.wantErr { + t.Errorf("CopyBuffer() error = %v, wantErr %v", err, tt.wantErr) + return + } + gotDst := dst.String() + if err == nil && gotDst != tt.wantDst { + t.Errorf("CopyBuffer() = %v, want %v", gotDst, tt.wantDst) + } + }) + } +} From 9c5f17d429df6dd7d72ae57c19962ef8a00b3a27 Mon Sep 17 00:00:00 2001 From: wangxiaoxuan273 Date: Tue, 20 Sep 2022 07:54:41 +0000 Subject: [PATCH 08/11] used new descriptor from byte Signed-off-by: wangxiaoxuan273 --- content/reader_test.go | 36 +++++++----------------------------- 1 file changed, 7 insertions(+), 29 deletions(-) diff --git a/content/reader_test.go b/content/reader_test.go index 63e71df5..b1fbbdc5 100644 --- a/content/reader_test.go +++ b/content/reader_test.go @@ -29,10 +29,7 @@ import ( func TestVerifyReader_Read(t *testing.T) { // matched content and descriptor with small buffer content := []byte("example content") - desc := ocispec.Descriptor{ - MediaType: ocispec.MediaTypeImageLayer, - Digest: digest.FromBytes(content), - Size: int64(len(content))} + desc := NewDescriptorFromBytes("test", content) r := bytes.NewReader(content) vr := NewVerifyReader(r, desc) buf := make([]byte, 5) @@ -50,10 +47,7 @@ func TestVerifyReader_Read(t *testing.T) { // matched content and descriptor with sufficient buffer content = []byte("foo foo") - desc = ocispec.Descriptor{ - MediaType: ocispec.MediaTypeImageLayer, - Digest: digest.FromBytes(content), - Size: int64(len(content))} + desc = NewDescriptorFromBytes("test", content) r = bytes.NewReader(content) vr = NewVerifyReader(r, desc) buf = make([]byte, len(content)) @@ -81,10 +75,7 @@ func TestVerifyReader_Read(t *testing.T) { func TestVerifyReader_Verify(t *testing.T) { // matched content and descriptor content := []byte("example content") - desc := ocispec.Descriptor{ - MediaType: ocispec.MediaTypeImageLayer, - Digest: digest.FromBytes(content), - Size: int64(len(content))} + desc := NewDescriptorFromBytes("test", content) r := bytes.NewReader(content) vr := NewVerifyReader(r, desc) buf := make([]byte, len(content)) @@ -119,10 +110,7 @@ func TestVerifyReader_Verify(t *testing.T) { // mismatched content and descriptor, wrong digest content = []byte("bar") r = bytes.NewReader(content) - desc = ocispec.Descriptor{ - MediaType: ocispec.MediaTypeImageLayer, - Digest: digest.FromBytes([]byte("foo")), - Size: int64(len(content))} + desc = NewDescriptorFromBytes("test", []byte("foo")) vr = NewVerifyReader(r, desc) buf = make([]byte, len(content)) if _, err := vr.Read(buf); err != nil { @@ -140,10 +128,7 @@ func TestVerifyReader_Verify(t *testing.T) { func TestReadAll_CorrectDescriptor(t *testing.T) { content := []byte("example content") - desc := ocispec.Descriptor{ - MediaType: ocispec.MediaTypeImageLayer, - Digest: digest.FromBytes(content), - Size: int64(len(content))} + desc := NewDescriptorFromBytes("test", content) r := bytes.NewReader([]byte(content)) got, err := ReadAll(r, desc) if err != nil { @@ -182,10 +167,7 @@ func TestReadAll_ReadSizeLargerThanDescriptorSize(t *testing.T) { func TestReadAll_InvalidDigest(t *testing.T) { content := []byte("example content") - desc := ocispec.Descriptor{ - MediaType: ocispec.MediaTypeImageLayer, - Digest: digest.FromBytes([]byte("wrong content")), - Size: int64(len(content))} + desc := NewDescriptorFromBytes("test", []byte("another content")) r := bytes.NewReader([]byte(content)) _, err := ReadAll(r, desc) if err == nil || !errors.Is(err, ErrMismatchedDigest) { @@ -195,11 +177,7 @@ func TestReadAll_InvalidDigest(t *testing.T) { func TestReadAll_EmptyContent(t *testing.T) { content := []byte("") - desc := ocispec.Descriptor{ - MediaType: ocispec.MediaTypeImageLayer, - Digest: digest.FromBytes(content), - Size: int64(len(content)), - } + desc := NewDescriptorFromBytes("test", content) r := bytes.NewReader([]byte(content)) got, err := ReadAll(r, desc) if err != nil { From 2e8aa0c1aaf4ab070d8aa0162df71fc44b28285f Mon Sep 17 00:00:00 2001 From: wangxiaoxuan273 Date: Tue, 20 Sep 2022 08:04:40 +0000 Subject: [PATCH 09/11] resolved the easy comments Signed-off-by: wangxiaoxuan273 --- content/example_test.go | 8 +++----- content/reader_test.go | 1 - 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/content/example_test.go b/content/example_test.go index 82d02abe..8b1a1467 100644 --- a/content/example_test.go +++ b/content/example_test.go @@ -19,7 +19,6 @@ import ( "bytes" "fmt" - "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "oras.land/oras-go/v2/content" ) @@ -27,10 +26,7 @@ import ( // ExampleVerifyReader gives an example of creating and using VerifyReader. func ExampleVerifyReader() { blob := []byte("hello world") - desc := ocispec.Descriptor{ - MediaType: ocispec.MediaTypeImageLayer, - Digest: digest.FromBytes(blob), - Size: int64(len(blob))} + desc := content.NewDescriptorFromBytes(ocispec.MediaTypeImageLayer, blob) r := bytes.NewReader(blob) vr := content.NewVerifyReader(r, desc) buf := make([]byte, len(blob)) @@ -42,6 +38,8 @@ func ExampleVerifyReader() { panic(err) } + // please note: users should not trust the the read content until + // Verify() returns nil. fmt.Println(string(buf)) // Output: diff --git a/content/reader_test.go b/content/reader_test.go index b1fbbdc5..bde30203 100644 --- a/content/reader_test.go +++ b/content/reader_test.go @@ -34,7 +34,6 @@ func TestVerifyReader_Read(t *testing.T) { vr := NewVerifyReader(r, desc) buf := make([]byte, 5) n, err := vr.Read(buf) - if err != nil { t.Fatal("Read() error = ", err) } From e3644b1e0db85281b928934cfa097cf520b0a7fe Mon Sep 17 00:00:00 2001 From: wangxiaoxuan273 Date: Tue, 20 Sep 2022 08:24:37 +0000 Subject: [PATCH 10/11] resolved more easy comments Signed-off-by: wangxiaoxuan273 --- content/reader_test.go | 21 +++++++++++++++++++++ internal/ioutil/io_test.go | 11 ++++++----- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/content/reader_test.go b/content/reader_test.go index bde30203..14ec4b26 100644 --- a/content/reader_test.go +++ b/content/reader_test.go @@ -106,6 +106,27 @@ func TestVerifyReader_Verify(t *testing.T) { t.Fatalf("2nd Verify() error = %v, want %v", err, ErrTrailingData) } + // // mismatched content and descriptor, read size smaller than descriptor size + // content = []byte("foo") + // r = bytes.NewReader(content) + // desc = ocispec.Descriptor{ + // MediaType: ocispec.MediaTypeImageLayer, + // Digest: digest.FromBytes(content), + // Size: int64(len(content)) + 1} + // vr = NewVerifyReader(r, desc) + // buf = make([]byte, len(content)) + // if _, err := vr.Read(buf); err != nil { + // t.Fatal("Read() error = ", err) + // } + // if err := vr.Verify(); !errors.Is(err, ErrTrailingData) { + // t.Fatalf("Verify() error = %v, want %v", err, ErrTrailingData) + // } + + // call vr.Verify again, the result should be the same + if err := vr.Verify(); !errors.Is(err, ErrTrailingData) { + t.Fatalf("2nd Verify() error = %v, want %v", err, ErrTrailingData) + } + // mismatched content and descriptor, wrong digest content = []byte("bar") r = bytes.NewReader(content) diff --git a/internal/ioutil/io_test.go b/internal/ioutil/io_test.go index 392e0955..8aeeffe9 100644 --- a/internal/ioutil/io_test.go +++ b/internal/ioutil/io_test.go @@ -56,6 +56,7 @@ func TestUnwrapNopCloser(t *testing.T) { } func TestCopyBuffer(t *testing.T) { + blob := []byte("foo") type args struct { src io.Reader buf []byte @@ -69,31 +70,31 @@ func TestCopyBuffer(t *testing.T) { }{ { name: "exact buffer size, no errors", - args: args{bytes.NewReader([]byte("foo")), make([]byte, 3), content.NewDescriptorFromBytes("test", []byte("foo"))}, + args: args{bytes.NewReader(blob), make([]byte, 3), content.NewDescriptorFromBytes("test", blob)}, wantDst: "foo", wantErr: nil, }, { name: "small buffer size, no errors", - args: args{bytes.NewReader([]byte("foo")), make([]byte, 1), content.NewDescriptorFromBytes("test", []byte("foo"))}, + args: args{bytes.NewReader(blob), make([]byte, 1), content.NewDescriptorFromBytes("test", blob)}, wantDst: "foo", wantErr: nil, }, { name: "big buffer size, no errors", - args: args{bytes.NewReader([]byte("foo")), make([]byte, 5), content.NewDescriptorFromBytes("test", []byte("foo"))}, + args: args{bytes.NewReader(blob), make([]byte, 5), content.NewDescriptorFromBytes("test", blob)}, wantDst: "foo", wantErr: nil, }, { name: "wrong digest", - args: args{bytes.NewReader([]byte("foo")), make([]byte, 3), content.NewDescriptorFromBytes("test", []byte("bar"))}, + args: args{bytes.NewReader(blob), make([]byte, 3), content.NewDescriptorFromBytes("test", []byte("bar"))}, wantDst: "foo", wantErr: content.ErrMismatchedDigest, }, { name: "wrong size", - args: args{bytes.NewReader([]byte("foo")), make([]byte, 3), content.NewDescriptorFromBytes("test", []byte("fo"))}, + args: args{bytes.NewReader(blob), make([]byte, 3), content.NewDescriptorFromBytes("test", []byte("fo"))}, wantDst: "foo", wantErr: content.ErrTrailingData, }, From b6185a01a719091b001b1ac68ef202251bd38045 Mon Sep 17 00:00:00 2001 From: wangxiaoxuan273 Date: Tue, 20 Sep 2022 09:27:10 +0000 Subject: [PATCH 11/11] resolved those final, hard comments Signed-off-by: wangxiaoxuan273 --- content/example_test.go | 13 ++++++------ content/reader.go | 10 +++++++-- content/reader_test.go | 43 ++++++++++++++++++++------------------ internal/ioutil/io_test.go | 11 ++++++++-- 4 files changed, 47 insertions(+), 30 deletions(-) diff --git a/content/example_test.go b/content/example_test.go index 8b1a1467..e8d5f580 100644 --- a/content/example_test.go +++ b/content/example_test.go @@ -18,6 +18,7 @@ package content_test import ( "bytes" "fmt" + "io" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "oras.land/oras-go/v2/content" @@ -29,18 +30,18 @@ func ExampleVerifyReader() { desc := content.NewDescriptorFromBytes(ocispec.MediaTypeImageLayer, blob) r := bytes.NewReader(blob) vr := content.NewVerifyReader(r, desc) - buf := make([]byte, len(blob)) - - if _, err := vr.Read(buf); err != nil { + buf := bytes.NewBuffer(nil) + if _, err := io.Copy(buf, vr); err != nil { panic(err) } + + // note: users should not trust the the read content until + // Verify() returns nil. if err := vr.Verify(); err != nil { panic(err) } - // please note: users should not trust the the read content until - // Verify() returns nil. - fmt.Println(string(buf)) + fmt.Println(buf) // Output: // hello world diff --git a/content/reader.go b/content/reader.go index 16f758c4..11d27b23 100644 --- a/content/reader.go +++ b/content/reader.go @@ -38,6 +38,12 @@ var ( ErrTrailingData = errors.New("trailing data") ) +var ( + // errEarlyVerify is returned by VerifyReader.Verify() when + // Verify() is called before completing reading the entire content blob. + errEarlyVerify = errors.New("early verify") +) + // VerifyReader reads the content described by its descriptor and verifies // against its size and digest. type VerifyReader struct { @@ -71,7 +77,7 @@ func (vr *VerifyReader) Verify() error { } if vr.err == nil { if vr.base.N > 0 { - return errors.New("early verify") + return errEarlyVerify } } else if vr.err != io.EOF { return vr.err @@ -91,7 +97,7 @@ func (vr *VerifyReader) Verify() error { return nil } -// NewVerifyReader returns a pointer to a new VerifyReader. +// NewVerifyReader wraps r for reading content with verification against desc. func NewVerifyReader(r io.Reader, desc ocispec.Descriptor) *VerifyReader { verifier := desc.Digest.Verifier() lr := &io.LimitedReader{ diff --git a/content/reader_test.go b/content/reader_test.go index 14ec4b26..138c139e 100644 --- a/content/reader_test.go +++ b/content/reader_test.go @@ -57,6 +57,9 @@ func TestVerifyReader_Read(t *testing.T) { if n != len(content) { t.Fatalf("incorrect number of bytes read: %d", n) } + if !bytes.Equal(buf, content) { + t.Fatalf("incorrect read content: %s", buf) + } // mismatched content and descriptor with sufficient buffer r = bytes.NewReader([]byte("bar")) @@ -84,6 +87,9 @@ func TestVerifyReader_Verify(t *testing.T) { if err := vr.Verify(); err != nil { t.Fatal("Verify() error = ", err) } + if !bytes.Equal(buf, content) { + t.Fatalf("incorrect read content: %s", buf) + } // mismatched content and descriptor, read size larger than descriptor size content = []byte("foo") @@ -100,31 +106,29 @@ func TestVerifyReader_Verify(t *testing.T) { if err := vr.Verify(); !errors.Is(err, ErrTrailingData) { t.Fatalf("Verify() error = %v, want %v", err, ErrTrailingData) } - // call vr.Verify again, the result should be the same if err := vr.Verify(); !errors.Is(err, ErrTrailingData) { t.Fatalf("2nd Verify() error = %v, want %v", err, ErrTrailingData) } - // // mismatched content and descriptor, read size smaller than descriptor size - // content = []byte("foo") - // r = bytes.NewReader(content) - // desc = ocispec.Descriptor{ - // MediaType: ocispec.MediaTypeImageLayer, - // Digest: digest.FromBytes(content), - // Size: int64(len(content)) + 1} - // vr = NewVerifyReader(r, desc) - // buf = make([]byte, len(content)) - // if _, err := vr.Read(buf); err != nil { - // t.Fatal("Read() error = ", err) - // } - // if err := vr.Verify(); !errors.Is(err, ErrTrailingData) { - // t.Fatalf("Verify() error = %v, want %v", err, ErrTrailingData) - // } - + // mismatched content and descriptor, read size smaller than descriptor size + content = []byte("foo") + r = bytes.NewReader(content) + desc = ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageLayer, + Digest: digest.FromBytes(content), + Size: int64(len(content)) + 1} + vr = NewVerifyReader(r, desc) + buf = make([]byte, len(content)) + if _, err := vr.Read(buf); err != nil { + t.Fatal("Read() error = ", err) + } + if err := vr.Verify(); !errors.Is(err, errEarlyVerify) { + t.Fatalf("Verify() error = %v, want %v", err, errEarlyVerify) + } // call vr.Verify again, the result should be the same - if err := vr.Verify(); !errors.Is(err, ErrTrailingData) { - t.Fatalf("2nd Verify() error = %v, want %v", err, ErrTrailingData) + if err := vr.Verify(); !errors.Is(err, errEarlyVerify) { + t.Fatalf("Verify() error = %v, want %v", err, errEarlyVerify) } // mismatched content and descriptor, wrong digest @@ -139,7 +143,6 @@ func TestVerifyReader_Verify(t *testing.T) { if err := vr.Verify(); !errors.Is(err, ErrMismatchedDigest) { t.Fatalf("Verify() error = %v, want %v", err, ErrMismatchedDigest) } - // call vr.Verify again, the result should be the same if err := vr.Verify(); !errors.Is(err, ErrMismatchedDigest) { t.Fatalf("2nd Verify() error = %v, want %v", err, ErrMismatchedDigest) diff --git a/internal/ioutil/io_test.go b/internal/ioutil/io_test.go index 8aeeffe9..16a9873e 100644 --- a/internal/ioutil/io_test.go +++ b/internal/ioutil/io_test.go @@ -17,6 +17,7 @@ package ioutil import ( "bytes" + "errors" "io" "os" "reflect" @@ -93,17 +94,23 @@ func TestCopyBuffer(t *testing.T) { wantErr: content.ErrMismatchedDigest, }, { - name: "wrong size", + name: "wrong size, descriptor size is smaller", args: args{bytes.NewReader(blob), make([]byte, 3), content.NewDescriptorFromBytes("test", []byte("fo"))}, wantDst: "foo", wantErr: content.ErrTrailingData, }, + { + name: "wrong size, descriptor size is larger", + args: args{bytes.NewReader(blob), make([]byte, 3), content.NewDescriptorFromBytes("test", []byte("fooo"))}, + wantDst: "foo", + wantErr: io.ErrUnexpectedEOF, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { dst := &bytes.Buffer{} err := CopyBuffer(dst, tt.args.src, tt.args.buf, tt.args.desc) - if err != tt.wantErr { + if !errors.Is(err, tt.wantErr) { t.Errorf("CopyBuffer() error = %v, wantErr %v", err, tt.wantErr) return }