From 994320b07be5da33e98785a7dab4bd2bb7cb8f12 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 28 Feb 2018 02:17:12 +0800 Subject: [PATCH] fs: throw writeSync errors in JS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/19041 Reviewed-By: Michaël Zasso Reviewed-By: James M Snell --- lib/fs.js | 19 ++++--- src/node_file.cc | 69 +++++++++++++++---------- test/parallel/test-fs-error-messages.js | 41 +++++++++++++++ 3 files changed, 96 insertions(+), 33 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 0671a05ae00..2c96b08144b 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -635,6 +635,8 @@ Object.defineProperty(fs.write, internalUtil.customPromisifyArgs, // fs.writeSync(fd, string[, position[, encoding]]); fs.writeSync = function(fd, buffer, offset, length, position) { validateUint32(fd, 'fd'); + const ctx = {}; + let result; if (isUint8Array(buffer)) { if (position === undefined) position = null; @@ -643,13 +645,18 @@ fs.writeSync = function(fd, buffer, offset, length, position) { if (typeof length !== 'number') length = buffer.length - offset; validateOffsetLengthWrite(offset, length, buffer.byteLength); - return binding.writeBuffer(fd, buffer, offset, length, position); + result = binding.writeBuffer(fd, buffer, offset, length, position, + undefined, ctx); + } else { + if (typeof buffer !== 'string') + buffer += ''; + if (offset === undefined) + offset = null; + result = binding.writeString(fd, buffer, offset, length, + undefined, ctx); } - if (typeof buffer !== 'string') - buffer += ''; - if (offset === undefined) - offset = null; - return binding.writeString(fd, buffer, offset, length, position); + handleErrorFromBinding(ctx); + return result; }; fs.rename = function(oldPath, newPath, callback) { diff --git a/src/node_file.cc b/src/node_file.cc index 61b4ed95873..bf036bc309e 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -1270,35 +1270,43 @@ static void CopyFile(const FunctionCallbackInfo& args) { static void WriteBuffer(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); + const int argc = args.Length(); + CHECK_GE(argc, 4); + CHECK(args[0]->IsInt32()); + const int fd = args[0].As()->Value(); + CHECK(Buffer::HasInstance(args[1])); + Local buffer_obj = args[1].As(); + char* buffer_data = Buffer::Data(buffer_obj); + size_t buffer_length = Buffer::Length(buffer_obj); - int fd = args[0]->Int32Value(); - Local obj = args[1].As(); - const char* buf = Buffer::Data(obj); - size_t buffer_length = Buffer::Length(obj); - size_t off = args[2]->Uint32Value(); - size_t len = args[3]->Uint32Value(); - int64_t pos = GET_OFFSET(args[4]); - + CHECK(args[2]->IsInt32()); + const size_t off = static_cast(args[2].As()->Value()); CHECK_LE(off, buffer_length); + + CHECK(args[3]->IsInt32()); + const size_t len = static_cast(args[3].As()->Value()); + CHECK(Buffer::IsWithinBounds(off, len, buffer_length)); CHECK_LE(len, buffer_length); CHECK_GE(off + len, off); - CHECK(Buffer::IsWithinBounds(off, len, buffer_length)); - buf += off; + const int64_t pos = GET_OFFSET(args[4]); + char* buf = buffer_data + off; uv_buf_t uvbuf = uv_buf_init(const_cast(buf), len); FSReqBase* req_wrap = GetReqWrap(env, args[5]); - if (req_wrap != nullptr) { + if (req_wrap != nullptr) { // write(fd, buffer, off, len, pos, req) AsyncCall(env, req_wrap, args, "write", UTF8, AfterInteger, uv_fs_write, fd, &uvbuf, 1, pos); - return; + } else { // write(fd, buffer, off, len, pos, undefined, ctx) + CHECK_EQ(argc, 7); + fs_req_wrap req_wrap; + int bytesWritten = SyncCall(env, args[6], &req_wrap, "write", + uv_fs_write, fd, &uvbuf, 1, pos); + args.GetReturnValue().Set(bytesWritten); } - - SYNC_CALL(write, nullptr, fd, &uvbuf, 1, pos) - args.GetReturnValue().Set(SYNC_RESULT); } @@ -1350,19 +1358,23 @@ static void WriteBuffers(const FunctionCallbackInfo& args) { static void WriteString(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - CHECK(args[0]->IsInt32()); + const int argc = args.Length(); + CHECK_GE(argc, 4); + + CHECK(args[0]->IsInt32()); + const int fd = args[0].As()->Value(); - std::unique_ptr delete_on_return; - Local req; - Local value = args[1]; - int fd = args[0]->Int32Value(); - char* buf = nullptr; - size_t len; const int64_t pos = GET_OFFSET(args[2]); + const auto enc = ParseEncoding(env->isolate(), args[3], UTF8); + std::unique_ptr delete_on_return; + Local value = args[1]; + char* buf = nullptr; + size_t len; + FSReqBase* req_wrap = GetReqWrap(env, args[4]); - const auto is_async = req_wrap != nullptr; + const bool is_async = req_wrap != nullptr; // Avoid copying the string when it is externalized but only when: // 1. The target encoding is compatible with the string's encoding, and @@ -1396,12 +1408,15 @@ static void WriteString(const FunctionCallbackInfo& args) { uv_buf_t uvbuf = uv_buf_init(buf, len); - if (req_wrap != nullptr) { + if (is_async) { // write(fd, string, pos, enc, req) AsyncCall(env, req_wrap, args, "write", UTF8, AfterInteger, uv_fs_write, fd, &uvbuf, 1, pos); - } else { - SYNC_CALL(write, nullptr, fd, &uvbuf, 1, pos) - return args.GetReturnValue().Set(SYNC_RESULT); + } else { // write(fd, string, pos, enc, undefined, ctx) + CHECK_EQ(argc, 6); + fs_req_wrap req_wrap; + int bytesWritten = SyncCall(env, args[5], &req_wrap, "write", + uv_fs_write, fd, &uvbuf, 1, pos); + args.GetReturnValue().Set(bytesWritten); } } diff --git a/test/parallel/test-fs-error-messages.js b/test/parallel/test-fs-error-messages.js index 1aa4e122949..27533f122fc 100644 --- a/test/parallel/test-fs-error-messages.js +++ b/test/parallel/test-fs-error-messages.js @@ -770,3 +770,44 @@ if (!common.isWindows) { ); }); } + +// write buffer +{ + const validateError = (err) => { + assert.strictEqual(err.message, 'EBADF: bad file descriptor, write'); + assert.strictEqual(err.errno, uv.UV_EBADF); + assert.strictEqual(err.code, 'EBADF'); + assert.strictEqual(err.syscall, 'write'); + return true; + }; + + common.runWithInvalidFD((fd) => { + const buf = Buffer.alloc(5); + fs.write(fd, buf, 0, 1, 1, common.mustCall(validateError)); + + assert.throws( + () => fs.writeSync(fd, buf, 0, 1, 1), + validateError + ); + }); +} + +// write string +{ + const validateError = (err) => { + assert.strictEqual(err.message, 'EBADF: bad file descriptor, write'); + assert.strictEqual(err.errno, uv.UV_EBADF); + assert.strictEqual(err.code, 'EBADF'); + assert.strictEqual(err.syscall, 'write'); + return true; + }; + + common.runWithInvalidFD((fd) => { + fs.write(fd, 'test', 1, common.mustCall(validateError)); + + assert.throws( + () => fs.writeSync(fd, 'test', 1), + validateError + ); + }); +}