Skip to content

Commit 77acb11

Browse files
committed
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.
1 parent b3ce1b5 commit 77acb11

File tree

4 files changed

+16
-7
lines changed

4 files changed

+16
-7
lines changed

src/credentials.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ CredUsernamePassword_dealloc(CredUsernamePassword *self)
7272
free(self->username);
7373
free(self->password);
7474

75-
PyObject_Del(self);
75+
Py_TYPE(self)->tp_free(self);
7676
}
7777

7878
PyMemberDef CredUsernamePassword_members[] = {
@@ -169,7 +169,7 @@ CredSshKey_dealloc(CredSshKey *self)
169169
free(self->privkey);
170170
free(self->passphrase);
171171

172-
PyObject_Del(self);
172+
Py_TYPE(self)->tp_free(self);
173173
}
174174

175175
PyMemberDef CredSshKey_members[] = {

src/pygit2.c

+5-3
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ clone_repository(PyObject *self, PyObject *args) {
154154
const char *path;
155155
unsigned int bare, ignore_cert_errors;
156156
const char *remote_name, *checkout_branch;
157-
PyObject *credentials;
157+
PyObject *credentials = NULL;
158158
int err;
159159
git_clone_options opts = GIT_CLONE_OPTIONS_INIT;
160160

@@ -167,8 +167,10 @@ clone_repository(PyObject *self, PyObject *args) {
167167
opts.remote_name = remote_name;
168168
opts.checkout_branch = checkout_branch;
169169

170-
opts.remote_callbacks.credentials = credentials_cb;
171-
opts.remote_callbacks.payload = credentials;
170+
if (credentials != Py_None) {
171+
opts.remote_callbacks.credentials = credentials_cb;
172+
opts.remote_callbacks.payload = credentials;
173+
}
172174

173175
err = git_clone(&repo, url, path, &opts);
174176
if (err < 0)

src/utils.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -202,9 +202,9 @@ int
202202
callable_to_credentials(git_cred **out, const char *url, const char *username_from_url, unsigned int allowed_types, PyObject *credentials)
203203
{
204204
int err;
205-
PyObject *py_cred, *arglist;
205+
PyObject *py_cred = NULL, *arglist = NULL;
206206

207-
if (credentials == NULL)
207+
if (credentials == NULL || credentials == Py_None)
208208
return 0;
209209

210210
if (!PyCallable_Check(credentials)) {

test/test_repository.py

+7
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,13 @@ def test_clone_remote_name(self):
461461
self.assertFalse(repo.is_empty)
462462
self.assertEqual(repo.remotes[0].name, "custom_remote")
463463

464+
def test_clone_with_credentials(self):
465+
credentials = pygit2.UserPass("libgit2", "libgit2")
466+
repo = clone_repository(
467+
"https://bitbucket.org/libgit2/testgitrepository.git",
468+
self._temp_dir, credentials=credentials)
469+
470+
self.assertFalse(repo.is_empty)
464471

465472
# FIXME The tests below are commented because they are broken:
466473
#

0 commit comments

Comments
 (0)