Skip to content

Commit

Permalink
fs: validate encoding to binding.writeString()
Browse files Browse the repository at this point in the history
The binding layer performs some validation of the encoding and
data passed to WriteString(). This commit adds similar validation
to the JS layer for better error handling.

PR-URL: #38183
Fixes: #38168
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
  • Loading branch information
cjihrig committed Apr 12, 2021
1 parent 28bca33 commit 8e76397
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 1 deletion.
7 changes: 6 additions & 1 deletion lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ const {
validateBoolean,
validateBuffer,
validateCallback,
validateEncoding,
validateFunction,
validateInteger,
} = require('internal/validators');
Expand Down Expand Up @@ -702,11 +703,14 @@ function write(fd, buffer, offset, length, position, callback) {
}
length = 'utf8';
}

const str = String(buffer);
validateEncoding(str, length);
callback = maybeCallback(position);

const req = new FSReqCallback();
req.oncomplete = wrapper;
return binding.writeString(fd, String(buffer), offset, length, req);
return binding.writeString(fd, str, offset, length, req);
}

ObjectDefineProperty(write, internalUtil.customPromisifyArgs,
Expand Down Expand Up @@ -735,6 +739,7 @@ function writeSync(fd, buffer, offset, length, position) {
undefined, ctx);
} else {
validateStringAfterArrayBufferView(buffer, 'buffer');
validateEncoding(buffer, length);

if (offset === undefined)
offset = null;
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ const {
validateAbortSignal,
validateBoolean,
validateBuffer,
validateEncoding,
validateInteger,
} = require('internal/validators');
const pathModule = require('path');
Expand Down Expand Up @@ -467,6 +468,7 @@ async function write(handle, buffer, offset, length, position) {
}

validateStringAfterArrayBufferView(buffer, 'buffer');
validateEncoding(buffer, length);
const bytesWritten = (await binding.writeString(handle.fd, buffer, offset,
length, kUsePromises)) || 0;
return { bytesWritten, buffer };
Expand Down
16 changes: 16 additions & 0 deletions test/parallel/test-fs-promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,22 @@ async function getHandle(dest) {
);
}

// Regression test for https://github.com/nodejs/node/issues/38168
{
const handle = await getHandle(dest);

assert.rejects(
async () => handle.write('abc', 0, 'hex'),
{
code: 'ERR_INVALID_ARG_VALUE',
message: /'encoding' is invalid for data of length 3/
}
);

const ret = await handle.write('abcd', 0, 'hex');
assert.strictEqual(ret.bytesWritten, 2);
await handle.close();
}
}

doTest().then(common.mustCall());
Expand Down
29 changes: 29 additions & 0 deletions test/parallel/test-fs-write.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const fn = path.join(tmpdir.path, 'write.txt');
const fn2 = path.join(tmpdir.path, 'write2.txt');
const fn3 = path.join(tmpdir.path, 'write3.txt');
const fn4 = path.join(tmpdir.path, 'write4.txt');
const fn5 = path.join(tmpdir.path, 'write5.txt');
const expected = 'ümlaut.';
const constants = fs.constants;

Expand Down Expand Up @@ -170,3 +171,31 @@ fs.open(fn4, 'w', 0o644, common.mustSucceed((fd) => {
}
);
});

{
// Regression test for https://github.com/nodejs/node/issues/38168
const fd = fs.openSync(fn5, 'w');

assert.throws(
() => fs.writeSync(fd, 'abc', 0, 'hex'),
{
code: 'ERR_INVALID_ARG_VALUE',
message: /'encoding' is invalid for data of length 3/
}
);

assert.throws(
() => fs.writeSync(fd, 'abc', 0, 'hex', common.mustNotCall()),
{
code: 'ERR_INVALID_ARG_VALUE',
message: /'encoding' is invalid for data of length 3/
}
);

assert.strictEqual(fs.writeSync(fd, 'abcd', 0, 'hex'), 2);

fs.write(fd, 'abcd', 0, 'hex', common.mustSucceed((written) => {
assert.strictEqual(written, 2);
fs.closeSync(fd);
}));
}

0 comments on commit 8e76397

Please sign in to comment.