From 41ca10aca751b8c67f0cfd86f8874d698e55c1f7 Mon Sep 17 00:00:00 2001 From: Igor Halfeld Date: Wed, 14 Oct 2020 13:20:21 -0300 Subject: [PATCH 1/6] fix ftruncate for negative and maximum values --- doc/api/errors.md | 5 +++++ lib/fs.js | 9 ++++++++- lib/internal/errors.js | 1 + 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index d0e0b8794f09ce..4685f9d527a136 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1074,6 +1074,11 @@ Path is a directory. An attempt has been made to read a file whose size is larger than the maximum allowed size for a `Buffer`. + +### `ERR_FS_FILE_TOO_SMALL` + +An attempt has been made to read a file whose size is a negative value + ### `ERR_FS_INVALID_SYMLINK_TYPE` diff --git a/lib/fs.js b/lib/fs.js index 39f35371393f4a..5d97b7b155cb67 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -73,6 +73,7 @@ const { Buffer } = require('buffer'); const { codes: { ERR_FS_FILE_TOO_LARGE, + ERR_FS_FILE_TOO_SMALL, ERR_INVALID_ARG_VALUE, ERR_INVALID_ARG_TYPE, ERR_INVALID_CALLBACK, @@ -856,7 +857,13 @@ function ftruncate(fd, len = 0, callback) { function ftruncateSync(fd, len = 0) { validateInt32(fd, 'fd', 0); validateInteger(len, 'len'); - len = MathMax(0, len); + + if (len < 0) { + throw new ERR_FS_FILE_TOO_SMALL(len); + } else if (len > kIoMaxLength) { + throw new ERR_FS_FILE_TOO_LARGE(len); + } + const ctx = {}; binding.ftruncate(fd, len, undefined, ctx); handleErrorFromBinding(ctx); diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 8a7e744c5fe667..02d53e56c28dea 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -866,6 +866,7 @@ E('ERR_FEATURE_UNAVAILABLE_ON_PLATFORM', TypeError); E('ERR_FS_EISDIR', 'Path is a directory', SystemError); E('ERR_FS_FILE_TOO_LARGE', 'File size (%s) is greater than 2 GB', RangeError); +E('ERR_FS_FILE_TOO_SMALL', 'File size (%s) is a negative value', RangeError); E('ERR_FS_INVALID_SYMLINK_TYPE', 'Symlink type must be one of "dir", "file", or "junction". Received "%s"', Error); // Switch to TypeError. The current implementation does not seem right From 96947836edd7a5c80c6c773ec287d290cefae003 Mon Sep 17 00:00:00 2001 From: Igor Halfeld Date: Wed, 14 Oct 2020 14:06:18 -0300 Subject: [PATCH 2/6] put same validation for async ftruncate --- lib/fs.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/fs.js b/lib/fs.js index 5d97b7b155cb67..0ed3dbe16fb0cd 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -846,7 +846,13 @@ function ftruncate(fd, len = 0, callback) { } validateInt32(fd, 'fd', 0); validateInteger(len, 'len'); - len = MathMax(0, len); + + if (len < 0) { + throw new ERR_FS_FILE_TOO_SMALL(len); + } else if (len > kIoMaxLength) { + throw new ERR_FS_FILE_TOO_LARGE(len); + } + callback = makeCallback(callback); const req = new FSReqCallback(); From e8617766303fbdc15d77f91df9e639ae2b168693 Mon Sep 17 00:00:00 2001 From: Igor Halfeld Date: Wed, 14 Oct 2020 14:17:09 -0300 Subject: [PATCH 3/6] remove unused var MathMax from lib/fs.js --- lib/fs.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/fs.js b/lib/fs.js index 0ed3dbe16fb0cd..62e5039fec763a 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -35,7 +35,6 @@ const kIoMaxLength = 2 ** 31 - 1; const { ArrayPrototypePush, BigIntPrototypeToString, - MathMax, Number, NumberIsSafeInteger, ObjectCreate, From 7688f6c75234abece33c4c55bc0d2437bbe4069c Mon Sep 17 00:00:00 2001 From: Igor Halfeld Date: Wed, 14 Oct 2020 17:34:39 -0300 Subject: [PATCH 4/6] change error type when parameter is not valid for ftruncate --- doc/api/errors.md | 5 ----- lib/fs.js | 9 ++------- lib/internal/errors.js | 1 - 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index 4685f9d527a136..d0e0b8794f09ce 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1074,11 +1074,6 @@ Path is a directory. An attempt has been made to read a file whose size is larger than the maximum allowed size for a `Buffer`. - -### `ERR_FS_FILE_TOO_SMALL` - -An attempt has been made to read a file whose size is a negative value - ### `ERR_FS_INVALID_SYMLINK_TYPE` diff --git a/lib/fs.js b/lib/fs.js index 62e5039fec763a..c803aef366b495 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -72,7 +72,6 @@ const { Buffer } = require('buffer'); const { codes: { ERR_FS_FILE_TOO_LARGE, - ERR_FS_FILE_TOO_SMALL, ERR_INVALID_ARG_VALUE, ERR_INVALID_ARG_TYPE, ERR_INVALID_CALLBACK, @@ -847,9 +846,7 @@ function ftruncate(fd, len = 0, callback) { validateInteger(len, 'len'); if (len < 0) { - throw new ERR_FS_FILE_TOO_SMALL(len); - } else if (len > kIoMaxLength) { - throw new ERR_FS_FILE_TOO_LARGE(len); + throw new ERR_INVALID_ARG_VALUE(len); } callback = makeCallback(callback); @@ -864,9 +861,7 @@ function ftruncateSync(fd, len = 0) { validateInteger(len, 'len'); if (len < 0) { - throw new ERR_FS_FILE_TOO_SMALL(len); - } else if (len > kIoMaxLength) { - throw new ERR_FS_FILE_TOO_LARGE(len); + throw new ERR_INVALID_ARG_VALUE(len); } const ctx = {}; diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 02d53e56c28dea..8a7e744c5fe667 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -866,7 +866,6 @@ E('ERR_FEATURE_UNAVAILABLE_ON_PLATFORM', TypeError); E('ERR_FS_EISDIR', 'Path is a directory', SystemError); E('ERR_FS_FILE_TOO_LARGE', 'File size (%s) is greater than 2 GB', RangeError); -E('ERR_FS_FILE_TOO_SMALL', 'File size (%s) is a negative value', RangeError); E('ERR_FS_INVALID_SYMLINK_TYPE', 'Symlink type must be one of "dir", "file", or "junction". Received "%s"', Error); // Switch to TypeError. The current implementation does not seem right From ffb18618b34a89d3751627c5da26754142eac6da Mon Sep 17 00:00:00 2001 From: Igor Halfeld Date: Wed, 14 Oct 2020 22:51:00 -0300 Subject: [PATCH 5/6] put required arg on ERR_INVALID_ARG_VALUE --- lib/fs.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index c803aef366b495..118d7fe800cbfa 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -846,7 +846,7 @@ function ftruncate(fd, len = 0, callback) { validateInteger(len, 'len'); if (len < 0) { - throw new ERR_INVALID_ARG_VALUE(len); + throw new ERR_INVALID_ARG_VALUE('len', len); } callback = makeCallback(callback); @@ -861,7 +861,7 @@ function ftruncateSync(fd, len = 0) { validateInteger(len, 'len'); if (len < 0) { - throw new ERR_INVALID_ARG_VALUE(len); + throw new ERR_INVALID_ARG_VALUE('len', len); } const ctx = {}; From e04fd9befce0dd7a81dea5319922d15302cfe94d Mon Sep 17 00:00:00 2001 From: Igor Halfeld Date: Thu, 15 Oct 2020 00:22:02 -0300 Subject: [PATCH 6/6] fix tests for ftruncate --- test/parallel/test-fs-truncate.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/test/parallel/test-fs-truncate.js b/test/parallel/test-fs-truncate.js index 418d56c047d4e3..e5965a5b7a7b1f 100644 --- a/test/parallel/test-fs-truncate.js +++ b/test/parallel/test-fs-truncate.js @@ -224,13 +224,14 @@ function testFtruncate(cb) { } { - const file6 = path.resolve(tmp, 'truncate-file-6.txt'); - fs.writeFileSync(file6, 'Hi'); - const fd = fs.openSync(file6, 'r+'); - process.on('beforeExit', () => fs.closeSync(fd)); - fs.ftruncate(fd, -1, common.mustSucceed(() => { - assert(fs.readFileSync(file6).equals(Buffer.from(''))); - })); + assert.throws( + () => fs.ftruncate(fd, -1), + { + code: 'ERR_INVALID_ARG_VALUE', + name: /(TypeError|RangeError)/, + message: 'The argument \'len\' is invalid. Received -1' + } + ); } {