Skip to content

Commit

Permalink
return ErrUnexpectedEOF when Decode input is too short
Browse files Browse the repository at this point in the history
The added test cases would fail before this fix:

    --- FAIL: TestDecodeError/Empty (0.00s)
        unmarshal_test.go:28: Decode("") got "EOF", want "unexpected EOF"
    --- FAIL: TestDecodeError/NoSpace (0.00s)
        unmarshal_test.go:28: Decode("foo") got "EOF", want "unexpected EOF"

They would still return an error, but they'd just return io.EOF.
While that error signals that we've hit the end of input,
it doesn't signal to the codec user that something went wrong.
An io.EOF is not a worrying error, in many situations.

If the input is empty, or is too short to be a valid git object,
we should return a clear error to signal the operation did not work.
io.ErrUnexpectedEOF is unambiguous,
and will make go-ipfs start erroring on this edge case.

For ipfs/kubo#8378.
  • Loading branch information
mvdan committed Sep 4, 2021
1 parent cd34211 commit c56b126
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 2 deletions.
10 changes: 8 additions & 2 deletions unmarshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ func Decode(na ipld.NodeAssembler, r io.Reader) error {
rd := bufio.NewReader(r)

typ, err := rd.ReadString(' ')
if err == io.EOF {
return io.ErrUnexpectedEOF
}
if err != nil {
return err
}
Expand All @@ -28,7 +31,7 @@ func Decode(na ipld.NodeAssembler, r io.Reader) error {
case "tag":
return DecodeTag(na, rd)
default:
return fmt.Errorf("unrecognized object type: %s", typ)
return fmt.Errorf("unrecognized object type: %q", typ)
}
}

Expand All @@ -37,6 +40,9 @@ func ParseObject(r io.Reader) (ipld.Node, error) {
rd := bufio.NewReader(r)

typ, err := rd.ReadString(' ')
if err == io.EOF {
return nil, io.ErrUnexpectedEOF
}
if err != nil {
return nil, err
}
Expand All @@ -58,7 +64,7 @@ func ParseObject(r io.Reader) (ipld.Node, error) {
na = Type.Tag.NewBuilder()
decode = DecodeTag
default:
return nil, fmt.Errorf("unrecognized object type: %s", typ)
return nil, fmt.Errorf("unrecognized object type: %q", typ)
}
// fmt.Printf("type %s\n", typ)

Expand Down
39 changes: 39 additions & 0 deletions unmarshal_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package ipldgit

import (
"fmt"
"strings"
"testing"

basicnode "github.com/ipld/go-ipld-prime/node/basic"
)

func TestUnmarshalError(t *testing.T) {
tests := []struct {
name string
input string
want string
}{
{"Empty", "", "unexpected EOF"},
{"Whitespace", " ", "unrecognized object type"},
{"NoSpace", "foo", "unexpected EOF"},
{"BadType", "foo ", "unrecognized object type"},
}
for _, test := range tests {
t.Run("Decode/"+test.name, func(t *testing.T) {
nb := basicnode.Prototype.Any.NewBuilder()
err := Decode(nb, strings.NewReader(test.input))
got := fmt.Sprint(err)
if !strings.Contains(got, test.want) {
t.Fatalf("Decode(%q) got %q, want %q", test.input, got, test.want)
}
})
t.Run("ParseObject/"+test.name, func(t *testing.T) {
_, err := ParseObject(strings.NewReader(test.input))
got := fmt.Sprint(err)
if !strings.Contains(got, test.want) {
t.Fatalf("ParseObject(%q) got %q, want %q", test.input, got, test.want)
}
})
}
}

0 comments on commit c56b126

Please sign in to comment.