From 4e7bc74874564852990663e39773183c07b5ca22 Mon Sep 17 00:00:00 2001 From: Rick Olson Date: Fri, 25 Sep 2015 15:39:02 -0600 Subject: [PATCH] Better handling for tiny files below the blob size threshold that contain 'git-lfs' --- lfs/errors.go | 32 ++++++++++++++++++++++++++++++++ lfs/pointer.go | 8 +++++++- lfs/pointer_test.go | 13 +++++++++++++ test/test-pull.sh | 3 --- test/test-smudge.sh | 1 + 5 files changed, 53 insertions(+), 4 deletions(-) diff --git a/lfs/errors.go b/lfs/errors.go index dd89a6cf..86e6a203 100644 --- a/lfs/errors.go +++ b/lfs/errors.go @@ -162,6 +162,19 @@ func IsNotAPointerError(err error) bool { return false } +// IsBadPointerKeyError indicates that the parsed data has an invalid key. +func IsBadPointerKeyError(err error) bool { + if e, ok := err.(interface { + BadPointerKeyError() bool + }); ok { + return e.BadPointerKeyError() + } + if e, ok := err.(errorWrapper); ok { + return IsBadPointerKeyError(e.InnerError()) + } + return false +} + // IsDownloadDeclinedError indicates that the smudge operation should not download. // TODO: I don't really like using errors to control that flow, it should be refactored. func IsDownloadDeclinedError(err error) bool { @@ -483,6 +496,25 @@ func newNotAPointerError(err error) error { return notAPointerError{newWrappedError(err, "Not a valid Git LFS pointer file.")} } +type badPointerKeyError struct { + Expected string + Actual string + errorWrapper +} + +func (e badPointerKeyError) InnerError() error { + return e.errorWrapper +} + +func (e badPointerKeyError) BadPointerKeyError() bool { + return true +} + +func newBadPointerKeyError(expected, actual string) error { + err := fmt.Errorf("Error parsing LFS Pointer. Expected key %s, got %s", expected, actual) + return badPointerKeyError{expected, actual, newWrappedError(err, "")} +} + // Definitions for IsDownloadDeclinedError() type downloadDeclinedError struct { diff --git a/lfs/pointer.go b/lfs/pointer.go index 1b129d68..400a9097 100644 --- a/lfs/pointer.go +++ b/lfs/pointer.go @@ -131,6 +131,12 @@ func verifyVersion(version string) error { func decodeKV(data []byte) (*Pointer, error) { kvps, exts, err := decodeKVData(data) if err != nil { + if IsBadPointerKeyError(err) { + badErr := err.(badPointerKeyError) + if badErr.Expected == "version" { + return nil, newNotAPointerError(err) + } + } return nil, err } @@ -252,7 +258,7 @@ func decodeKVData(data []byte) (kvps map[string]string, exts map[string]string, if expected := pointerKeys[line]; key != expected { if !extRE.Match([]byte(key)) { - err = fmt.Errorf("Expected key %s, got %s", expected, key) + err = newBadPointerKeyError(expected, key) return } if exts == nil { diff --git a/lfs/pointer_test.go b/lfs/pointer_test.go index 862fb0f5..7873b4b4 100644 --- a/lfs/pointer_test.go +++ b/lfs/pointer_test.go @@ -4,6 +4,7 @@ import ( "bufio" "bytes" "io" + "reflect" "strings" "testing" @@ -60,6 +61,18 @@ func assertLine(t *testing.T, r *bufio.Reader, expected string) { assert.Equal(t, expected, actual) } +func TestDecodeTinyFile(t *testing.T) { + ex := "this is not a git-lfs file!" + p, err := DecodePointer(bytes.NewBufferString(ex)) + if p != nil { + t.Errorf("pointer was decoded: %v", p) + } + + if !IsNotAPointerError(err) { + t.Errorf("error is not a NotAPointerError: %s: '%v'", reflect.TypeOf(err), err) + } +} + func TestDecode(t *testing.T) { ex := `version https://git-lfs.github.com/spec/v1 oid sha256:4d7a214614ab2935c943f9e0ff69d22eadbb8f32b1258daaa5e2ca24d17e2393 diff --git a/test/test-pull.sh b/test/test-pull.sh index 66ae31f9..af22b152 100755 --- a/test/test-pull.sh +++ b/test/test-pull.sh @@ -69,7 +69,6 @@ begin_test "pull" git lfs pull [ "a" = "$(cat a.dat)" ] - # Test include / exclude filters supplied in gitconfig rm -rf .git/lfs/objects git config "lfs.fetchinclude" "a*" @@ -91,8 +90,6 @@ begin_test "pull" rm -rf .git/lfs/objects git lfs pull --exclude="a*" refute_local_object "$contents_oid" - - ) end_test diff --git a/test/test-smudge.sh b/test/test-smudge.sh index 04ab1497..56f3e323 100755 --- a/test/test-smudge.sh +++ b/test/test-smudge.sh @@ -44,6 +44,7 @@ begin_test "smudge with invalid pointer" cd repo [ "wat" = "$(echo "wat" | git lfs smudge)" ] + [ "not a git-lfs file" = "$(echo "wat" | git lfs smudge)" ] ) end_test