Skip to content

Commit

Permalink
fs: fix confusing flags TypeError msg
Browse files Browse the repository at this point in the history
File open flags must be an int when passed to the binding layer, but
they must be a string when passed to the fs module (numbers are
permitted, though undocumented). The module used to do no type checking,
so the binding layer error would be thrown, and it was wrong:

    > fs.openSync('_')
    TypeError: flags must be an int
        at TypeError (native)
        at Object.fs.openSync (fs.js:549:18)

It is now:

    > fs.openSync('_')
    TypeError: flag must be a string

PR-URL: #2902
Reviewed-By: Ben Noordhuis <ben@strongloop.com>
  • Loading branch information
sam-github committed Sep 16, 2015
1 parent 6cb9e8a commit 493864a
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 3 deletions.
7 changes: 4 additions & 3 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -465,10 +465,11 @@ fs.readFileSync = function(path, options) {

// Used by binding.open and friends
function stringToFlags(flag) {
// Only mess with strings
if (typeof flag !== 'string') {
if (typeof flag === 'number')
return flag;
}

if (typeof flag !== 'string')
throw new TypeError('flag must be a string');

switch (flag) {
case 'r' : return O_RDONLY;
Expand Down
14 changes: 14 additions & 0 deletions test/parallel/test-fs-open-flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,17 @@ assert.equal(fs._stringToFlags('xa+'), O_APPEND | O_CREAT | O_RDWR | O_EXCL);
'x +x x+ rx rx+ wxx wax xwx xxx').split(' ').forEach(function(flags) {
assert.throws(function() { fs._stringToFlags(flags); });
});

// Use of numeric flags is permitted.
assert.equal(fs._stringToFlags(O_RDONLY), O_RDONLY);

// Non-numeric/string flags are a type error.
assert.throws(function() { fs._stringToFlags(undefined); }, TypeError);
assert.throws(function() { fs._stringToFlags(null); }, TypeError);
assert.throws(function() { fs._stringToFlags(assert); }, TypeError);
assert.throws(function() { fs._stringToFlags([]); }, TypeError);
assert.throws(function() { fs._stringToFlags({}); }, TypeError);

// Numeric flags that are not int will be passed to the binding, which
// will throw a TypeError
assert.throws(function() { fs.openSync('_', O_RDONLY + 0.1); }, TypeError);

0 comments on commit 493864a

Please sign in to comment.