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

(*Worktree).Commit will panic if CommitOptions parameter is nil. #1051

Open
onee-only opened this issue Mar 13, 2024 · 1 comment
Open

(*Worktree).Commit will panic if CommitOptions parameter is nil. #1051

onee-only opened this issue Mar 13, 2024 · 1 comment

Comments

@onee-only
Copy link
Contributor

I found that (*Worktree).Commit method is taking pointer as parameter, and derenferencing it without checking if it is nil.
If someone passes nil there, It should panic for sure.

I think if we need users to pass non-nil pointer, we should document it.
Or we could just replace it with pointer to empty struct if it is nil.

I thought about changing the parameter to non-pointer struct, but it seems like it is breaking change and we will lose consistency in codebase.

Example code:

package main

import (
	"github.com/go-git/go-billy/v5/memfs"
	"github.com/go-git/go-billy/v5/util"
	"github.com/go-git/go-git/v5"
	"github.com/go-git/go-git/v5/storage/memory"
)

func main() {
	fs := memfs.New()
	storage := memory.NewStorage()

	r, _ := git.Init(storage, fs)
	w, _ := r.Worktree()

	util.WriteFile(fs, "foo", []byte("foo"), 0755)
	w.Add("foo")

	_, err := w.Commit("this will panic", nil)
	if err != nil {
		panic(err)
	}
}
@onee-only
Copy link
Contributor Author

Seems like a number of functions in this codebase taking pointer to XXXOptions struct do not check if it is nil.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant