Skip to content

Commit

Permalink
Revert "fs: validate args of truncate functions in js"
Browse files Browse the repository at this point in the history 
This reverts commit c86c1ee.

original commit message:

    This patch

     1. moves the basic validation of arguments to `truncate` family
        of functions to the JavaScript layer from the C++ layer.

     2. makes sure that the File Descriptors are validated strictly.

    PR-URL: #2498
    Reviewed-By: Trevor Norris <trev.norris@gmail.com>

PR-URL: #7950
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
@MylesBorins
Myles Borins authored and MylesBorins committed Aug 4, 2016
1 parent 320f433 commit c5a18e7
Show file tree
Hide file tree
Showing 6 changed files with 118 additions and 301 deletions.
107 changes: 37 additions & 70 deletions lib/fs.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,16 +81,6 @@ function throwOptionsError(options) {
'but got ' + typeof options + ' instead');
}

function isFD(fd) {
return Number.isInteger(fd) && fd >= 0 && fd <= 0xFFFFFFFF;
}

function sanitizeFD(fd) {
if (!isFD(fd))
throw new TypeError('file descriptor must be a unsigned 32-bit integer');
return fd;
}

// Ensure that callbacks run in the global context. Only use this function
// for callbacks that are passed to the binding layer, callbacks that are
// invoked from JS already run in the proper scope.
Expand Down Expand Up @@ -122,6 +112,10 @@ function nullCheck(path, callback) {
return true;
}

function isFd(path) {
return (path >>> 0) === path;
}

// Static method to set the stats properties on a Stats object.
fs.Stats = function(
dev,
Expand Down Expand Up @@ -263,7 +257,7 @@ fs.readFile = function(path, options, callback) {
return;

var context = new ReadFileContext(callback, encoding);
context.isUserFd = isFD(path); // file descriptor ownership
context.isUserFd = isFd(path); // file descriptor ownership
var req = new FSReqWrap();
req.context = context;
req.oncomplete = readFileAfterOpen;
Expand Down Expand Up @@ -479,7 +473,7 @@ fs.readFileSync = function(path, options) {
assertEncoding(encoding);

var flag = options.flag || 'r';
var isUserFd = isFD(path); // file descriptor ownership
var isUserFd = isFd(path); // file descriptor ownership
var fd = isUserFd ? path : fs.openSync(path, flag, 0o666);

var st = tryStatSync(fd, isUserFd);
Expand Down Expand Up @@ -576,12 +570,12 @@ Object.defineProperty(exports, '_stringToFlags', {

fs.close = function(fd, callback) {
var req = new FSReqWrap();
req.oncomplete = makeCallback(callback);
binding.close(sanitizeFD(fd), req);
req.oncomplete = makeCallback(arguments[arguments.length - 1]);
binding.close(fd, req);
};

fs.closeSync = function(fd) {
return binding.close(sanitizeFD(fd));
return binding.close(fd);
};

function modeNum(m, def) {
Expand Down Expand Up @@ -618,7 +612,6 @@ fs.openSync = function(path, flags, mode) {
var readWarned = false;
fs.read = function(fd, buffer, offset, length, position, callback) {
callback = makeCallback(arguments[arguments.length - 1]);
fd = sanitizeFD(fd);
if (!(buffer instanceof Buffer)) {
// legacy string interface (fd, length, position, encoding, callback)
readWarned = printDeprecation('fs.read\'s legacy String interface ' +
Expand Down Expand Up @@ -679,7 +672,6 @@ fs.readSync = function(fd, buffer, offset, length, position) {
var legacy = false;
var encoding;

fd = sanitizeFD(fd);
if (!(buffer instanceof Buffer)) {
// legacy string interface (fd, length, position, encoding, callback)
readSyncWarned = printDeprecation('fs.readSync\'s legacy String interface' +
Expand Down Expand Up @@ -721,7 +713,6 @@ fs.readSync = function(fd, buffer, offset, length, position) {
// fs.write(fd, string[, position[, encoding]], callback);
fs.write = function(fd, buffer, offset, length, position, callback) {
callback = makeCallback(arguments[arguments.length - 1]);
fd = sanitizeFD(fd);
function wrapper(err, written) {
// Retain a reference to buffer so that it can't be GC'ed too soon.
callback(err, written || 0, buffer);
Expand Down Expand Up @@ -757,7 +748,6 @@ fs.write = function(fd, buffer, offset, length, position, callback) {
// OR
// fs.writeSync(fd, string[, position[, encoding]]);
fs.writeSync = function(fd, buffer, offset, length, position) {
fd = sanitizeFD(fd);
if (buffer instanceof Buffer) {
if (position === undefined)
position = null;
Expand Down Expand Up @@ -789,18 +779,14 @@ fs.renameSync = function(oldPath, newPath) {
};

fs.truncate = function(path, len, callback) {
callback = makeCallback(arguments[arguments.length - 1]);

if (isFD(path))
if (typeof path === 'number') {
return fs.ftruncate(path, len, callback);
}

if (typeof path !== 'string')
throw new TypeError('path must be a string');
callback = makeCallback(arguments[arguments.length - 1]);

if (typeof len === 'function' || len == undefined) {
if (typeof len === 'function' || len === undefined) {
len = 0;
} else if (!Number.isInteger(len) || len < 0) {
throw new TypeError('length must be a positive integer');
}

fs.open(path, 'r+', function(er, fd) {
Expand All @@ -816,18 +802,13 @@ fs.truncate = function(path, len, callback) {
};

fs.truncateSync = function(path, len) {
if (isFD(path))
if (typeof path === 'number') {
// legacy
return fs.ftruncateSync(path, len);

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

if (len === undefined || len === null) {
}
if (len === undefined) {
len = 0;
} else if (!Number.isInteger(len) || len < 0) {
throw new TypeError('length must be a positive integer');
}

// allow error to be thrown, but still close fd.
var fd = fs.openSync(path, 'r+');
var ret;
Expand All @@ -841,30 +822,18 @@ fs.truncateSync = function(path, len) {
};

fs.ftruncate = function(fd, len, callback) {
callback = makeCallback(arguments[arguments.length - 1]);

fd = sanitizeFD(fd);

if (typeof len === 'function' || len == undefined) {
if (typeof len === 'function' || len === undefined) {
len = 0;
} else if (!Number.isInteger(len) || len < 0) {
throw new TypeError('length must be a positive integer');
}

var req = new FSReqWrap();
req.oncomplete = callback;
req.oncomplete = makeCallback(arguments[arguments.length - 1]);
binding.ftruncate(fd, len, req);
};

fs.ftruncateSync = function(fd, len) {
fd = sanitizeFD(fd);

if (len === undefined || len === null) {
if (len === undefined) {
len = 0;
} else if (!Number.isInteger(len) || len < 0) {
throw new TypeError('length must be a positive integer');
}

return binding.ftruncate(fd, len);
};

Expand All @@ -884,21 +853,21 @@ fs.rmdirSync = function(path) {
fs.fdatasync = function(fd, callback) {
var req = new FSReqWrap();
req.oncomplete = makeCallback(callback);
binding.fdatasync(sanitizeFD(fd), req);
binding.fdatasync(fd, req);
};

fs.fdatasyncSync = function(fd) {
return binding.fdatasync(sanitizeFD(fd));
return binding.fdatasync(fd);
};

fs.fsync = function(fd, callback) {
var req = new FSReqWrap();
req.oncomplete = makeCallback(callback);
binding.fsync(sanitizeFD(fd), req);
req.oncomplete = makeCallback(arguments[arguments.length - 1]);
binding.fsync(fd, req);
};

fs.fsyncSync = function(fd) {
return binding.fsync(sanitizeFD(fd));
return binding.fsync(fd);
};

fs.mkdir = function(path, mode, callback) {
Expand Down Expand Up @@ -946,8 +915,8 @@ fs.readdirSync = function(path, options) {

fs.fstat = function(fd, callback) {
var req = new FSReqWrap();
req.oncomplete = makeCallback(callback);
binding.fstat(sanitizeFD(fd), req);
req.oncomplete = makeCallback(arguments[arguments.length - 1]);
binding.fstat(fd, req);
};

fs.lstat = function(path, callback) {
Expand All @@ -967,7 +936,7 @@ fs.stat = function(path, callback) {
};

fs.fstatSync = function(fd) {
return binding.fstat(sanitizeFD(fd));
return binding.fstat(fd);
};

fs.lstatSync = function(path) {
Expand Down Expand Up @@ -1084,11 +1053,11 @@ fs.unlinkSync = function(path) {
fs.fchmod = function(fd, mode, callback) {
var req = new FSReqWrap();
req.oncomplete = makeCallback(arguments[arguments.length - 1]);
binding.fchmod(sanitizeFD(fd), modeNum(mode), req);
binding.fchmod(fd, modeNum(mode), req);
};

fs.fchmodSync = function(fd, mode) {
return binding.fchmod(sanitizeFD(fd), modeNum(mode));
return binding.fchmod(fd, modeNum(mode));
};

if (constants.hasOwnProperty('O_SYMLINK')) {
Expand Down Expand Up @@ -1167,11 +1136,11 @@ if (constants.hasOwnProperty('O_SYMLINK')) {
fs.fchown = function(fd, uid, gid, callback) {
var req = new FSReqWrap();
req.oncomplete = makeCallback(arguments[arguments.length - 1]);
binding.fchown(sanitizeFD(fd), uid, gid, req);
binding.fchown(fd, uid, gid, req);
};

fs.fchownSync = function(fd, uid, gid) {
return binding.fchown(sanitizeFD(fd), uid, gid);
return binding.fchown(fd, uid, gid);
};

fs.chown = function(path, uid, gid, callback) {
Expand Down Expand Up @@ -1228,7 +1197,6 @@ fs.utimesSync = function(path, atime, mtime) {

fs.futimes = function(fd, atime, mtime, callback) {
callback = makeCallback(arguments[arguments.length - 1]);
fd = sanitizeFD(fd);
atime = toUnixTimestamp(atime);
mtime = toUnixTimestamp(mtime);
var req = new FSReqWrap();
Expand All @@ -1237,7 +1205,6 @@ fs.futimes = function(fd, atime, mtime, callback) {
};

fs.futimesSync = function(fd, atime, mtime) {
fd = sanitizeFD(fd);
atime = toUnixTimestamp(atime);
mtime = toUnixTimestamp(mtime);
binding.futimes(fd, atime, mtime);
Expand Down Expand Up @@ -1290,7 +1257,7 @@ fs.writeFile = function(path, data, options, callback) {

var flag = options.flag || 'w';

if (isFD(path)) {
if (isFd(path)) {
writeFd(path, true);
return;
}
Expand Down Expand Up @@ -1324,7 +1291,7 @@ fs.writeFileSync = function(path, data, options) {
assertEncoding(options.encoding);

var flag = options.flag || 'w';
var isUserFd = isFD(path); // file descriptor ownership
var isUserFd = isFd(path); // file descriptor ownership
var fd = isUserFd ? path : fs.openSync(path, flag, options.mode);

if (!(data instanceof Buffer)) {
Expand Down Expand Up @@ -1362,7 +1329,7 @@ fs.appendFile = function(path, data, options, callback) {
options = util._extend({ flag: 'a' }, options);

// force append behavior when using a supplied file descriptor
if (isFD(path))
if (isFd(path))
options.flag = 'a';

fs.writeFile(path, data, options, callback);
Expand All @@ -1381,7 +1348,7 @@ fs.appendFileSync = function(path, data, options) {
options = util._extend({ flag: 'a' }, options);

// force append behavior when using a supplied file descriptor
if (isFD(path))
if (isFd(path))
options.flag = 'a';

fs.writeFileSync(path, data, options);
Expand Down Expand Up @@ -1965,7 +1932,7 @@ function SyncWriteStream(fd, options) {

options = options || {};

this.fd = sanitizeFD(fd);
this.fd = fd;
this.writable = true;
this.readable = false;
this.autoClose = options.autoClose === undefined ? true : options.autoClose;
Expand Down
Loading

0 comments on commit c5a18e7

Please sign in to comment.