From 8f9ed33446c160b49e2269d098acaf7d6a05ef87 Mon Sep 17 00:00:00 2001 From: Tim Branyen Date: Mon, 23 Feb 2015 19:36:38 -0500 Subject: [PATCH 1/4] Unit test showing segfault via #406 If you attempt to clone a private repository without passing along a username, `NULL` will make `NanNew` very upset. --- test/tests/clone.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/tests/clone.js b/test/tests/clone.js index 234a38916..befee0fc9 100644 --- a/test/tests/clone.js +++ b/test/tests/clone.js @@ -103,4 +103,18 @@ describe("Clone", function() { return prepTestAndClean(url, file); }); + + it("will not segfault when accessing a url without username", function() { + var url = "https://github.com/nodegit/private"; + + return Clone.clone(url, git, { + certificateCheck: function() { return 1; }, + remoteCallbacks: { + credentials: function() { + return NodeGit.Cred.userpassPlaintextNew("fake-token", + "x-oauth-basic"); + } + } + }).catch(); + }); }); From 887459931b7d79b765cb0dcbdb194133077cda6e Mon Sep 17 00:00:00 2001 From: Tim Branyen Date: Mon, 23 Feb 2015 19:18:54 -0500 Subject: [PATCH 2/4] Fixes segfault issue recorded in #406 A segfault was occuring when passing null values to `NanNew`, this patch will the bug for `const char *` and `unsigned int` C types. We'll definitely need to add more in the future. --- generate/templates/partials/field_accessors.cc | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/generate/templates/partials/field_accessors.cc b/generate/templates/partials/field_accessors.cc index b5a6f5968..cd3a1e4a0 100644 --- a/generate/templates/partials/field_accessors.cc +++ b/generate/templates/partials/field_accessors.cc @@ -122,6 +122,20 @@ return; } + {% each field.args|argsInfo as arg %} + {% if arg.name == "payload" %} + {%-- Do nothing --%} + {% elsif arg.isJsArg %} + if (baton->{{ arg.name }} == NULL) { + {% if arg.cType == "const char *" %} + baton->{{ arg.name }} = ""; + {% elsif arg.cType == "unsigned int" %} + baton->{{ arg.name }} = 0; + {% endif %} + } + {% endif %} + {% endeach %} + Local argv[{{ field.args|jsArgsCount }}] = { {% each field.args|argsInfo as arg %} {% if arg.name == "payload" %} From 5d455566a97250e91ab79c2659f688f4078e2705 Mon Sep 17 00:00:00 2001 From: Tim Date: Mon, 23 Feb 2015 20:17:23 -0500 Subject: [PATCH 3/4] Refactored tests to use before to clean This allowed us to reuse folders. I also fixed the tests to allow use a certificate check callback (where required). --- test/tests/clone.js | 65 ++++++++++++++++++++++++++++++--------------- 1 file changed, 44 insertions(+), 21 deletions(-) diff --git a/test/tests/clone.js b/test/tests/clone.js index befee0fc9..af7bf6404 100644 --- a/test/tests/clone.js +++ b/test/tests/clone.js @@ -12,7 +12,6 @@ describe("Clone", function() { var http = local("../repos/http"); var https = local("../repos/https"); var ssh = local("../repos/ssh"); - var sshManual = local("../repos/sshmanual"); var git = local("../repos/git"); var file = local("../repos/file"); @@ -22,15 +21,15 @@ describe("Clone", function() { // Set a reasonable timeout here now that our repository has grown. this.timeout(15000); - function prepTestAndClean(url, location, opts) { - return fse.remove(location) - .then(function() { - return Clone.clone(url, location, opts); - }) - .then(function(repo) { - assert.ok(repo instanceof Repository); - }); - } + beforeEach(function() { + return NodeGit.Promise.all([ + fse.remove(http), + fse.remove(https), + fse.remove(ssh), + fse.remove(git), + fse.remove(file) + ]); + }); it.skip("can clone with http", function() { var url = "http://github.com/nodegit/test.git"; @@ -42,7 +41,9 @@ describe("Clone", function() { } }; - return prepTestAndClean(url, http, opts); + return Clone.clone(url, http, opts).then(function(repo) { + assert.ok(repo instanceof Repository); + }); }); it("can clone with https", function() { @@ -55,28 +56,36 @@ describe("Clone", function() { } }; - return prepTestAndClean(url, https, opts); + return Clone.clone(url, https, opts).then(function(repo) { + assert.ok(repo instanceof Repository); + }); }); it("can clone with ssh", function() { var url = "git@github.com:nodegit/test.git"; var opts = { - ignoreCertErrors: 1, remoteCallbacks: { + certificateCheck: function() { + return 1; + }, credentials: function(url, userName) { return NodeGit.Cred.sshKeyFromAgent(userName); } } }; - return prepTestAndClean(url, ssh, opts); + return Clone.clone(url, ssh, opts).then(function(repo) { + assert.ok(repo instanceof Repository); + }); }); it("can clone with ssh while manually loading a key", function() { var url = "git@github.com:nodegit/test.git"; var opts = { - ignoreCertErrors: 1, remoteCallbacks: { + certificateCheck: function() { + return 1; + }, credentials: function(url, userName) { return NodeGit.Cred.sshKeyNew( userName, @@ -87,34 +96,48 @@ describe("Clone", function() { } }; - return prepTestAndClean(url, sshManual, opts); + return Clone.clone(url, ssh, opts).then(function(repo) { + assert.ok(repo instanceof Repository); + }); }); it("can clone with git", function() { var url = "git://github.com/nodegit/test.git"; - var opts = { ignoreCertErrors: 1 }; + var opts = { + remoteCallbacks: { + certificateCheck: function() { + return 1; + } + } + }; - return prepTestAndClean(url, git, opts); + return Clone.clone(url, git, opts).then(function(repo) { + assert.ok(repo instanceof Repository); + }); }); it("can clone with filesystem", function() { var prefix = process.platform === "win32" ? "" : "file://"; var url = prefix + local("../repos/empty"); - return prepTestAndClean(url, file); + return Clone.clone(url, file).then(function(repo) { + assert.ok(repo instanceof Repository); + }); }); it("will not segfault when accessing a url without username", function() { var url = "https://github.com/nodegit/private"; return Clone.clone(url, git, { - certificateCheck: function() { return 1; }, remoteCallbacks: { + certificateCheck: function() { + return 1; + }, credentials: function() { return NodeGit.Cred.userpassPlaintextNew("fake-token", "x-oauth-basic"); } } - }).catch(); + }).catch(function unhandledError() { }); }); }); From ad310b300e9ed8c318e41c4990026c5d45ca5601 Mon Sep 17 00:00:00 2001 From: Tim Branyen Date: Mon, 23 Feb 2015 23:45:14 -0500 Subject: [PATCH 4/4] Attempt to fix the beforeEach --- test/tests/clone.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/tests/clone.js b/test/tests/clone.js index af7bf6404..bbe6dfa4e 100644 --- a/test/tests/clone.js +++ b/test/tests/clone.js @@ -28,7 +28,7 @@ describe("Clone", function() { fse.remove(ssh), fse.remove(git), fse.remove(file) - ]); + ]).catch(function unhandledFunction() {}); }); it.skip("can clone with http", function() { @@ -103,7 +103,7 @@ describe("Clone", function() { it("can clone with git", function() { var url = "git://github.com/nodegit/test.git"; - var opts = { + var opts = { remoteCallbacks: { certificateCheck: function() { return 1;