From 77acb11cd0bb5d9c2be8fc046e71296b501de874 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 26 Mar 2014 19:14:57 +0100 Subject: [PATCH] credentials: memory safety The docs say to use tp_free() to free the memory, and even though we use PyObject_Del() everywhere else, using this in the credentials does cause issues. --- src/credentials.c | 4 ++-- src/pygit2.c | 8 +++++--- src/utils.c | 4 ++-- test/test_repository.py | 7 +++++++ 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/credentials.c b/src/credentials.c index 5711f04..c3127ae 100644 --- a/src/credentials.c +++ b/src/credentials.c @@ -72,7 +72,7 @@ CredUsernamePassword_dealloc(CredUsernamePassword *self) free(self->username); free(self->password); - PyObject_Del(self); + Py_TYPE(self)->tp_free(self); } PyMemberDef CredUsernamePassword_members[] = { @@ -169,7 +169,7 @@ CredSshKey_dealloc(CredSshKey *self) free(self->privkey); free(self->passphrase); - PyObject_Del(self); + Py_TYPE(self)->tp_free(self); } PyMemberDef CredSshKey_members[] = { diff --git a/src/pygit2.c b/src/pygit2.c index 0126dc8..199bbac 100644 --- a/src/pygit2.c +++ b/src/pygit2.c @@ -154,7 +154,7 @@ clone_repository(PyObject *self, PyObject *args) { const char *path; unsigned int bare, ignore_cert_errors; const char *remote_name, *checkout_branch; - PyObject *credentials; + PyObject *credentials = NULL; int err; git_clone_options opts = GIT_CLONE_OPTIONS_INIT; @@ -167,8 +167,10 @@ clone_repository(PyObject *self, PyObject *args) { opts.remote_name = remote_name; opts.checkout_branch = checkout_branch; - opts.remote_callbacks.credentials = credentials_cb; - opts.remote_callbacks.payload = credentials; + if (credentials != Py_None) { + opts.remote_callbacks.credentials = credentials_cb; + opts.remote_callbacks.payload = credentials; + } err = git_clone(&repo, url, path, &opts); if (err < 0) diff --git a/src/utils.c b/src/utils.c index 66e546d..59e7ad5 100644 --- a/src/utils.c +++ b/src/utils.c @@ -202,9 +202,9 @@ int callable_to_credentials(git_cred **out, const char *url, const char *username_from_url, unsigned int allowed_types, PyObject *credentials) { int err; - PyObject *py_cred, *arglist; + PyObject *py_cred = NULL, *arglist = NULL; - if (credentials == NULL) + if (credentials == NULL || credentials == Py_None) return 0; if (!PyCallable_Check(credentials)) { diff --git a/test/test_repository.py b/test/test_repository.py index 8e8101c..71214cc 100644 --- a/test/test_repository.py +++ b/test/test_repository.py @@ -461,6 +461,13 @@ class CloneRepositoryTest(utils.NoRepoTestCase): self.assertFalse(repo.is_empty) self.assertEqual(repo.remotes[0].name, "custom_remote") + def test_clone_with_credentials(self): + credentials = pygit2.UserPass("libgit2", "libgit2") + repo = clone_repository( + "https://bitbucket.org/libgit2/testgitrepository.git", + self._temp_dir, credentials=credentials) + + self.assertFalse(repo.is_empty) # FIXME The tests below are commented because they are broken: #