From 693515f9dd001f0b774444ef2d7705923219289f Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sun, 24 Mar 2019 21:59:20 +0100 Subject: [PATCH 1/3] util: increase function length when using `callbackify()` The returned function from `util.callbackify()` should increase the `length` property by one due to the added callback. --- lib/util.js | 9 +++++++-- test/parallel/test-util-callbackify.js | 10 ++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lib/util.js b/lib/util.js index f6f99f82b45722..1935955ad28c32 100644 --- a/lib/util.js +++ b/lib/util.js @@ -195,8 +195,13 @@ function callbackify(original) { } Object.setPrototypeOf(callbackified, Object.getPrototypeOf(original)); - Object.defineProperties(callbackified, - Object.getOwnPropertyDescriptors(original)); + const descriptors = Object.getOwnPropertyDescriptors(original); + // It is possible to manipulate a functions `length` property. This guards + // against the manipulation. + if (typeof descriptors.length.value === 'number') { + descriptors.length.value++; + } + Object.defineProperties(callbackified, descriptors); return callbackified; } diff --git a/test/parallel/test-util-callbackify.js b/test/parallel/test-util-callbackify.js index 9be3b132f95172..4629d62afbcddb 100644 --- a/test/parallel/test-util-callbackify.js +++ b/test/parallel/test-util-callbackify.js @@ -74,6 +74,7 @@ const values = [ } const cbAsyncFn = callbackify(asyncFn); + assert.strictEqual(cbAsyncFn.length, 1); cbAsyncFn(common.mustCall((err, ret) => { assert.strictEqual(ret, undefined); if (err instanceof Error) { @@ -146,6 +147,7 @@ const values = [ } const cbAsyncFn = callbackify(asyncFn); + assert.strictEqual(cbAsyncFn.length, 2); cbAsyncFn(value, common.mustCall((err, ret) => { assert.ifError(err); assert.strictEqual(ret, value); @@ -155,8 +157,16 @@ const values = [ assert.strictEqual(arg, value); return Promise.resolve(arg); } + const obj = {}; + Object.defineProperty(promiseFn, 'length', { + value: obj, + writable: false, + enumerable: false, + configurable: true + }); const cbPromiseFn = callbackify(promiseFn); + assert.strictEqual(promiseFn.length, obj); cbPromiseFn(value, common.mustCall((err, ret) => { assert.ifError(err); assert.strictEqual(ret, value); From 386340bc10d6d7931c388505f40e06231a911f45 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sun, 24 Mar 2019 22:11:47 +0100 Subject: [PATCH 2/3] util: rename callbackified function This makes sure the function returned by `util.callbackify()` has a new name that is not identical to the inputs function name. --- lib/util.js | 7 +++++-- test/parallel/test-util-callbackify.js | 9 +++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/util.js b/lib/util.js index 1935955ad28c32..4db05e478c4e86 100644 --- a/lib/util.js +++ b/lib/util.js @@ -196,11 +196,14 @@ function callbackify(original) { Object.setPrototypeOf(callbackified, Object.getPrototypeOf(original)); const descriptors = Object.getOwnPropertyDescriptors(original); - // It is possible to manipulate a functions `length` property. This guards - // against the manipulation. + // It is possible to manipulate a functions `length` or `name` property. This + // guards against the manipulation. if (typeof descriptors.length.value === 'number') { descriptors.length.value++; } + if (typeof descriptors.name.value === 'string') { + descriptors.name.value += 'Callbackified'; + } Object.defineProperties(callbackified, descriptors); return callbackified; } diff --git a/test/parallel/test-util-callbackify.js b/test/parallel/test-util-callbackify.js index 4629d62afbcddb..5a0e6c003c6dd0 100644 --- a/test/parallel/test-util-callbackify.js +++ b/test/parallel/test-util-callbackify.js @@ -75,6 +75,7 @@ const values = [ const cbAsyncFn = callbackify(asyncFn); assert.strictEqual(cbAsyncFn.length, 1); + assert.strictEqual(cbAsyncFn.name, 'asyncFnCallbackified'); cbAsyncFn(common.mustCall((err, ret) => { assert.strictEqual(ret, undefined); if (err instanceof Error) { @@ -94,8 +95,16 @@ const values = [ function promiseFn() { return Promise.reject(value); } + const obj = {}; + Object.defineProperty(promiseFn, 'name', { + value: obj, + writable: false, + enumerable: false, + configurable: true + }); const cbPromiseFn = callbackify(promiseFn); + assert.strictEqual(promiseFn.name, obj); cbPromiseFn(common.mustCall((err, ret) => { assert.strictEqual(ret, undefined); if (err instanceof Error) { From a15dd01c2fd440d43f0410ddc6a0f8ea941804c0 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sun, 24 Mar 2019 22:20:55 +0100 Subject: [PATCH 3/3] util: don't set the prototype of callbackified functions Using `util.callbackify()` should not set the prototype for the returned function to the one from the input function. It could cause confusion while debugging otherwise. --- lib/util.js | 1 - test/parallel/test-util-callbackify.js | 5 +++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/util.js b/lib/util.js index 4db05e478c4e86..f58f0e7a3e7605 100644 --- a/lib/util.js +++ b/lib/util.js @@ -194,7 +194,6 @@ function callbackify(original) { (rej) => process.nextTick(callbackifyOnRejected, rej, cb)); } - Object.setPrototypeOf(callbackified, Object.getPrototypeOf(original)); const descriptors = Object.getOwnPropertyDescriptors(original); // It is possible to manipulate a functions `length` or `name` property. This // guards against the manipulation. diff --git a/test/parallel/test-util-callbackify.js b/test/parallel/test-util-callbackify.js index 5a0e6c003c6dd0..2a956d06beef3e 100644 --- a/test/parallel/test-util-callbackify.js +++ b/test/parallel/test-util-callbackify.js @@ -157,6 +157,11 @@ const values = [ const cbAsyncFn = callbackify(asyncFn); assert.strictEqual(cbAsyncFn.length, 2); + assert.notStrictEqual( + Object.getPrototypeOf(cbAsyncFn), + Object.getPrototypeOf(asyncFn) + ); + assert.strictEqual(Object.getPrototypeOf(cbAsyncFn), Function.prototype); cbAsyncFn(value, common.mustCall((err, ret) => { assert.ifError(err); assert.strictEqual(ret, value);