From e97df9c8102c92701489cb2693294064f1488b4b Mon Sep 17 00:00:00 2001 From: Alec Lanter Date: Sat, 21 Jan 2023 16:10:02 -0600 Subject: fix(#971): parse submodule .git files instead of erroring Altered logic for detecting git directory. Instead of erroring on non-direcctory .git files, now parses the file and returns the linked gitdir. --- repository/gogit.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/repository/gogit.go b/repository/gogit.go index 93806026..18f7fb59 100644 --- a/repository/gogit.go +++ b/repository/gogit.go @@ -1,6 +1,7 @@ package repository import ( + "bufio" "bytes" "errors" "fmt" @@ -170,6 +171,28 @@ func detectGitPath(path string) (string, error) { fi, err := os.Stat(filepath.Join(path, ".git")) if err == nil { if !fi.IsDir() { + // See if our .git item is a dotfile that holds a submodule reference + dotfile, err := os.Open(fi.Name()) + if err != nil { + // Can't open" error + return "", fmt.Errorf(".git exists but is not a directory or a readable file: %w", err) + } + // We aren't going to defer the dotfile.Close, because we might keep looping, so we have to be sure to + // clean up before returning an error + reader := bufio.NewReader(dotfile) + line, _, err := reader.ReadLine() + if err != nil { + _ = dotfile.Close() + return "", fmt.Errorf(".git exists but is not a direcctory and cannot be read: %w", err) + } + dotContent := string(line) + if strings.HasPrefix(dotContent, "gitdir:") { + _ = dotfile.Close() + // This is a submodule parent path link. Strip the prefix, clean the string of whitespace just to + // be safe, and return + dotContent = strings.TrimSpace(dotContent[7:]) + return dotContent, nil + } return "", fmt.Errorf(".git exist but is not a directory") } return filepath.Join(path, ".git"), nil -- cgit From 70f405e7b15148ba0c3de89355b7dd5cb3befa60 Mon Sep 17 00:00:00 2001 From: Alec Lanter Date: Mon, 23 Jan 2023 10:10:11 -0600 Subject: test: resolve changes for PR #1004, add unit test, fix issue uncovered by unit test --- repository/gogit.go | 9 ++++----- repository/gogit_test.go | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/repository/gogit.go b/repository/gogit.go index 18f7fb59..649614fb 100644 --- a/repository/gogit.go +++ b/repository/gogit.go @@ -172,25 +172,24 @@ func detectGitPath(path string) (string, error) { if err == nil { if !fi.IsDir() { // See if our .git item is a dotfile that holds a submodule reference - dotfile, err := os.Open(fi.Name()) + dotfile, err := os.Open(filepath.Join(path, fi.Name())) if err != nil { - // Can't open" error + // Can't open error return "", fmt.Errorf(".git exists but is not a directory or a readable file: %w", err) } // We aren't going to defer the dotfile.Close, because we might keep looping, so we have to be sure to // clean up before returning an error reader := bufio.NewReader(dotfile) line, _, err := reader.ReadLine() + _ = dotfile.Close() if err != nil { - _ = dotfile.Close() return "", fmt.Errorf(".git exists but is not a direcctory and cannot be read: %w", err) } dotContent := string(line) if strings.HasPrefix(dotContent, "gitdir:") { - _ = dotfile.Close() // This is a submodule parent path link. Strip the prefix, clean the string of whitespace just to // be safe, and return - dotContent = strings.TrimSpace(dotContent[7:]) + dotContent = strings.TrimSpace(strings.TrimPrefix(dotContent, "gitdir: ")) return dotContent, nil } return "", fmt.Errorf(".git exist but is not a directory") diff --git a/repository/gogit_test.go b/repository/gogit_test.go index 02bd42fd..2bec97a5 100644 --- a/repository/gogit_test.go +++ b/repository/gogit_test.go @@ -1,6 +1,7 @@ package repository import ( + "os" "path" "path/filepath" "testing" @@ -81,3 +82,21 @@ func TestGoGitRepo_Indexes(t *testing.T) { require.NoError(t, err) require.NotZero(t, indexA) } + +func TestGoGit_DetectsSubmodules(t *testing.T) { + expectedPath := "../foo/bar" + submoduleData := "gitdir: " + expectedPath + d := t.TempDir() + if f, err := os.Create(filepath.Join(d, ".git")); err != nil { + t.Fatal("could not create necessary temp file:", err) + } else { + t.Log(f.Name()) + if _, err := f.Write([]byte(submoduleData)); err != nil { + t.Fatal("could not write necessary data to temp file:", err) + } + _ = f.Close() + } + result, err := detectGitPath(d) + assert.Empty(t, err) + assert.Equal(t, expectedPath, result) +} -- cgit From d9ac658392cc09d42928f68a60b0e337f7dc7d04 Mon Sep 17 00:00:00 2001 From: Alec Lanter Date: Mon, 23 Jan 2023 10:12:46 -0600 Subject: chore: updated error message when detectGitPath fails --- repository/gogit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/repository/gogit.go b/repository/gogit.go index 649614fb..01eace6d 100644 --- a/repository/gogit.go +++ b/repository/gogit.go @@ -192,7 +192,7 @@ func detectGitPath(path string) (string, error) { dotContent = strings.TrimSpace(strings.TrimPrefix(dotContent, "gitdir: ")) return dotContent, nil } - return "", fmt.Errorf(".git exist but is not a directory") + return "", fmt.Errorf(".git exist but is not a directory or module/workspace file") } return filepath.Join(path, ".git"), nil } -- cgit From 27c96a4044f05a338d6ac6187135e6b9ac487e9f Mon Sep 17 00:00:00 2001 From: Michael Muré Date: Sat, 4 Mar 2023 13:10:38 +0100 Subject: repo: improve support for gitdir indirection - add a limited reader to avoid abuse - support recusive indirection (up to depth 10) - check that the pointed to repo does exist --- repository/gogit.go | 19 ++++++++++++++----- repository/gogit_test.go | 22 +++++++++------------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/repository/gogit.go b/repository/gogit.go index 01eace6d..96d62665 100644 --- a/repository/gogit.go +++ b/repository/gogit.go @@ -5,6 +5,7 @@ import ( "bytes" "errors" "fmt" + "io" "io/ioutil" "os" "path/filepath" @@ -56,7 +57,7 @@ type GoGitRepo struct { // of "~/myrepo" and a namespace of "git-bug", local storage for the // GoGitRepo will be configured at "~/myrepo/.git/git-bug". func OpenGoGitRepo(path, namespace string, clockLoaders []ClockLoader) (*GoGitRepo, error) { - path, err := detectGitPath(path) + path, err := detectGitPath(path, 0) if err != nil { return nil, err } @@ -160,7 +161,11 @@ func InitBareGoGitRepo(path, namespace string) (*GoGitRepo, error) { }, nil } -func detectGitPath(path string) (string, error) { +func detectGitPath(path string, depth int) (string, error) { + if depth >= 10 { + return "", fmt.Errorf("gitdir loop detected") + } + // normalize the path path, err := filepath.Abs(path) if err != nil { @@ -179,18 +184,22 @@ func detectGitPath(path string) (string, error) { } // We aren't going to defer the dotfile.Close, because we might keep looping, so we have to be sure to // clean up before returning an error - reader := bufio.NewReader(dotfile) + reader := bufio.NewReader(io.LimitReader(dotfile, 2048)) line, _, err := reader.ReadLine() _ = dotfile.Close() if err != nil { - return "", fmt.Errorf(".git exists but is not a direcctory and cannot be read: %w", err) + return "", fmt.Errorf(".git exists but is not a directory and cannot be read: %w", err) } dotContent := string(line) if strings.HasPrefix(dotContent, "gitdir:") { // This is a submodule parent path link. Strip the prefix, clean the string of whitespace just to // be safe, and return dotContent = strings.TrimSpace(strings.TrimPrefix(dotContent, "gitdir: ")) - return dotContent, nil + p, err := detectGitPath(dotContent, depth+1) + if err != nil { + return "", fmt.Errorf(".git gitdir error: %w", err) + } + return p, nil } return "", fmt.Errorf(".git exist but is not a directory or module/workspace file") } diff --git a/repository/gogit_test.go b/repository/gogit_test.go index 2bec97a5..21acd5df 100644 --- a/repository/gogit_test.go +++ b/repository/gogit_test.go @@ -1,6 +1,7 @@ package repository import ( + "fmt" "os" "path" "path/filepath" @@ -84,19 +85,14 @@ func TestGoGitRepo_Indexes(t *testing.T) { } func TestGoGit_DetectsSubmodules(t *testing.T) { - expectedPath := "../foo/bar" - submoduleData := "gitdir: " + expectedPath + repo := CreateGoGitTestRepo(t, false) + expected := filepath.Join(goGitRepoDir(t, repo), "/.git") + d := t.TempDir() - if f, err := os.Create(filepath.Join(d, ".git")); err != nil { - t.Fatal("could not create necessary temp file:", err) - } else { - t.Log(f.Name()) - if _, err := f.Write([]byte(submoduleData)); err != nil { - t.Fatal("could not write necessary data to temp file:", err) - } - _ = f.Close() - } - result, err := detectGitPath(d) + err := os.WriteFile(filepath.Join(d, ".git"), []byte(fmt.Sprintf("gitdir: %s", expected)), 0600) + require.NoError(t, err) + + result, err := detectGitPath(d, 0) assert.Empty(t, err) - assert.Equal(t, expectedPath, result) + assert.Equal(t, expected, result) } -- cgit