Skip to content

Commit 8e76397

Browse files
committed
fs: validate encoding to binding.writeString()
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 <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Anto Aravinth <[email protected]>
1 parent 28bca33 commit 8e76397

File tree

4 files changed

+53
-1
lines changed

4 files changed

+53
-1
lines changed

Diff for: lib/fs.js

+6-1
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ const {
133133
validateBoolean,
134134
validateBuffer,
135135
validateCallback,
136+
validateEncoding,
136137
validateFunction,
137138
validateInteger,
138139
} = require('internal/validators');
@@ -702,11 +703,14 @@ function write(fd, buffer, offset, length, position, callback) {
702703
}
703704
length = 'utf8';
704705
}
706+
707+
const str = String(buffer);
708+
validateEncoding(str, length);
705709
callback = maybeCallback(position);
706710

707711
const req = new FSReqCallback();
708712
req.oncomplete = wrapper;
709-
return binding.writeString(fd, String(buffer), offset, length, req);
713+
return binding.writeString(fd, str, offset, length, req);
710714
}
711715

712716
ObjectDefineProperty(write, internalUtil.customPromisifyArgs,
@@ -735,6 +739,7 @@ function writeSync(fd, buffer, offset, length, position) {
735739
undefined, ctx);
736740
} else {
737741
validateStringAfterArrayBufferView(buffer, 'buffer');
742+
validateEncoding(buffer, length);
738743

739744
if (offset === undefined)
740745
offset = null;

Diff for: lib/internal/fs/promises.js

+2
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ const {
7070
validateAbortSignal,
7171
validateBoolean,
7272
validateBuffer,
73+
validateEncoding,
7374
validateInteger,
7475
} = require('internal/validators');
7576
const pathModule = require('path');
@@ -467,6 +468,7 @@ async function write(handle, buffer, offset, length, position) {
467468
}
468469

469470
validateStringAfterArrayBufferView(buffer, 'buffer');
471+
validateEncoding(buffer, length);
470472
const bytesWritten = (await binding.writeString(handle.fd, buffer, offset,
471473
length, kUsePromises)) || 0;
472474
return { bytesWritten, buffer };

Diff for: test/parallel/test-fs-promises.js

+16
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,22 @@ async function getHandle(dest) {
436436
);
437437
}
438438

439+
// Regression test for https://github.com/nodejs/node/issues/38168
440+
{
441+
const handle = await getHandle(dest);
442+
443+
assert.rejects(
444+
async () => handle.write('abc', 0, 'hex'),
445+
{
446+
code: 'ERR_INVALID_ARG_VALUE',
447+
message: /'encoding' is invalid for data of length 3/
448+
}
449+
);
450+
451+
const ret = await handle.write('abcd', 0, 'hex');
452+
assert.strictEqual(ret.bytesWritten, 2);
453+
await handle.close();
454+
}
439455
}
440456

441457
doTest().then(common.mustCall());

Diff for: test/parallel/test-fs-write.js

+29
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ const fn = path.join(tmpdir.path, 'write.txt');
3333
const fn2 = path.join(tmpdir.path, 'write2.txt');
3434
const fn3 = path.join(tmpdir.path, 'write3.txt');
3535
const fn4 = path.join(tmpdir.path, 'write4.txt');
36+
const fn5 = path.join(tmpdir.path, 'write5.txt');
3637
const expected = 'ümlaut.';
3738
const constants = fs.constants;
3839

@@ -170,3 +171,31 @@ fs.open(fn4, 'w', 0o644, common.mustSucceed((fd) => {
170171
}
171172
);
172173
});
174+
175+
{
176+
// Regression test for https://github.com/nodejs/node/issues/38168
177+
const fd = fs.openSync(fn5, 'w');
178+
179+
assert.throws(
180+
() => fs.writeSync(fd, 'abc', 0, 'hex'),
181+
{
182+
code: 'ERR_INVALID_ARG_VALUE',
183+
message: /'encoding' is invalid for data of length 3/
184+
}
185+
);
186+
187+
assert.throws(
188+
() => fs.writeSync(fd, 'abc', 0, 'hex', common.mustNotCall()),
189+
{
190+
code: 'ERR_INVALID_ARG_VALUE',
191+
message: /'encoding' is invalid for data of length 3/
192+
}
193+
);
194+
195+
assert.strictEqual(fs.writeSync(fd, 'abcd', 0, 'hex'), 2);
196+
197+
fs.write(fd, 'abcd', 0, 'hex', common.mustSucceed((written) => {
198+
assert.strictEqual(written, 2);
199+
fs.closeSync(fd);
200+
}));
201+
}

0 commit comments

Comments
 (0)