From 715f9ae35845cf28304d10391efef71e68011e2e Mon Sep 17 00:00:00 2001 From: James M Snell Date: Tue, 1 Apr 2025 05:52:49 -0700 Subject: [PATCH] src: improve StringBytes error handling --- src/api/encoding.cc | 24 ++++++--- src/crypto/crypto_ec.cc | 12 ++--- src/crypto/crypto_hash.cc | 43 ++++++--------- src/crypto/crypto_hmac.cc | 15 ++---- src/crypto/crypto_keys.cc | 19 +++---- src/crypto/crypto_spkac.cc | 19 ++++--- src/crypto/crypto_util.cc | 12 ++--- src/encoding_binding.cc | 11 +--- src/fs_event_wrap.cc | 22 ++++---- src/node.h | 35 ++++++++++--- src/node_buffer.cc | 15 ++---- src/node_dir.cc | 38 ++++++-------- src/node_file.cc | 86 ++++++++++++------------------ src/node_i18n.cc | 7 +-- src/node_os.cc | 14 ++--- src/string_bytes.cc | 104 ++++++++++++++++--------------------- src/string_bytes.h | 9 ++-- src/string_decoder.cc | 11 +--- 18 files changed, 213 insertions(+), 283 deletions(-) diff --git a/src/api/encoding.cc b/src/api/encoding.cc index 0bc3b23a344fa8..52f41980507bcb 100644 --- a/src/api/encoding.cc +++ b/src/api/encoding.cc @@ -8,6 +8,8 @@ namespace node { using v8::HandleScope; using v8::Isolate; using v8::Local; +using v8::MaybeLocal; +using v8::TryCatch; using v8::Value; enum encoding ParseEncoding(const char* encoding, @@ -133,20 +135,30 @@ enum encoding ParseEncoding(Isolate* isolate, return ParseEncoding(*encoding, default_encoding); } +MaybeLocal TryEncode(Isolate* isolate, + const char* buf, + size_t len, + enum encoding encoding) { + CHECK_NE(encoding, UCS2); + return StringBytes::Encode(isolate, buf, len, encoding); +} + +MaybeLocal TryEncode(Isolate* isolate, const uint16_t* buf, size_t len) { + return StringBytes::Encode(isolate, buf, len).ToLocalChecked(); +} + Local Encode(Isolate* isolate, const char* buf, size_t len, enum encoding encoding) { CHECK_NE(encoding, UCS2); - Local error; - return StringBytes::Encode(isolate, buf, len, encoding, &error) - .ToLocalChecked(); + TryCatch try_catch(isolate); + return StringBytes::Encode(isolate, buf, len, encoding).ToLocalChecked(); } Local Encode(Isolate* isolate, const uint16_t* buf, size_t len) { - Local error; - return StringBytes::Encode(isolate, buf, len, &error) - .ToLocalChecked(); + TryCatch try_catch(isolate); + return StringBytes::Encode(isolate, buf, len).ToLocalChecked(); } // Returns -1 if the handle was not valid for decoding diff --git a/src/crypto/crypto_ec.cc b/src/crypto/crypto_ec.cc index 6c4ef342425a74..1d879f569a8055 100644 --- a/src/crypto/crypto_ec.cc +++ b/src/crypto/crypto_ec.cc @@ -800,17 +800,11 @@ bool ExportJWKEdKey(Environment* env, Local target, Local key) { Local encoded; - Local error; if (!data) return false; const ncrypto::Buffer out = data; - if (!StringBytes::Encode( - env->isolate(), out.data, out.len, BASE64URL, &error) - .ToLocal(&encoded) || - target->Set(env->context(), key, encoded).IsNothing()) { - if (!error.IsEmpty()) env->isolate()->ThrowException(error); - return false; - } - return true; + return StringBytes::Encode(env->isolate(), out.data, out.len, BASE64URL) + .ToLocal(&encoded) && + target->Set(env->context(), key, encoded).IsJust(); }; return !( diff --git a/src/crypto/crypto_hash.cc b/src/crypto/crypto_hash.cc index 52cda485aaa8da..590b343dbf6728 100644 --- a/src/crypto/crypto_hash.cc +++ b/src/crypto/crypto_hash.cc @@ -252,19 +252,14 @@ void Hash::OneShotDigest(const FunctionCallbackInfo& args) { return ThrowCryptoError(env, ERR_get_error()); } - Local error; - MaybeLocal rc = - StringBytes::Encode(env->isolate(), + Local ret; + if (StringBytes::Encode(env->isolate(), static_cast(output.get()), output.size(), - output_enc, - &error); - if (rc.IsEmpty()) [[unlikely]] { - CHECK(!error.IsEmpty()); - env->isolate()->ThrowException(error); - return; + output_enc) + .ToLocal(&ret)) { + args.GetReturnValue().Set(ret); } - args.GetReturnValue().Set(rc.FromMaybe(Local())); } void Hash::Initialize(Environment* env, Local target) { @@ -410,15 +405,12 @@ void Hash::HashDigest(const FunctionCallbackInfo& args) { hash->digest_ = ByteSource::Allocated(data.release()); } - Local error; - MaybeLocal rc = StringBytes::Encode( - env->isolate(), hash->digest_.data(), len, encoding, &error); - if (rc.IsEmpty()) [[unlikely]] { - CHECK(!error.IsEmpty()); - env->isolate()->ThrowException(error); - return; + Local ret; + if (StringBytes::Encode( + env->isolate(), hash->digest_.data(), len, encoding) + .ToLocal(&ret)) { + args.GetReturnValue().Set(ret); } - args.GetReturnValue().Set(rc.FromMaybe(Local())); } HashConfig::HashConfig(HashConfig&& other) noexcept @@ -541,19 +533,14 @@ void InternalVerifyIntegrity(const v8::FunctionCallbackInfo& args) { if (digest_size != expected.size() || CRYPTO_memcmp(digest, expected.data(), digest_size) != 0) { - Local error; - MaybeLocal rc = - StringBytes::Encode(env->isolate(), + Local ret; + if (StringBytes::Encode(env->isolate(), reinterpret_cast(digest), digest_size, - BASE64, - &error); - if (rc.IsEmpty()) [[unlikely]] { - CHECK(!error.IsEmpty()); - env->isolate()->ThrowException(error); - return; + BASE64) + .ToLocal(&ret)) { + args.GetReturnValue().Set(ret); } - args.GetReturnValue().Set(rc.FromMaybe(Local())); } } } // namespace crypto diff --git a/src/crypto/crypto_hmac.cc b/src/crypto/crypto_hmac.cc index e37d919808c994..f804e6ef3a8a9c 100644 --- a/src/crypto/crypto_hmac.cc +++ b/src/crypto/crypto_hmac.cc @@ -145,19 +145,14 @@ void Hmac::HmacDigest(const FunctionCallbackInfo& args) { hmac->ctx_.reset(); } - Local error; - MaybeLocal rc = - StringBytes::Encode(env->isolate(), + Local ret; + if (StringBytes::Encode(env->isolate(), reinterpret_cast(md_value), buf.len, - encoding, - &error); - if (rc.IsEmpty()) [[unlikely]] { - CHECK(!error.IsEmpty()); - env->isolate()->ThrowException(error); - return; + encoding) + .ToLocal(&ret)) { + args.GetReturnValue().Set(ret); } - args.GetReturnValue().Set(rc.FromMaybe(Local())); } HmacConfig::HmacConfig(HmacConfig&& other) noexcept diff --git a/src/crypto/crypto_keys.cc b/src/crypto/crypto_keys.cc index db002875d2657b..190975d9a19d86 100644 --- a/src/crypto/crypto_keys.cc +++ b/src/crypto/crypto_keys.cc @@ -130,20 +130,13 @@ bool ExportJWKSecretKey(Environment* env, Local target) { CHECK_EQ(key.GetKeyType(), kKeyTypeSecret); - Local error; Local raw; - if (!StringBytes::Encode(env->isolate(), - key.GetSymmetricKey(), - key.GetSymmetricKeySize(), - BASE64URL, - &error) - .ToLocal(&raw)) { - DCHECK(!error.IsEmpty()); - env->isolate()->ThrowException(error); - return false; - } - - return target + return StringBytes::Encode(env->isolate(), + key.GetSymmetricKey(), + key.GetSymmetricKeySize(), + BASE64URL) + .ToLocal(&raw) && + target ->Set(env->context(), env->jwk_kty_string(), env->jwk_oct_string()) .IsJust() && target->Set(env->context(), env->jwk_k_string(), raw).IsJust(); diff --git a/src/crypto/crypto_spkac.cc b/src/crypto/crypto_spkac.cc index ba085226984767..e33f24b46d6d4f 100644 --- a/src/crypto/crypto_spkac.cc +++ b/src/crypto/crypto_spkac.cc @@ -5,6 +5,7 @@ #include "memory_tracker-inl.h" #include "ncrypto.h" #include "node.h" +#include "string_bytes.h" #include "v8.h" namespace node { @@ -52,18 +53,22 @@ void ExportChallenge(const FunctionCallbackInfo& args) { ArrayBufferOrViewContents input(args[0]); if (input.empty()) return args.GetReturnValue().SetEmptyString(); - if (!input.CheckSizeInt32()) [[unlikely]] + if (!input.CheckSizeInt32()) [[unlikely]] { return THROW_ERR_OUT_OF_RANGE(env, "spkac is too large"); + } auto cert = ByteSource::Allocated( ncrypto::ExportChallenge(input.data(), input.size())); - if (!cert) + if (!cert) { return args.GetReturnValue().SetEmptyString(); - - Local outString = - Encode(env->isolate(), cert.data(), cert.size(), BUFFER); - - args.GetReturnValue().Set(outString); + } + + Local outString; + if (StringBytes::Encode( + env->isolate(), cert.data(), cert.size(), BUFFER) + .ToLocal(&outString)) { + args.GetReturnValue().Set(outString); + } } void Initialize(Environment* env, Local target) { diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc index 55cd7382eb7802..44036dd6b788c3 100644 --- a/src/crypto/crypto_util.cc +++ b/src/crypto/crypto_util.cc @@ -36,6 +36,7 @@ using v8::ArrayBuffer; using v8::BackingStore; using v8::BigInt; using v8::Context; +using v8::EscapableHandleScope; using v8::Exception; using v8::FunctionCallbackInfo; using v8::HandleScope; @@ -605,21 +606,18 @@ void SetEngine(const FunctionCallbackInfo& args) { #endif // !OPENSSL_NO_ENGINE MaybeLocal EncodeBignum(Environment* env, const BIGNUM* bn, int size) { + EscapableHandleScope scope(env->isolate()); auto buf = BignumPointer::EncodePadded(bn, size); CHECK_EQ(buf.size(), static_cast(size)); Local ret; - Local error; if (!StringBytes::Encode(env->isolate(), reinterpret_cast(buf.get()), buf.size(), - BASE64URL, - &error) + BASE64URL) .ToLocal(&ret)) { - CHECK(!error.IsEmpty()); - env->isolate()->ThrowException(error); - return MaybeLocal(); + return {}; } - return ret; + return scope.Escape(ret); } Maybe SetEncodedValue(Environment* env, diff --git a/src/encoding_binding.cc b/src/encoding_binding.cc index 61d692dfe5ffa0..3cba0e932ed6c4 100644 --- a/src/encoding_binding.cc +++ b/src/encoding_binding.cc @@ -183,17 +183,10 @@ void BindingData::DecodeUTF8(const FunctionCallbackInfo& args) { if (length == 0) return args.GetReturnValue().SetEmptyString(); - Local error; Local ret; - - if (!StringBytes::Encode(env->isolate(), data, length, UTF8, &error) - .ToLocal(&ret)) { - CHECK(!error.IsEmpty()); - env->isolate()->ThrowException(error); - return; + if (StringBytes::Encode(env->isolate(), data, length, UTF8).ToLocal(&ret)) { + args.GetReturnValue().Set(ret); } - - args.GetReturnValue().Set(ret); } void BindingData::ToASCII(const FunctionCallbackInfo& args) { diff --git a/src/fs_event_wrap.cc b/src/fs_event_wrap.cc index 400a84af1ff3c3..6d954e3de17c86 100644 --- a/src/fs_event_wrap.cc +++ b/src/fs_event_wrap.cc @@ -44,6 +44,7 @@ using v8::PropertyAttribute; using v8::ReadOnly; using v8::Signature; using v8::String; +using v8::TryCatch; using v8::Value; namespace { @@ -217,18 +218,19 @@ void FSEventWrap::OnEvent(uv_fs_event_t* handle, const char* filename, }; if (filename != nullptr) { - Local error; - MaybeLocal fn = StringBytes::Encode(env->isolate(), - filename, - wrap->encoding_, - &error); + // TODO(@jasnell): Historically, this code has failed to correctly + // propagate any error returned by the StringBytes::Encode method, + // and would instead just crash the process. That behavior is preserved + // here but should be looked at. Preferrably errors would be handled + // correctly here. + TryCatch try_catch(env->isolate()); + MaybeLocal fn = + StringBytes::Encode(env->isolate(), filename, wrap->encoding_); if (fn.IsEmpty()) { argv[0] = Integer::New(env->isolate(), UV_EINVAL); - argv[2] = StringBytes::Encode(env->isolate(), - filename, - strlen(filename), - BUFFER, - &error).ToLocalChecked(); + argv[2] = StringBytes::Encode( + env->isolate(), filename, strlen(filename), BUFFER) + .ToLocalChecked(); } else { argv[2] = fn.ToLocalChecked(); } diff --git a/src/node.h b/src/node.h index d0163523f08d84..6031012a04f29e 100644 --- a/src/node.h +++ b/src/node.h @@ -1123,16 +1123,37 @@ NODE_EXTERN enum encoding ParseEncoding( NODE_EXTERN void FatalException(v8::Isolate* isolate, const v8::TryCatch& try_catch); -NODE_EXTERN v8::Local Encode(v8::Isolate* isolate, - const char* buf, - size_t len, - enum encoding encoding = LATIN1); +NODE_EXTERN v8::MaybeLocal TryEncode( + v8::Isolate* isolate, + const char* buf, + size_t len, + enum encoding encoding = LATIN1); + +// Warning: This reverses endianness on Big Endian platforms, even though the +// signature using uint16_t implies that it should not. +NODE_EXTERN v8::MaybeLocal TryEncode(v8::Isolate* isolate, + const uint16_t* buf, + size_t len); + +// The original Encode(...) functions are deprecated because they do not +// appropriately propagate exceptions and instead rely on ToLocalChecked() +// which crashes the process if an exception occurs. We cannot just remove +// these as it would break ABI compatibility, so we keep them around but +// deprecate them in favor of the TryEncode(...) variations which return +// a MaybeLocal<> and do not crash the process if an exception occurs. +NODE_DEPRECATED( + "Use TryEncode(...) instead", + NODE_EXTERN v8::Local Encode(v8::Isolate* isolate, + const char* buf, + size_t len, + enum encoding encoding = LATIN1)); // Warning: This reverses endianness on Big Endian platforms, even though the // signature using uint16_t implies that it should not. -NODE_EXTERN v8::Local Encode(v8::Isolate* isolate, - const uint16_t* buf, - size_t len); +NODE_DEPRECATED("Use TryEncode(...) instead", + NODE_EXTERN v8::Local Encode(v8::Isolate* isolate, + const uint16_t* buf, + size_t len)); // Returns -1 if the handle was not valid for decoding NODE_EXTERN ssize_t DecodeBytes(v8::Isolate* isolate, diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 0f249f1947a393..36af659a5339a1 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -544,20 +544,11 @@ void StringSlice(const FunctionCallbackInfo& args) { THROW_AND_RETURN_IF_OOB(Just(end <= buffer.length())); size_t length = end - start; - Local error; - MaybeLocal maybe_ret = - StringBytes::Encode(isolate, - buffer.data() + start, - length, - encoding, - &error); Local ret; - if (!maybe_ret.ToLocal(&ret)) { - CHECK(!error.IsEmpty()); - isolate->ThrowException(error); - return; + if (StringBytes::Encode(isolate, buffer.data() + start, length, encoding) + .ToLocal(&ret)) { + args.GetReturnValue().Set(ret); } - args.GetReturnValue().Set(ret); } // Assume caller has properly validated args. diff --git a/src/node_dir.cc b/src/node_dir.cc index 2dfb1305a18909..c9173d404c79a6 100644 --- a/src/node_dir.cc +++ b/src/node_dir.cc @@ -42,6 +42,7 @@ using v8::Null; using v8::Number; using v8::Object; using v8::ObjectTemplate; +using v8::TryCatch; using v8::Value; static const char* get_dir_func_name_by_type(uv_fs_type req_type) { @@ -206,27 +207,20 @@ void DirHandle::Close(const FunctionCallbackInfo& args) { } } -static MaybeLocal DirentListToArray( - Environment* env, - uv_dirent_t* ents, - int num, - enum encoding encoding, - Local* err_out) { +static MaybeLocal DirentListToArray(Environment* env, + uv_dirent_t* ents, + int num, + enum encoding encoding) { MaybeStackBuffer, 64> entries(num * 2); // Return an array of all read filenames. int j = 0; for (int i = 0; i < num; i++) { Local filename; - Local error; const size_t namelen = strlen(ents[i].name); - if (!StringBytes::Encode(env->isolate(), - ents[i].name, - namelen, - encoding, - &error).ToLocal(&filename)) { - *err_out = error; - return MaybeLocal(); + if (!StringBytes::Encode(env->isolate(), ents[i].name, namelen, encoding) + .ToLocal(&filename)) { + return {}; } entries[j++] = filename; @@ -258,18 +252,18 @@ static void AfterDirRead(uv_fs_t* req) { uv_dir_t* dir = static_cast(req->ptr); - Local error; + TryCatch try_catch(isolate); Local js_array; if (!DirentListToArray(env, dir->dirents, static_cast(req->result), - req_wrap->encoding(), - &error) + req_wrap->encoding()) .ToLocal(&js_array)) { // Clear libuv resources *before* delivering results to JS land because // that can schedule another operation on the same uv_dir_t. Ditto below. after.Clear(); - return req_wrap->Reject(error); + CHECK(try_catch.CanContinue()); + return req_wrap->Reject(try_catch.Exception()); } after.Clear(); @@ -320,16 +314,16 @@ void DirHandle::Read(const FunctionCallbackInfo& args) { CHECK_GE(req_wrap_sync.req.result, 0); - Local error; + TryCatch try_catch(isolate); Local js_array; if (!DirentListToArray(env, dir->dir()->dirents, static_cast(req_wrap_sync.req.result), - encoding, - &error) + encoding) .ToLocal(&js_array)) { // TODO(anonrig): Initializing BufferValue here is wasteful. - BufferValue error_payload(isolate, error); + CHECK(try_catch.CanContinue()); + BufferValue error_payload(isolate, try_catch.Exception()); env->ThrowError(error_payload.out()); return; } diff --git a/src/node_file.cc b/src/node_file.cc index d7cca08521c1c6..fc8aa6101a45ad 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -82,6 +82,7 @@ using v8::Object; using v8::ObjectTemplate; using v8::Promise; using v8::String; +using v8::TryCatch; using v8::Undefined; using v8::Value; @@ -845,11 +846,13 @@ void AfterMkdirp(uv_fs_t* req) { if (first_path.empty()) return req_wrap->Resolve(Undefined(req_wrap->env()->isolate())); Local path; - Local error; - if (!StringBytes::Encode(req_wrap->env()->isolate(), first_path.c_str(), - req_wrap->encoding(), - &error).ToLocal(&path)) { - return req_wrap->Reject(error); + TryCatch try_catch(req_wrap->env()->isolate()); + if (!StringBytes::Encode(req_wrap->env()->isolate(), + first_path.c_str(), + req_wrap->encoding()) + .ToLocal(&path)) { + CHECK(try_catch.CanContinue()); + return req_wrap->Reject(try_catch.Exception()); } return req_wrap->Resolve(path); } @@ -861,15 +864,14 @@ void AfterStringPath(uv_fs_t* req) { FS_ASYNC_TRACE_END1( req->fs_type, req_wrap, "result", static_cast(req->result)) MaybeLocal link; - Local error; if (after.Proceed()) { - link = StringBytes::Encode(req_wrap->env()->isolate(), - req->path, - req_wrap->encoding(), - &error); + TryCatch try_catch(req_wrap->env()->isolate()); + link = StringBytes::Encode( + req_wrap->env()->isolate(), req->path, req_wrap->encoding()); if (link.IsEmpty()) { - req_wrap->Reject(error); + CHECK(try_catch.CanContinue()); + req_wrap->Reject(try_catch.Exception()); } else { Local val; if (link.ToLocal(&val)) req_wrap->Resolve(val); @@ -883,15 +885,15 @@ void AfterStringPtr(uv_fs_t* req) { FS_ASYNC_TRACE_END1( req->fs_type, req_wrap, "result", static_cast(req->result)) MaybeLocal link; - Local error; if (after.Proceed()) { + TryCatch try_catch(req_wrap->env()->isolate()); link = StringBytes::Encode(req_wrap->env()->isolate(), static_cast(req->ptr), - req_wrap->encoding(), - &error); + req_wrap->encoding()); if (link.IsEmpty()) { - req_wrap->Reject(error); + CHECK(try_catch.CanContinue()); + req_wrap->Reject(try_catch.Exception()); } else { Local val; if (link.ToLocal(&val)) req_wrap->Resolve(val); @@ -910,7 +912,6 @@ void AfterScanDir(uv_fs_t* req) { Environment* env = req_wrap->env(); Isolate* isolate = env->isolate(); - Local error; int r; LocalVector name_v(isolate); @@ -930,9 +931,11 @@ void AfterScanDir(uv_fs_t* req) { } Local filename; - if (!StringBytes::Encode(isolate, ent.name, req_wrap->encoding(), &error) + TryCatch try_catch(isolate); + if (!StringBytes::Encode(isolate, ent.name, req_wrap->encoding()) .ToLocal(&filename)) { - return req_wrap->Reject(error); + CHECK(try_catch.CanContinue()); + return req_wrap->Reject(try_catch.Exception()); } name_v.push_back(filename); @@ -1422,16 +1425,10 @@ static void ReadLink(const FunctionCallbackInfo& args) { } const char* link_path = static_cast(req_wrap_sync.req.ptr); - Local error; Local ret; - if (!StringBytes::Encode(isolate, link_path, encoding, &error) - .ToLocal(&ret)) { - DCHECK(!error.IsEmpty()); - env->isolate()->ThrowException(error); - return; + if (StringBytes::Encode(isolate, link_path, encoding).ToLocal(&ret)) { + args.GetReturnValue().Set(ret); } - - args.GetReturnValue().Set(ret); } } @@ -1932,17 +1929,12 @@ static void MKDir(const FunctionCallbackInfo& args) { return; } if (!req_wrap_sync.continuation_data()->first_path().empty()) { - Local error; Local ret; std::string first_path(req_wrap_sync.continuation_data()->first_path()); - if (!StringBytes::Encode( - env->isolate(), first_path.c_str(), UTF8, &error) - .ToLocal(&ret)) { - DCHECK(!error.IsEmpty()); - env->isolate()->ThrowException(error); - return; + if (StringBytes::Encode(env->isolate(), first_path.c_str(), UTF8) + .ToLocal(&ret)) { + args.GetReturnValue().Set(ret); } - args.GetReturnValue().Set(ret); } } else { SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_mkdir, *path, mode); @@ -1982,16 +1974,10 @@ static void RealPath(const FunctionCallbackInfo& args) { const char* link_path = static_cast(req_wrap_sync.req.ptr); - Local error; Local ret; - if (!StringBytes::Encode(isolate, link_path, encoding, &error) - .ToLocal(&ret)) { - DCHECK(!error.IsEmpty()); - env->isolate()->ThrowException(error); - return; + if (StringBytes::Encode(isolate, link_path, encoding).ToLocal(&ret)) { + args.GetReturnValue().Set(ret); } - - args.GetReturnValue().Set(ret); } } @@ -2077,12 +2063,8 @@ static void ReadDir(const FunctionCallbackInfo& args) { return; } - Local error; Local fn; - if (!StringBytes::Encode(isolate, ent.name, encoding, &error) - .ToLocal(&fn)) { - DCHECK(!error.IsEmpty()); - isolate->ThrowException(error); + if (!StringBytes::Encode(isolate, ent.name, encoding).ToLocal(&fn)) { return; } @@ -3106,15 +3088,11 @@ static void Mkdtemp(const FunctionCallbackInfo& args) { if (is_uv_error(result)) { return; } - Local error; Local ret; - if (!StringBytes::Encode(isolate, req_wrap_sync.req.path, encoding, &error) - .ToLocal(&ret)) { - DCHECK(!error.IsEmpty()); - env->isolate()->ThrowException(error); - return; + if (StringBytes::Encode(isolate, req_wrap_sync.req.path, encoding) + .ToLocal(&ret)) { + args.GetReturnValue().Set(ret); } - args.GetReturnValue().Set(ret); } } diff --git a/src/node_i18n.cc b/src/node_i18n.cc index dbda7d2f653b38..9f3ae18dcfe2b4 100644 --- a/src/node_i18n.cc +++ b/src/node_i18n.cc @@ -500,7 +500,6 @@ void ConverterObject::Decode(const FunctionCallbackInfo& args) { } } - Local error; UChar* output = result.out(); size_t beginning = 0; size_t length = result.length() * sizeof(UChar); @@ -517,11 +516,9 @@ void ConverterObject::Decode(const FunctionCallbackInfo& args) { CHECK(nbytes::SwapBytes16(value, length)); } - MaybeLocal encoded = - StringBytes::Encode(env->isolate(), value, length, UCS2, &error); - Local ret; - if (encoded.ToLocal(&ret)) { + if (StringBytes::Encode(env->isolate(), value, length, UCS2) + .ToLocal(&ret)) { args.GetReturnValue().Set(ret); return; } diff --git a/src/node_os.cc b/src/node_os.cc index 8da7e7f9eb78c8..e46d32cd9e50f5 100644 --- a/src/node_os.cc +++ b/src/node_os.cc @@ -307,8 +307,6 @@ static void GetUserInfo(const FunctionCallbackInfo& args) { auto free_passwd = OnScopeLeave([&] { uv_os_free_passwd(&pwd); }); - Local error; - #ifdef _WIN32 Local uid = Number::New( env->isolate(), @@ -322,27 +320,21 @@ static void GetUserInfo(const FunctionCallbackInfo& args) { #endif Local username; - if (!StringBytes::Encode(env->isolate(), pwd.username, encoding, &error) + if (!StringBytes::Encode(env->isolate(), pwd.username, encoding) .ToLocal(&username)) { - CHECK(!error.IsEmpty()); - env->isolate()->ThrowException(error); return; } Local homedir; - if (!StringBytes::Encode(env->isolate(), pwd.homedir, encoding, &error) + if (!StringBytes::Encode(env->isolate(), pwd.homedir, encoding) .ToLocal(&homedir)) { - CHECK(!error.IsEmpty()); - env->isolate()->ThrowException(error); return; } Local shell = Null(env->isolate()); if (pwd.shell != nullptr && - !StringBytes::Encode(env->isolate(), pwd.shell, encoding, &error) + !StringBytes::Encode(env->isolate(), pwd.shell, encoding) .ToLocal(&shell)) { - CHECK(!error.IsEmpty()); - env->isolate()->ThrowException(error); return; } diff --git a/src/string_bytes.cc b/src/string_bytes.cc index 2cfbaf7253baef..29abd45685316e 100644 --- a/src/string_bytes.cc +++ b/src/string_bytes.cc @@ -74,37 +74,34 @@ class ExternString: public ResourceType { static MaybeLocal NewFromCopy(Isolate* isolate, const TypeName* data, - size_t length, - Local* error) { - if (length == 0) + size_t length) { + if (length == 0) { return String::Empty(isolate); + } - if (length < EXTERN_APEX) - return NewSimpleFromCopy(isolate, data, length, error); + if (length < EXTERN_APEX) { + return NewSimpleFromCopy(isolate, data, length); + } TypeName* new_data = node::UncheckedMalloc(length); if (new_data == nullptr) { - *error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate); + isolate->ThrowException(node::ERR_MEMORY_ALLOCATION_FAILED(isolate)); return MaybeLocal(); } memcpy(new_data, data, length * sizeof(*new_data)); - return ExternString::New(isolate, - new_data, - length, - error); + return ExternString::New(isolate, new_data, length); } // uses "data" for external resource, and will be free'd on gc static MaybeLocal New(Isolate* isolate, TypeName* data, - size_t length, - Local* error) { + size_t length) { if (length == 0) return String::Empty(isolate); if (length < EXTERN_APEX) { - MaybeLocal str = NewSimpleFromCopy(isolate, data, length, error); + MaybeLocal str = NewSimpleFromCopy(isolate, data, length); free(data); return str; } @@ -116,7 +113,7 @@ class ExternString: public ResourceType { if (!NewExternal(isolate, h_str).ToLocal(&str)) { delete h_str; - *error = node::ERR_STRING_TOO_LONG(isolate); + isolate->ThrowException(node::ERR_STRING_TOO_LONG(isolate)); return MaybeLocal(); } @@ -136,8 +133,7 @@ class ExternString: public ResourceType { // This method does not actually create ExternString instances. static MaybeLocal NewSimpleFromCopy(Isolate* isolate, const TypeName* data, - size_t length, - Local* error); + size_t length); Isolate* isolate_; const TypeName* data_; @@ -167,30 +163,27 @@ MaybeLocal ExternTwoByteString::NewExternal( template <> MaybeLocal ExternOneByteString::NewSimpleFromCopy(Isolate* isolate, const char* data, - size_t length, - Local* error) { + size_t length) { Local str; if (!String::NewFromOneByte(isolate, reinterpret_cast(data), v8::NewStringType::kNormal, length) .ToLocal(&str)) { - *error = node::ERR_STRING_TOO_LONG(isolate); + isolate->ThrowException(node::ERR_STRING_TOO_LONG(isolate)); return MaybeLocal(); } return str; } - template <> MaybeLocal ExternTwoByteString::NewSimpleFromCopy(Isolate* isolate, const uint16_t* data, - size_t length, - Local* error) { + size_t length) { Local str; if (!String::NewFromTwoByte(isolate, data, v8::NewStringType::kNormal, length) .ToLocal(&str)) { - *error = node::ERR_STRING_TOO_LONG(isolate); + isolate->ThrowException(node::ERR_STRING_TOO_LONG(isolate)); return MaybeLocal(); } return str; @@ -489,20 +482,18 @@ Maybe StringBytes::Size(Isolate* isolate, UNREACHABLE(); } -#define CHECK_BUFLEN_IN_RANGE(len) \ - do { \ - if ((len) > Buffer::kMaxLength) { \ - *error = node::ERR_BUFFER_TOO_LARGE(isolate); \ - return MaybeLocal(); \ - } \ +#define CHECK_BUFLEN_IN_RANGE(len) \ + do { \ + if ((len) > Buffer::kMaxLength) { \ + isolate->ThrowException(node::ERR_BUFFER_TOO_LARGE(isolate)); \ + return MaybeLocal(); \ + } \ } while (0) - MaybeLocal StringBytes::Encode(Isolate* isolate, const char* buf, size_t buflen, - enum encoding encoding, - Local* error) { + enum encoding encoding) { CHECK_BUFLEN_IN_RANGE(buflen); if (!buflen && encoding != BUFFER) { @@ -517,7 +508,7 @@ MaybeLocal StringBytes::Encode(Isolate* isolate, auto maybe_buf = Buffer::Copy(isolate, buf, buflen); Local buf; if (!maybe_buf.ToLocal(&buf)) { - *error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate); + isolate->ThrowException(node::ERR_MEMORY_ALLOCATION_FAILED(isolate)); } return buf; } @@ -528,13 +519,13 @@ MaybeLocal StringBytes::Encode(Isolate* isolate, // The input contains non-ASCII bytes. char* out = node::UncheckedMalloc(buflen); if (out == nullptr) { - *error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate); + isolate->ThrowException(node::ERR_MEMORY_ALLOCATION_FAILED(isolate)); return MaybeLocal(); } nbytes::ForceAscii(buf, out, buflen); - return ExternOneByteString::New(isolate, out, buflen, error); + return ExternOneByteString::New(isolate, out, buflen); } else { - return ExternOneByteString::NewFromCopy(isolate, buf, buflen, error); + return ExternOneByteString::NewFromCopy(isolate, buf, buflen); } case UTF8: { @@ -543,28 +534,28 @@ MaybeLocal StringBytes::Encode(Isolate* isolate, String::NewFromUtf8(isolate, buf, v8::NewStringType::kNormal, buflen); Local str; if (!val.ToLocal(&str)) { - *error = node::ERR_STRING_TOO_LONG(isolate); + isolate->ThrowException(node::ERR_STRING_TOO_LONG(isolate)); } return str; } case LATIN1: buflen = keep_buflen_in_range(buflen); - return ExternOneByteString::NewFromCopy(isolate, buf, buflen, error); + return ExternOneByteString::NewFromCopy(isolate, buf, buflen); case BASE64: { buflen = keep_buflen_in_range(buflen); size_t dlen = simdutf::base64_length_from_binary(buflen); char* dst = node::UncheckedMalloc(dlen); if (dst == nullptr) { - *error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate); + isolate->ThrowException(node::ERR_MEMORY_ALLOCATION_FAILED(isolate)); return MaybeLocal(); } size_t written = simdutf::binary_to_base64(buf, buflen, dst); CHECK_EQ(written, dlen); - return ExternOneByteString::New(isolate, dst, dlen, error); + return ExternOneByteString::New(isolate, dst, dlen); } case BASE64URL: { @@ -573,7 +564,7 @@ MaybeLocal StringBytes::Encode(Isolate* isolate, simdutf::base64_length_from_binary(buflen, simdutf::base64_url); char* dst = node::UncheckedMalloc(dlen); if (dst == nullptr) { - *error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate); + isolate->ThrowException(node::ERR_MEMORY_ALLOCATION_FAILED(isolate)); return MaybeLocal(); } @@ -581,7 +572,7 @@ MaybeLocal StringBytes::Encode(Isolate* isolate, simdutf::binary_to_base64(buf, buflen, dst, simdutf::base64_url); CHECK_EQ(written, dlen); - return ExternOneByteString::New(isolate, dst, dlen, error); + return ExternOneByteString::New(isolate, dst, dlen); } case HEX: { @@ -589,13 +580,13 @@ MaybeLocal StringBytes::Encode(Isolate* isolate, size_t dlen = buflen * 2; char* dst = node::UncheckedMalloc(dlen); if (dst == nullptr) { - *error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate); + isolate->ThrowException(node::ERR_MEMORY_ALLOCATION_FAILED(isolate)); return MaybeLocal(); } size_t written = nbytes::HexEncode(buf, buflen, dst, dlen); CHECK_EQ(written, dlen); - return ExternOneByteString::New(isolate, dst, dlen, error); + return ExternOneByteString::New(isolate, dst, dlen); } case UCS2: { @@ -604,7 +595,7 @@ MaybeLocal StringBytes::Encode(Isolate* isolate, if constexpr (IsBigEndian()) { uint16_t* dst = node::UncheckedMalloc(str_len); if (str_len != 0 && dst == nullptr) { - *error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate); + isolate->ThrowException(node::ERR_MEMORY_ALLOCATION_FAILED(isolate)); return MaybeLocal(); } for (size_t i = 0, k = 0; k < str_len; i += 2, k += 1) { @@ -614,21 +605,21 @@ MaybeLocal StringBytes::Encode(Isolate* isolate, const uint8_t lo = static_cast(buf[i + 0]); dst[k] = static_cast(hi) << 8 | lo; } - return ExternTwoByteString::New(isolate, dst, str_len, error); + return ExternTwoByteString::New(isolate, dst, str_len); } if (reinterpret_cast(buf) % 2 != 0) { // Unaligned data still means we can't directly pass it to V8. char* dst = node::UncheckedMalloc(buflen); if (dst == nullptr) { - *error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate); + isolate->ThrowException(node::ERR_MEMORY_ALLOCATION_FAILED(isolate)); return MaybeLocal(); } memcpy(dst, buf, buflen); return ExternTwoByteString::New( - isolate, reinterpret_cast(dst), str_len, error); + isolate, reinterpret_cast(dst), str_len); } return ExternTwoByteString::NewFromCopy( - isolate, reinterpret_cast(buf), str_len, error); + isolate, reinterpret_cast(buf), str_len); } default: @@ -636,11 +627,9 @@ MaybeLocal StringBytes::Encode(Isolate* isolate, } } - MaybeLocal StringBytes::Encode(Isolate* isolate, const uint16_t* buf, - size_t buflen, - Local* error) { + size_t buflen) { if (buflen == 0) return String::Empty(isolate); CHECK_BUFLEN_IN_RANGE(buflen); @@ -651,24 +640,23 @@ MaybeLocal StringBytes::Encode(Isolate* isolate, if constexpr (IsBigEndian()) { uint16_t* dst = node::UncheckedMalloc(buflen); if (dst == nullptr) { - *error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate); + isolate->ThrowException(node::ERR_MEMORY_ALLOCATION_FAILED(isolate)); return MaybeLocal(); } size_t nbytes = buflen * sizeof(uint16_t); memcpy(dst, buf, nbytes); CHECK(nbytes::SwapBytes16(reinterpret_cast(dst), nbytes)); - return ExternTwoByteString::New(isolate, dst, buflen, error); + return ExternTwoByteString::New(isolate, dst, buflen); } else { - return ExternTwoByteString::NewFromCopy(isolate, buf, buflen, error); + return ExternTwoByteString::NewFromCopy(isolate, buf, buflen); } } MaybeLocal StringBytes::Encode(Isolate* isolate, const char* buf, - enum encoding encoding, - Local* error) { + enum encoding encoding) { const size_t len = strlen(buf); - return Encode(isolate, buf, len, encoding, error); + return Encode(isolate, buf, len, encoding); } } // namespace node diff --git a/src/string_bytes.h b/src/string_bytes.h index 53bc003fbda436..a52cc5148269dd 100644 --- a/src/string_bytes.h +++ b/src/string_bytes.h @@ -81,8 +81,7 @@ class StringBytes { static v8::MaybeLocal Encode(v8::Isolate* isolate, const char* buf, size_t buflen, - enum encoding encoding, - v8::Local* error); + enum encoding encoding); // Warning: This reverses endianness on BE platforms, even though the // signature using uint16_t implies that it should not. @@ -90,13 +89,11 @@ class StringBytes { // be changed easily. static v8::MaybeLocal Encode(v8::Isolate* isolate, const uint16_t* buf, - size_t buflen, - v8::Local* error); + size_t buflen); static v8::MaybeLocal Encode(v8::Isolate* isolate, const char* buf, - enum encoding encoding, - v8::Local* error); + enum encoding encoding); private: static size_t WriteUCS2(v8::Isolate* isolate, diff --git a/src/string_decoder.cc b/src/string_decoder.cc index c7a6f5e8e58d24..4787a83aafe3eb 100644 --- a/src/string_decoder.cc +++ b/src/string_decoder.cc @@ -28,7 +28,6 @@ MaybeLocal MakeString(Isolate* isolate, const char* data, size_t length, enum encoding encoding) { - Local error; MaybeLocal ret; if (encoding == UTF8) { MaybeLocal utf8_string; @@ -43,17 +42,11 @@ MaybeLocal MakeString(Isolate* isolate, return utf8_string; } } else { - ret = StringBytes::Encode( - isolate, - data, - length, - encoding, - &error); + ret = StringBytes::Encode(isolate, data, length, encoding); } if (ret.IsEmpty()) { - CHECK(!error.IsEmpty()); - isolate->ThrowException(error); + return {}; } DCHECK(ret.IsEmpty() || ret.ToLocalChecked()->IsString());