Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: opt-in way to allow empty list of roots in CAR headers #461

Merged
merged 4 commits into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 41 additions & 7 deletions car.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,11 +124,36 @@
}

type CarReader struct {
br *bufio.Reader // set nil on EOF
Header *CarHeader
br *bufio.Reader // set nil on EOF
Header *CarHeader
errorOnEmptyRoots bool
}

type CarReaderOption func(*CarReader) error

// WithErrorOnEmptyRoots is an option that can be passed to NewCarReader to
// specify the behavior when reading a car file that does not have any root
// cids set in the CAR header.
// Setting this option to true will cause CarReader to error on CAR that has
// no root CIDs listed in the header.
func WithErrorOnEmptyRoots(flag bool) CarReaderOption {
return func(cr *CarReader) error {
cr.errorOnEmptyRoots = flag
return nil
}
}

func NewCarReader(r io.Reader) (*CarReader, error) {
// BACKWARD COMPATIBILITY NOTE:
// WithErrorOnEmptyRoots(true) here is the legacy behavior
// which we need to keep for reasons described in
// https://github.com/ipfs/specs/pull/402#issuecomment-1599428849
// (mainly, we need to migrate Filecoin code to use explicit
// WithErrorOnEmptyRoots(true) before we can change the default here).
return NewCarReaderWithOptions(r, WithErrorOnEmptyRoots(true))
}

func NewCarReaderWithOptions(r io.Reader, opts ...CarReaderOption) (*CarReader, error) {
br := bufioReaderPool.Get().(*bufio.Reader)
br.Reset(r)
ch, err := ReadHeader(br)
Expand All @@ -141,14 +166,23 @@
return nil, fmt.Errorf("invalid car version: %d", ch.Version)
}

if len(ch.Roots) == 0 {
carReader := &CarReader{
br: br,
Header: ch,
}

for _, o := range opts {
err := o(carReader)
if err != nil {
return nil, err
}

Check warning on line 178 in car.go

View check run for this annotation

Codecov / codecov/patch

car.go#L177-L178

Added lines #L177 - L178 were not covered by tests
}

if carReader.errorOnEmptyRoots && len(ch.Roots) == 0 {
return nil, fmt.Errorf("empty car, no roots")
}

return &CarReader{
br: br,
Header: ch,
}, nil
return carReader, nil
}

func (cr *CarReader) Next() (blocks.Block, error) {
Expand Down
46 changes: 46 additions & 0 deletions car_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,52 @@ func TestEOFHandling(t *testing.T) {
}
})
}
func TestEmptyRootsInHeader(t *testing.T) {
testCases := []struct {
name string
errorOnEmptyRoots bool
hex string
errStr string // either the whole error string
}{
{
"errorOnEmptyRoots:true",
true,
"0aa16776657273696f6e01",
"empty car, no roots",
}, {
"errorOnEmptyRoots:false",
false,
"0aa16776657273696f6e01",
"",
},
}

makeCar := func(t *testing.T, byts string, flag bool) error {
fixture, err := hex.DecodeString(byts)
if err != nil {
t.Fatal(err)
}
_, err = car.NewCarReaderWithOptions(bytes.NewReader(fixture), car.WithErrorOnEmptyRoots(flag))
return err
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
err := makeCar(t, tc.hex, tc.errorOnEmptyRoots)
if err == nil && tc.errorOnEmptyRoots {
t.Fatal("expected error from empty roots, didn't get one")
}
if err != nil && !tc.errorOnEmptyRoots {
t.Fatal("expected no error from empty roots, but got one")
}
if tc.errStr != "" {
if err.Error() != tc.errStr {
t.Fatalf("bad error: %v", err)
}
}
})
}
}

func TestBadHeaders(t *testing.T) {
testCases := []struct {
Expand Down
Loading