Skip to content

Commit 994320b

Browse files
committed
fs: throw writeSync errors in JS
PR-URL: #19041 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 1650eae commit 994320b

File tree

3 files changed

+93
-30
lines changed

3 files changed

+93
-30
lines changed

lib/fs.js

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,8 @@ Object.defineProperty(fs.write, internalUtil.customPromisifyArgs,
635635
// fs.writeSync(fd, string[, position[, encoding]]);
636636
fs.writeSync = function(fd, buffer, offset, length, position) {
637637
validateUint32(fd, 'fd');
638+
const ctx = {};
639+
let result;
638640
if (isUint8Array(buffer)) {
639641
if (position === undefined)
640642
position = null;
@@ -643,13 +645,18 @@ fs.writeSync = function(fd, buffer, offset, length, position) {
643645
if (typeof length !== 'number')
644646
length = buffer.length - offset;
645647
validateOffsetLengthWrite(offset, length, buffer.byteLength);
646-
return binding.writeBuffer(fd, buffer, offset, length, position);
648+
result = binding.writeBuffer(fd, buffer, offset, length, position,
649+
undefined, ctx);
650+
} else {
651+
if (typeof buffer !== 'string')
652+
buffer += '';
653+
if (offset === undefined)
654+
offset = null;
655+
result = binding.writeString(fd, buffer, offset, length,
656+
undefined, ctx);
647657
}
648-
if (typeof buffer !== 'string')
649-
buffer += '';
650-
if (offset === undefined)
651-
offset = null;
652-
return binding.writeString(fd, buffer, offset, length, position);
658+
handleErrorFromBinding(ctx);
659+
return result;
653660
};
654661

655662
fs.rename = function(oldPath, newPath, callback) {

src/node_file.cc

Lines changed: 39 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,35 +1270,43 @@ static void CopyFile(const FunctionCallbackInfo<Value>& args) {
12701270
static void WriteBuffer(const FunctionCallbackInfo<Value>& args) {
12711271
Environment* env = Environment::GetCurrent(args);
12721272

1273+
const int argc = args.Length();
1274+
CHECK_GE(argc, 4);
1275+
12731276
CHECK(args[0]->IsInt32());
1274-
CHECK(Buffer::HasInstance(args[1]));
1277+
const int fd = args[0].As<Int32>()->Value();
12751278

1276-
int fd = args[0]->Int32Value();
1277-
Local<Object> obj = args[1].As<Object>();
1278-
const char* buf = Buffer::Data(obj);
1279-
size_t buffer_length = Buffer::Length(obj);
1280-
size_t off = args[2]->Uint32Value();
1281-
size_t len = args[3]->Uint32Value();
1282-
int64_t pos = GET_OFFSET(args[4]);
1279+
CHECK(Buffer::HasInstance(args[1]));
1280+
Local<Object> buffer_obj = args[1].As<Object>();
1281+
char* buffer_data = Buffer::Data(buffer_obj);
1282+
size_t buffer_length = Buffer::Length(buffer_obj);
12831283

1284+
CHECK(args[2]->IsInt32());
1285+
const size_t off = static_cast<size_t>(args[2].As<Int32>()->Value());
12841286
CHECK_LE(off, buffer_length);
1287+
1288+
CHECK(args[3]->IsInt32());
1289+
const size_t len = static_cast<size_t>(args[3].As<Int32>()->Value());
1290+
CHECK(Buffer::IsWithinBounds(off, len, buffer_length));
12851291
CHECK_LE(len, buffer_length);
12861292
CHECK_GE(off + len, off);
1287-
CHECK(Buffer::IsWithinBounds(off, len, buffer_length));
12881293

1289-
buf += off;
1294+
const int64_t pos = GET_OFFSET(args[4]);
12901295

1296+
char* buf = buffer_data + off;
12911297
uv_buf_t uvbuf = uv_buf_init(const_cast<char*>(buf), len);
12921298

12931299
FSReqBase* req_wrap = GetReqWrap(env, args[5]);
1294-
if (req_wrap != nullptr) {
1300+
if (req_wrap != nullptr) { // write(fd, buffer, off, len, pos, req)
12951301
AsyncCall(env, req_wrap, args, "write", UTF8, AfterInteger,
12961302
uv_fs_write, fd, &uvbuf, 1, pos);
1297-
return;
1303+
} else { // write(fd, buffer, off, len, pos, undefined, ctx)
1304+
CHECK_EQ(argc, 7);
1305+
fs_req_wrap req_wrap;
1306+
int bytesWritten = SyncCall(env, args[6], &req_wrap, "write",
1307+
uv_fs_write, fd, &uvbuf, 1, pos);
1308+
args.GetReturnValue().Set(bytesWritten);
12981309
}
1299-
1300-
SYNC_CALL(write, nullptr, fd, &uvbuf, 1, pos)
1301-
args.GetReturnValue().Set(SYNC_RESULT);
13021310
}
13031311

13041312

@@ -1350,19 +1358,23 @@ static void WriteBuffers(const FunctionCallbackInfo<Value>& args) {
13501358
static void WriteString(const FunctionCallbackInfo<Value>& args) {
13511359
Environment* env = Environment::GetCurrent(args);
13521360

1361+
const int argc = args.Length();
1362+
CHECK_GE(argc, 4);
1363+
13531364
CHECK(args[0]->IsInt32());
1365+
const int fd = args[0].As<Int32>()->Value();
1366+
1367+
const int64_t pos = GET_OFFSET(args[2]);
1368+
1369+
const auto enc = ParseEncoding(env->isolate(), args[3], UTF8);
13541370

13551371
std::unique_ptr<char[]> delete_on_return;
1356-
Local<Value> req;
13571372
Local<Value> value = args[1];
1358-
int fd = args[0]->Int32Value();
13591373
char* buf = nullptr;
13601374
size_t len;
1361-
const int64_t pos = GET_OFFSET(args[2]);
1362-
const auto enc = ParseEncoding(env->isolate(), args[3], UTF8);
13631375

13641376
FSReqBase* req_wrap = GetReqWrap(env, args[4]);
1365-
const auto is_async = req_wrap != nullptr;
1377+
const bool is_async = req_wrap != nullptr;
13661378

13671379
// Avoid copying the string when it is externalized but only when:
13681380
// 1. The target encoding is compatible with the string's encoding, and
@@ -1396,12 +1408,15 @@ static void WriteString(const FunctionCallbackInfo<Value>& args) {
13961408

13971409
uv_buf_t uvbuf = uv_buf_init(buf, len);
13981410

1399-
if (req_wrap != nullptr) {
1411+
if (is_async) { // write(fd, string, pos, enc, req)
14001412
AsyncCall(env, req_wrap, args, "write", UTF8, AfterInteger,
14011413
uv_fs_write, fd, &uvbuf, 1, pos);
1402-
} else {
1403-
SYNC_CALL(write, nullptr, fd, &uvbuf, 1, pos)
1404-
return args.GetReturnValue().Set(SYNC_RESULT);
1414+
} else { // write(fd, string, pos, enc, undefined, ctx)
1415+
CHECK_EQ(argc, 6);
1416+
fs_req_wrap req_wrap;
1417+
int bytesWritten = SyncCall(env, args[5], &req_wrap, "write",
1418+
uv_fs_write, fd, &uvbuf, 1, pos);
1419+
args.GetReturnValue().Set(bytesWritten);
14051420
}
14061421
}
14071422

test/parallel/test-fs-error-messages.js

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -770,3 +770,44 @@ if (!common.isWindows) {
770770
);
771771
});
772772
}
773+
774+
// write buffer
775+
{
776+
const validateError = (err) => {
777+
assert.strictEqual(err.message, 'EBADF: bad file descriptor, write');
778+
assert.strictEqual(err.errno, uv.UV_EBADF);
779+
assert.strictEqual(err.code, 'EBADF');
780+
assert.strictEqual(err.syscall, 'write');
781+
return true;
782+
};
783+
784+
common.runWithInvalidFD((fd) => {
785+
const buf = Buffer.alloc(5);
786+
fs.write(fd, buf, 0, 1, 1, common.mustCall(validateError));
787+
788+
assert.throws(
789+
() => fs.writeSync(fd, buf, 0, 1, 1),
790+
validateError
791+
);
792+
});
793+
}
794+
795+
// write string
796+
{
797+
const validateError = (err) => {
798+
assert.strictEqual(err.message, 'EBADF: bad file descriptor, write');
799+
assert.strictEqual(err.errno, uv.UV_EBADF);
800+
assert.strictEqual(err.code, 'EBADF');
801+
assert.strictEqual(err.syscall, 'write');
802+
return true;
803+
};
804+
805+
common.runWithInvalidFD((fd) => {
806+
fs.write(fd, 'test', 1, common.mustCall(validateError));
807+
808+
assert.throws(
809+
() => fs.writeSync(fd, 'test', 1),
810+
validateError
811+
);
812+
});
813+
}

0 commit comments

Comments
 (0)