diff --git a/.travis.yml b/.travis.yml index 2b317c86e..8d159873a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,7 +13,7 @@ sudo: false env: matrix: - - export NODE_VERSION="stable" TARGET_ARCH="x64" + - export NODE_VERSION="8.3.0" TARGET_ARCH="x64" - export NODE_VERSION="7.4" TARGET_ARCH="x64" - export NODE_VERSION="6.5" TARGET_ARCH="x64" @@ -21,7 +21,7 @@ matrix: fast_finish: true include: - os: linux - env: export NODE_VERSION="stable" TARGET_ARCH="ia32" + env: export NODE_VERSION="8.3.0" TARGET_ARCH="ia32" - os: linux env: export NODE_VERSION="7.4" TARGET_ARCH="ia32" - os: linux diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d8fdc98a..b01501353 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Change Log +## v0.20.2 [(2017-08-16)](https://github.com/nodegit/nodegit/releases/tag/v0.20.2) + +[Full Changelog](https://github.com/nodegit/nodegit/compare/v0.20.0...v0.20.2) + +#### Summary of changes outside of libgit2 bump: + - [Fixed an issue where large buffer allocations in filters (> 256MB) causes a segfault #1368](https://github.com/nodegit/nodegit/pull/1368) + - [Fix git_tree_entry double free #1332](https://github.com/nodegit/nodegit/pull/1332) + ## v0.20.0 [(2017-08-16)](https://github.com/nodegit/nodegit/releases/tag/v0.20.0) [Full Changelog](https://github.com/nodegit/nodegit/compare/v0.19.0...v0.20.0) diff --git a/README.md b/README.md index 3a9ee8b80..52f2a26f8 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,6 @@ NodeGit -**Stable (libgit2#master): 0.20.0** **Stable (libgit2@v0.26.0): 0.26.0** ## Have a problem? Come chat with us! ## diff --git a/appveyor.yml b/appveyor.yml index c25c50ebb..7d4463df7 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -27,7 +27,7 @@ environment: GYP_MSVS_VERSION: 2013 matrix: # Node.js - - nodejs_version: "stable" + - nodejs_version: "8.3.0" - nodejs_version: "7" - nodejs_version: "6" diff --git a/generate/input/descriptor.json b/generate/input/descriptor.json index 7d1953de8..c7ceafe73 100644 --- a/generate/input/descriptor.json +++ b/generate/input/descriptor.json @@ -173,7 +173,8 @@ "out": { "isReturn": true, "cppClassName": "GitBuf", - "jsClassName": "Buffer" + "jsClassName": "Buffer", + "shouldAlloc": true }, "blob": { "cppClassName": "GitBlob", @@ -259,7 +260,8 @@ "isReturn": true, "cppClassName": "GitBuf", "jsClassName": "Buffer", - "cType": "git_buf *" + "cType": "git_buf *", + "shouldAlloc": true }, "repo": { "cppClassName": "GitRepository", @@ -307,9 +309,9 @@ "jsFunctionName": "grow", "args": { "buffer": { - "isReturn": true, - "isSelf": false, - "shouldAlloc": true + "isReturn": false, + "isSelf": true, + "shouldAlloc": false } }, "return": { @@ -324,9 +326,9 @@ "jsFunctionName": "set", "args": { "buffer": { - "isReturn": true, - "isSelf": false, - "shouldAlloc": true + "isReturn": false, + "isSelf": true, + "shouldAlloc": false }, "data": { "cppClassName": "Buffer", @@ -537,6 +539,19 @@ "return": { "ownedByThis": true } + }, + "git_commit_header_field": { + "isAsync": true, + "args": { + "out": { + "isReturn": true, + "isSelf": false, + "shouldAlloc": true + } + }, + "return": { + "isErrorCode": true + } } } }, @@ -601,6 +616,12 @@ }, "git_config_get_string_buf": { "isAsync": true, + "args": { + "out": { + "isReturn": true, + "shouldAlloc": true + } + }, "return": { "isErrorCode": true } @@ -673,6 +694,19 @@ "return": { "isErrorCode": true } + }, + "git_config_find_programdata": { + "isAsync": true, + "return": { + "isErrorCode": true + }, + "args": { + "out": { + "isReturn": true, + "isSelf": false, + "shouldAlloc": true + } + } } }, "dependencies": [ @@ -882,6 +916,19 @@ "git_diff_status_char": { "ignore": true }, + "git_diff_to_buf": { + "isAsync": true, + "return": { + "isErrorCode": true + }, + "args": { + "out": { + "isReturn": true, + "isSelf": false, + "shouldAlloc": true + } + } + }, "git_diff_tree_to_index": { "args": { "old_tree": { @@ -1968,7 +2015,9 @@ "isAsync": true, "args": { "out": { - "isReturn": true + "isReturn": true, + "shouldAlloc": true, + "isSelf": false }, "remote": { "isSelf": true @@ -2115,6 +2164,13 @@ "isAsync": true, "return": { "isErrorCode": true + }, + "args": { + "out": { + "isReturn": true, + "isSelf": false, + "shouldAlloc": true + } } }, "git_repository_init_init_options": { @@ -2478,6 +2534,19 @@ "isErrorCode": true } }, + "git_submodule_resolve_url": { + "isAsync": true, + "args": { + "out": { + "isReturn": true, + "shouldAlloc": true, + "isSelf": false + } + }, + "return": { + "isErrorCode": true + } + }, "git_submodule_update": { "isAsync": true, "args": { @@ -2658,19 +2727,22 @@ "functions": { "git_tree_entry_byid": { "return": { - "ownedByThis": true + "ownedByThis": true, + "selfFreeing": false } }, "git_tree_entry_byindex": { "jsFunctionName": "_entryByIndex", "return": { - "ownedByThis": true + "ownedByThis": true, + "selfFreeing": false } }, "git_tree_entry_byname": { "jsFunctionName": "_entryByName", "return": { - "ownedByThis": true + "ownedByThis": true, + "selfFreeing": false } }, "git_tree_entrycount": { @@ -2691,6 +2763,21 @@ "git_treebuilder_filter": { "ignore": true }, + "git_treebuilder_get": { + "return": { + "selfFreeing": false, + "ownedByThis": true + } + }, + "git_treebuilder_insert": { + "isAsync": false, + "args": { + "out": { + "selfFreeing": false, + "ownedByThis": true + } + } + }, "git_treebuilder_write": { "args": { "id": { diff --git a/generate/templates/partials/async_function.cc b/generate/templates/partials/async_function.cc index 20813136d..191b8f20f 100644 --- a/generate/templates/partials/async_function.cc +++ b/generate/templates/partials/async_function.cc @@ -15,15 +15,6 @@ NAN_METHOD({{ cppClassName }}::{{ cppFunctionName }}) { {%if arg.globalPayload %} {{ cppFunctionName }}_globalPayload* globalPayload = new {{ cppFunctionName }}_globalPayload; {%endif%} - {%if arg.cppClassName == "GitBuf" %} - {%if cppFunctionName == "Set"%} - baton->{{arg.name}} = Nan::ObjectWrap::Unwrap<{{ arg.cppClassName }}>(info.This())->GetValue(); - {%else%} - baton->{{arg.name}} = ({{ arg.cType }})malloc(sizeof({{ arg.cType|replace '*' '' }})); - baton->{{arg.name}}->ptr = NULL; - baton->{{arg.name}}->size = baton->{{arg.name}}->asize = 0; - {%endif%} - {%endif%} {%endeach%} {%each args|argsInfo as arg %} @@ -61,9 +52,10 @@ NAN_METHOD({{ cppClassName }}::{{ cppFunctionName }}) { {%endif%} {%endif%} {%elsif arg.shouldAlloc %} + baton->{{arg.name}} = ({{ arg.cType }})malloc(sizeof({{ arg.cType|replace '*' '' }})); {%if arg.cppClassName == "GitBuf" %} - {%else%} - baton->{{ arg.name }} = ({{ arg.cType }})malloc(sizeof({{ arg.cType|replace '*' '' }})); + baton->{{arg.name}}->ptr = NULL; + baton->{{arg.name}}->size = baton->{{arg.name}}->asize = 0; {%endif%} {%endif%} {%endeach%} diff --git a/generate/templates/partials/convert_to_v8.cc b/generate/templates/partials/convert_to_v8.cc index bfe268f3e..3d3bdf834 100644 --- a/generate/templates/partials/convert_to_v8.cc +++ b/generate/templates/partials/convert_to_v8.cc @@ -43,12 +43,16 @@ to = tmpArray; {% elsif cppClassName == 'GitBuf' %} + {% if doNotConvert %} + to = Nan::Null(); + {% else %} if ({{= parsedName =}}) { to = Nan::New({{= parsedName =}}->ptr, {{= parsedName = }}->size).ToLocalChecked(); } else { to = Nan::Null(); } + {% endif %} {% else %} {% if copy %} if ({{= parsedName =}} != NULL) { diff --git a/package.json b/package.json index 268d00f86..85a78848a 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "nodegit", "description": "Node.js libgit2 asynchronous native bindings", - "version": "0.20.0", + "version": "0.20.2", "homepage": "http://nodegit.org", "keywords": [ "libgit2", diff --git a/test/tests/filter.js b/test/tests/filter.js index 1db1c9ecb..021de6426 100644 --- a/test/tests/filter.js +++ b/test/tests/filter.js @@ -83,7 +83,7 @@ describe("Filter", function() { .then(function(emptyRepo) { test.emptyRepo = emptyRepo; return fse.writeFile( - path.join(reposPath, ".gitattributes"), + path.join(reposPath, ".gitattributes"), "*.md filter=" + filterName + " -text", { encoding: "utf-8" } ); @@ -153,7 +153,7 @@ describe("Filter", function() { it("cannot unregister the filter twice", function() { return Registry.unregister(filterName) - .then(function(result) { + .then(function(result) { assert.strictEqual(result, NodeGit.Error.CODE.OK); return Registry.unregister(filterName); }) @@ -185,7 +185,7 @@ describe("Filter", function() { }) .then(function() { return fse.writeFile( - packageJsonPath, + packageJsonPath, "Changing content to trigger checkout" ); }) @@ -219,7 +219,7 @@ describe("Filter", function() { global.gc(); return fse.writeFile( - packageJsonPath, + packageJsonPath, "Changing content to trigger checkout" ); }) @@ -253,7 +253,7 @@ describe("Filter", function() { }) .then(function() { return fse.writeFile( - packageJsonPath, + packageJsonPath, "Changing content to trigger checkout" ); }) @@ -289,7 +289,7 @@ describe("Filter", function() { .then(function(result) { assert.strictEqual(result, NodeGit.Error.CODE.OK); return fse.writeFile( - packageJsonPath, + packageJsonPath, "Changing content to trigger checkout", { encoding: "utf-8" } ); @@ -309,7 +309,7 @@ describe("Filter", function() { assert.strictEqual(shutdown, true); }); }); - + it("filter successfully shuts down on garbage collect", function() { var test = this; var shutdown = false; @@ -325,7 +325,7 @@ describe("Filter", function() { .then(function(result) { assert.strictEqual(result, NodeGit.Error.CODE.OK); return fse.writeFile( - packageJsonPath, + packageJsonPath, "Changing content to trigger checkout", { encoding: "utf-8" } ); @@ -363,7 +363,7 @@ describe("Filter", function() { .then(function(result) { assert.strictEqual(result, NodeGit.Error.CODE.OK); return fse.writeFile( - packageJsonPath, + packageJsonPath, "Changing content to trigger checkout", { encoding: "utf-8" } ); @@ -389,14 +389,28 @@ describe("Filter", function() { }); describe("Apply", function() { + before(function() { + var test = this; + return fse.readFile(readmePath, "utf8") + .then((function(content) { + test.originalReadmeContent = content; + })); + }); + + afterEach(function() { + this.timeout(15000); + return fse.writeFile(readmePath, this.originalReadmeContent); + }); + var message = "some new fancy filter"; var length = message.length; var tempBuffer = new Buffer(message, "utf-8"); + var largeBufferSize = 500000000; it("should not apply when check returns GIT_PASSTHROUGH", function(){ var test = this; var applied = false; - + return Registry.register(filterName, { apply: function() { applied = true; @@ -408,7 +422,7 @@ describe("Filter", function() { .then(function(result) { assert.strictEqual(result, NodeGit.Error.CODE.OK); return fse.writeFile( - packageJsonPath, + packageJsonPath, "Changing content to trigger checkout", { encoding: "utf-8" } ); @@ -440,8 +454,8 @@ describe("Filter", function() { .then(function(result) { assert.strictEqual(result, NodeGit.Error.CODE.OK); return fse.writeFile( - packageJsonPath, - "Changing content to trigger checkout", + packageJsonPath, + "Changing content to trigger checkout", { encoding: "utf-8" } ); }) @@ -476,13 +490,13 @@ describe("Filter", function() { }) .then(function() { var readmeContent = fse.readFileSync( - packageJsonPath, + packageJsonPath, "utf-8" ); assert.notStrictEqual(readmeContent, message); return fse.writeFile( - packageJsonPath, + packageJsonPath, "Changing content to trigger checkout" ); }) @@ -495,7 +509,7 @@ describe("Filter", function() { }) .then(function() { var postInitializeReadmeContents = fse.readFileSync( - readmePath, + readmePath, "utf-8" ); @@ -509,7 +523,7 @@ describe("Filter", function() { return Registry.register(filterName, { apply: function(to, from, source) { return to.set(tempBuffer, length) - .then(function(buf) { + .then(function() { return NodeGit.Error.CODE.OK; }); }, @@ -522,7 +536,7 @@ describe("Filter", function() { }) .then(function() { var readmeContent = fse.readFileSync( - readmePath, + readmePath, "utf-8" ); assert.notStrictEqual(readmeContent, message); @@ -536,7 +550,7 @@ describe("Filter", function() { }) .then(function() { var postInitializeReadmeContents = fse.readFileSync( - readmePath, + readmePath, "utf-8" ); @@ -544,13 +558,76 @@ describe("Filter", function() { }); }); + // this test is useless on 32 bit CI, because we cannot construct + // a buffer big enough to test anything of significance :)... + if (process.arch === "x64") { + it("applies the massive filter data on checkout", function() { + this.timeout(350000); + var test = this; + var largeBuffer = Buffer.alloc(largeBufferSize, "a"); + + return Registry.register(filterName, { + apply: function(to, from, source) { + return to.set(largeBuffer, largeBufferSize) + .then(function() { + return NodeGit.Error.CODE.OK; + }); + }, + check: function(src, attr) { + return NodeGit.Error.CODE.OK; + } + }, 0) + .then(function(result) { + assert.strictEqual(result, 0); + }) + .then(function() { + var fd = fse.openSync(readmePath, "r"); + var readBuf = Buffer.allocUnsafe(largeBufferSize); + var readLength = fse.readSync( + fd, + readBuf, + 0, + largeBufferSize, + 0 + ); + fse.closeSync(fd); + + assert.notStrictEqual(readLength, largeBufferSize); + fse.writeFileSync(readmePath, "whoa", "utf8"); + + var opts = { + checkoutStrategy: Checkout.STRATEGY.FORCE, + paths: ["README.md"] + }; + return Checkout.head(test.repository, opts); + }) + .then(function() { + var fd = fse.openSync(readmePath, "r"); + var readBuf = Buffer.allocUnsafe(largeBufferSize); + var readLength = fse.readSync( + fd, + readBuf, + 0, + largeBufferSize, + 0 + ); + fse.closeSync(fd); + + assert.strictEqual( + readLength, + largeBufferSize + ); + }); + }); + } + it("applies the filter data on checkout with gc", function() { var test = this; return Registry.register(filterName, { apply: function(to, from, source) { return to.set(tempBuffer, length) - .then(function(buf) { + .then(function() { return NodeGit.Error.CODE.OK; }); }, @@ -563,7 +640,7 @@ describe("Filter", function() { }) .then(function() { var readmeContent = fse.readFileSync( - readmePath, + readmePath, "utf-8" ); assert.notStrictEqual(readmeContent, message); @@ -578,7 +655,7 @@ describe("Filter", function() { }) .then(function() { var postInitializeReadmeContents = fse.readFileSync( - readmePath, + readmePath, "utf-8" ); @@ -592,12 +669,12 @@ describe("Filter", function() { return Registry.register(filterName, { apply: function(to, from, source) { return to.set(tempBuffer, length) - .then(function(buf) { + .then(function() { return NodeGit.Error.CODE.OK; }); }, check: function(src, attr) { - return src.path() === "README.md" ? + return src.path() === "README.md" ? 0 : NodeGit.Error.CODE.PASSTHROUGH; }, cleanup: function() {} @@ -607,14 +684,14 @@ describe("Filter", function() { }) .then(function() { var readmeContent = fse.readFileSync( - readmePath, + readmePath, "utf-8" ); assert.notStrictEqual(readmeContent, "testing commit contents"); }) .then(function() { - return commitFile(test.repository, "README.md", - "testing commit contents", + return commitFile(test.repository, "README.md", + "testing commit contents", "test commit" ); }) @@ -623,7 +700,7 @@ describe("Filter", function() { }) .then(function(commit) { var postInitializeReadmeContents = fse.readFileSync( - readmePath, + readmePath, "utf-8" ); @@ -649,7 +726,7 @@ describe("Filter", function() { return Registry.register(filterName, { apply: function(to, from, source) { return to.set(tempBuffer, length) - .then(function(buf) { + .then(function() { return NodeGit.Error.CODE.OK; }); }, @@ -665,14 +742,14 @@ describe("Filter", function() { }) .then(function() { var readmeContent = fse.readFileSync( - readmePath, + readmePath, "utf-8" ); assert.notStrictEqual(readmeContent, "testing commit contents"); }) .then(function() { - return commitFile(test.repository, "README.md", - "testing commit contents", + return commitFile(test.repository, "README.md", + "testing commit contents", "test commit" ); }) @@ -682,7 +759,7 @@ describe("Filter", function() { }) .then(function(commit) { var postInitializeReadmeContents = fse.readFileSync( - readmePath, + readmePath, "utf-8" ); @@ -733,7 +810,7 @@ describe("Filter", function() { assert.notEqual(packageContent, ""); return fse.writeFile( - packageJsonPath, + packageJsonPath, "Changing content to trigger checkout", { encoding: "utf-8" } ); @@ -772,14 +849,14 @@ describe("Filter", function() { }) .then(function() { var packageContent = fse.readFileSync( - packageJsonPath, + packageJsonPath, "utf-8" ); assert.notEqual(packageContent, ""); global.gc(); return fse.writeFile( - packageJsonPath, + packageJsonPath, "Changing content to trigger checkout", { encoding: "utf-8" } ); @@ -819,11 +896,11 @@ describe("Filter", function() { }) .then(function() { var packageContent = fse.readFileSync( - packageJsonPath, + packageJsonPath, "utf-8" ); var readmeContent = fse.readFileSync( - readmePath, + readmePath, "utf-8" ); @@ -832,7 +909,7 @@ describe("Filter", function() { }) .then(function() { return fse.writeFile( - packageJsonPath, + packageJsonPath, "Changing content to trigger checkout", { encoding: "utf-8" } ); diff --git a/test/tests/treebuilder.js b/test/tests/treebuilder.js index 5e1ce6263..c7ef01f43 100644 --- a/test/tests/treebuilder.js +++ b/test/tests/treebuilder.js @@ -5,10 +5,14 @@ var promisify = require("promisify-node"); var readDir = promisify(fs.readdir); var local = path.join.bind(path, __dirname); +var leakTest = require("../utils/leak_test"); + describe("TreeBuilder", function(){ var Git = require("../../"); var reposPath = local("../repos/workdir"); + var oid = "111dd657329797f6165f52f5085f61ac976dcf04"; + //setup test repo each test beforeEach(function() { var test = this; @@ -76,4 +80,19 @@ describe("TreeBuilder", function(){ return Git.Tree.lookup(test.repo, newTreeEntry.oid()); }); }); + + it("does not leak inserts", function() { + var test = this; + + // The underlying C++ git_tree_entry is owned by the treebuilder that + // creates it. But since git_tree_entry is duplicable the generator will + // duplicate it and mark it as self freeing. + // Validate this with the leakTest. + return leakTest(Git.TreeEntry, function() { + return Git.Treebuilder.create(test.repo, null) + .then(function(builder) { + return builder.insert("test", oid, Git.TreeEntry.FILEMODE.BLOB); + }); + }); + }); });