Skip to content

Commit c5a18e7

Browse files
Myles BorinsMylesBorins
Myles Borins
authored andcommitted
Revert "fs: validate args of truncate functions in js"
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: nodejs#2498 Reviewed-By: Trevor Norris <trev.norris@gmail.com> PR-URL: nodejs#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>
1 parent 320f433 commit c5a18e7

6 files changed

+118
-301
lines changed

lib/fs.js

+37-70
Original file line numberDiff line numberDiff line change
@@ -81,16 +81,6 @@ function throwOptionsError(options) {
8181
'but got ' + typeof options + ' instead');
8282
}
8383

84-
function isFD(fd) {
85-
return Number.isInteger(fd) && fd >= 0 && fd <= 0xFFFFFFFF;
86-
}
87-
88-
function sanitizeFD(fd) {
89-
if (!isFD(fd))
90-
throw new TypeError('file descriptor must be a unsigned 32-bit integer');
91-
return fd;
92-
}
93-
9484
// Ensure that callbacks run in the global context. Only use this function
9585
// for callbacks that are passed to the binding layer, callbacks that are
9686
// invoked from JS already run in the proper scope.
@@ -122,6 +112,10 @@ function nullCheck(path, callback) {
122112
return true;
123113
}
124114

115+
function isFd(path) {
116+
return (path >>> 0) === path;
117+
}
118+
125119
// Static method to set the stats properties on a Stats object.
126120
fs.Stats = function(
127121
dev,
@@ -263,7 +257,7 @@ fs.readFile = function(path, options, callback) {
263257
return;
264258

265259
var context = new ReadFileContext(callback, encoding);
266-
context.isUserFd = isFD(path); // file descriptor ownership
260+
context.isUserFd = isFd(path); // file descriptor ownership
267261
var req = new FSReqWrap();
268262
req.context = context;
269263
req.oncomplete = readFileAfterOpen;
@@ -479,7 +473,7 @@ fs.readFileSync = function(path, options) {
479473
assertEncoding(encoding);
480474

481475
var flag = options.flag || 'r';
482-
var isUserFd = isFD(path); // file descriptor ownership
476+
var isUserFd = isFd(path); // file descriptor ownership
483477
var fd = isUserFd ? path : fs.openSync(path, flag, 0o666);
484478

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

577571
fs.close = function(fd, callback) {
578572
var req = new FSReqWrap();
579-
req.oncomplete = makeCallback(callback);
580-
binding.close(sanitizeFD(fd), req);
573+
req.oncomplete = makeCallback(arguments[arguments.length - 1]);
574+
binding.close(fd, req);
581575
};
582576

583577
fs.closeSync = function(fd) {
584-
return binding.close(sanitizeFD(fd));
578+
return binding.close(fd);
585579
};
586580

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

682-
fd = sanitizeFD(fd);
683675
if (!(buffer instanceof Buffer)) {
684676
// legacy string interface (fd, length, position, encoding, callback)
685677
readSyncWarned = printDeprecation('fs.readSync\'s legacy String interface' +
@@ -721,7 +713,6 @@ fs.readSync = function(fd, buffer, offset, length, position) {
721713
// fs.write(fd, string[, position[, encoding]], callback);
722714
fs.write = function(fd, buffer, offset, length, position, callback) {
723715
callback = makeCallback(arguments[arguments.length - 1]);
724-
fd = sanitizeFD(fd);
725716
function wrapper(err, written) {
726717
// Retain a reference to buffer so that it can't be GC'ed too soon.
727718
callback(err, written || 0, buffer);
@@ -757,7 +748,6 @@ fs.write = function(fd, buffer, offset, length, position, callback) {
757748
// OR
758749
// fs.writeSync(fd, string[, position[, encoding]]);
759750
fs.writeSync = function(fd, buffer, offset, length, position) {
760-
fd = sanitizeFD(fd);
761751
if (buffer instanceof Buffer) {
762752
if (position === undefined)
763753
position = null;
@@ -789,18 +779,14 @@ fs.renameSync = function(oldPath, newPath) {
789779
};
790780

791781
fs.truncate = function(path, len, callback) {
792-
callback = makeCallback(arguments[arguments.length - 1]);
793-
794-
if (isFD(path))
782+
if (typeof path === 'number') {
795783
return fs.ftruncate(path, len, callback);
784+
}
796785

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

800-
if (typeof len === 'function' || len == undefined) {
788+
if (typeof len === 'function' || len === undefined) {
801789
len = 0;
802-
} else if (!Number.isInteger(len) || len < 0) {
803-
throw new TypeError('length must be a positive integer');
804790
}
805791

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

818804
fs.truncateSync = function(path, len) {
819-
if (isFD(path))
805+
if (typeof path === 'number') {
806+
// legacy
820807
return fs.ftruncateSync(path, len);
821-
822-
if (typeof path !== 'string')
823-
throw new TypeError('path must be a string');
824-
825-
if (len === undefined || len === null) {
808+
}
809+
if (len === undefined) {
826810
len = 0;
827-
} else if (!Number.isInteger(len) || len < 0) {
828-
throw new TypeError('length must be a positive integer');
829811
}
830-
831812
// allow error to be thrown, but still close fd.
832813
var fd = fs.openSync(path, 'r+');
833814
var ret;
@@ -841,30 +822,18 @@ fs.truncateSync = function(path, len) {
841822
};
842823

843824
fs.ftruncate = function(fd, len, callback) {
844-
callback = makeCallback(arguments[arguments.length - 1]);
845-
846-
fd = sanitizeFD(fd);
847-
848-
if (typeof len === 'function' || len == undefined) {
825+
if (typeof len === 'function' || len === undefined) {
849826
len = 0;
850-
} else if (!Number.isInteger(len) || len < 0) {
851-
throw new TypeError('length must be a positive integer');
852827
}
853-
854828
var req = new FSReqWrap();
855-
req.oncomplete = callback;
829+
req.oncomplete = makeCallback(arguments[arguments.length - 1]);
856830
binding.ftruncate(fd, len, req);
857831
};
858832

859833
fs.ftruncateSync = function(fd, len) {
860-
fd = sanitizeFD(fd);
861-
862-
if (len === undefined || len === null) {
834+
if (len === undefined) {
863835
len = 0;
864-
} else if (!Number.isInteger(len) || len < 0) {
865-
throw new TypeError('length must be a positive integer');
866836
}
867-
868837
return binding.ftruncate(fd, len);
869838
};
870839

@@ -884,21 +853,21 @@ fs.rmdirSync = function(path) {
884853
fs.fdatasync = function(fd, callback) {
885854
var req = new FSReqWrap();
886855
req.oncomplete = makeCallback(callback);
887-
binding.fdatasync(sanitizeFD(fd), req);
856+
binding.fdatasync(fd, req);
888857
};
889858

890859
fs.fdatasyncSync = function(fd) {
891-
return binding.fdatasync(sanitizeFD(fd));
860+
return binding.fdatasync(fd);
892861
};
893862

894863
fs.fsync = function(fd, callback) {
895864
var req = new FSReqWrap();
896-
req.oncomplete = makeCallback(callback);
897-
binding.fsync(sanitizeFD(fd), req);
865+
req.oncomplete = makeCallback(arguments[arguments.length - 1]);
866+
binding.fsync(fd, req);
898867
};
899868

900869
fs.fsyncSync = function(fd) {
901-
return binding.fsync(sanitizeFD(fd));
870+
return binding.fsync(fd);
902871
};
903872

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

947916
fs.fstat = function(fd, callback) {
948917
var req = new FSReqWrap();
949-
req.oncomplete = makeCallback(callback);
950-
binding.fstat(sanitizeFD(fd), req);
918+
req.oncomplete = makeCallback(arguments[arguments.length - 1]);
919+
binding.fstat(fd, req);
951920
};
952921

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

969938
fs.fstatSync = function(fd) {
970-
return binding.fstat(sanitizeFD(fd));
939+
return binding.fstat(fd);
971940
};
972941

973942
fs.lstatSync = function(path) {
@@ -1084,11 +1053,11 @@ fs.unlinkSync = function(path) {
10841053
fs.fchmod = function(fd, mode, callback) {
10851054
var req = new FSReqWrap();
10861055
req.oncomplete = makeCallback(arguments[arguments.length - 1]);
1087-
binding.fchmod(sanitizeFD(fd), modeNum(mode), req);
1056+
binding.fchmod(fd, modeNum(mode), req);
10881057
};
10891058

10901059
fs.fchmodSync = function(fd, mode) {
1091-
return binding.fchmod(sanitizeFD(fd), modeNum(mode));
1060+
return binding.fchmod(fd, modeNum(mode));
10921061
};
10931062

10941063
if (constants.hasOwnProperty('O_SYMLINK')) {
@@ -1167,11 +1136,11 @@ if (constants.hasOwnProperty('O_SYMLINK')) {
11671136
fs.fchown = function(fd, uid, gid, callback) {
11681137
var req = new FSReqWrap();
11691138
req.oncomplete = makeCallback(arguments[arguments.length - 1]);
1170-
binding.fchown(sanitizeFD(fd), uid, gid, req);
1139+
binding.fchown(fd, uid, gid, req);
11711140
};
11721141

11731142
fs.fchownSync = function(fd, uid, gid) {
1174-
return binding.fchown(sanitizeFD(fd), uid, gid);
1143+
return binding.fchown(fd, uid, gid);
11751144
};
11761145

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

12291198
fs.futimes = function(fd, atime, mtime, callback) {
12301199
callback = makeCallback(arguments[arguments.length - 1]);
1231-
fd = sanitizeFD(fd);
12321200
atime = toUnixTimestamp(atime);
12331201
mtime = toUnixTimestamp(mtime);
12341202
var req = new FSReqWrap();
@@ -1237,7 +1205,6 @@ fs.futimes = function(fd, atime, mtime, callback) {
12371205
};
12381206

12391207
fs.futimesSync = function(fd, atime, mtime) {
1240-
fd = sanitizeFD(fd);
12411208
atime = toUnixTimestamp(atime);
12421209
mtime = toUnixTimestamp(mtime);
12431210
binding.futimes(fd, atime, mtime);
@@ -1290,7 +1257,7 @@ fs.writeFile = function(path, data, options, callback) {
12901257

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

1293-
if (isFD(path)) {
1260+
if (isFd(path)) {
12941261
writeFd(path, true);
12951262
return;
12961263
}
@@ -1324,7 +1291,7 @@ fs.writeFileSync = function(path, data, options) {
13241291
assertEncoding(options.encoding);
13251292

13261293
var flag = options.flag || 'w';
1327-
var isUserFd = isFD(path); // file descriptor ownership
1294+
var isUserFd = isFd(path); // file descriptor ownership
13281295
var fd = isUserFd ? path : fs.openSync(path, flag, options.mode);
13291296

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

13641331
// force append behavior when using a supplied file descriptor
1365-
if (isFD(path))
1332+
if (isFd(path))
13661333
options.flag = 'a';
13671334

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

13831350
// force append behavior when using a supplied file descriptor
1384-
if (isFD(path))
1351+
if (isFd(path))
13851352
options.flag = 'a';
13861353

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

19661933
options = options || {};
19671934

1968-
this.fd = sanitizeFD(fd);
1935+
this.fd = fd;
19691936
this.writable = true;
19701937
this.readable = false;
19711938
this.autoClose = options.autoClose === undefined ? true : options.autoClose;

0 commit comments

Comments
 (0)