From 1f40b2a63616efe0e4c0744a1f630161526e4236 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Wed, 18 Feb 2015 12:55:13 -0500 Subject: [PATCH] fs: add type checking to makeCallback() This commit adds proper type checking to makeCallback(). Anything other than undefined or a function will throw. PR-URL: https://github.com/iojs/io.js/pull/866 Reviewed-By: Ben Noordhuis Reviewed-By: Vladimir Kurchatkin --- lib/fs.js | 6 +++++- test/parallel/test-fs-make-callback.js | 27 ++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-fs-make-callback.js diff --git a/lib/fs.js b/lib/fs.js index d4f77083413d13..b2aba37f561be8 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -65,10 +65,14 @@ function maybeCallback(cb) { // for callbacks that are passed to the binding layer, callbacks that are // invoked from JS already run in the proper scope. function makeCallback(cb) { - if (typeof cb !== 'function') { + if (cb === undefined) { return rethrow(); } + if (typeof cb !== 'function') { + throw new TypeError('callback must be a function'); + } + return function() { return cb.apply(null, arguments); }; diff --git a/test/parallel/test-fs-make-callback.js b/test/parallel/test-fs-make-callback.js new file mode 100644 index 00000000000000..91990015b8220c --- /dev/null +++ b/test/parallel/test-fs-make-callback.js @@ -0,0 +1,27 @@ +var common = require('../common'); +var assert = require('assert'); +var fs = require('fs'); + +function test(cb) { + return function() { + // fs.stat() calls makeCallback() on its second argument + fs.stat(__filename, cb); + }; +} + +// Verify the case where a callback function is provided +assert.doesNotThrow(test(function() {})); + +// Passing undefined calls rethrow() internally, which is fine +assert.doesNotThrow(test(undefined)); + +// Anything else should throw +assert.throws(test(null)); +assert.throws(test(true)); +assert.throws(test(false)); +assert.throws(test(1)); +assert.throws(test(0)); +assert.throws(test('foo')); +assert.throws(test(/foo/)); +assert.throws(test([])); +assert.throws(test({}));