Skip to content

Commit b2a955a

Browse files
ryzokukentargos
ryzokuken
and
targos
committed
src: rework (mostly internal) functions to use Maybes
Rework all affected functions to use Maybes, thus improving error handling substantially in internal functions, API functions as well as utilities. Co-authored-by: Michaël Zasso <[email protected]> PR-URL: #21935 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
1 parent 0a65727 commit b2a955a

10 files changed

+149
-117
lines changed

‎src/node_buffer.cc

+6-3
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,9 @@ MaybeLocal<Object> New(Isolate* isolate,
236236
enum encoding enc) {
237237
EscapableHandleScope scope(isolate);
238238

239-
const size_t length = StringBytes::Size(isolate, string, enc);
239+
size_t length;
240+
if (!StringBytes::Size(isolate, string, enc).To(&length))
241+
return Local<Object>();
240242
size_t actual = 0;
241243
char* data = nullptr;
242244

@@ -828,7 +830,8 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {
828830
const size_t haystack_length = (enc == UCS2) ?
829831
ts_obj_length &~ 1 : ts_obj_length; // NOLINT(whitespace/operators)
830832

831-
const size_t needle_length = StringBytes::Size(isolate, needle, enc);
833+
size_t needle_length;
834+
if (!StringBytes::Size(isolate, needle, enc).To(&needle_length)) return;
832835

833836
int64_t opt_offset = IndexOfOffset(haystack_length,
834837
offset_i64,
@@ -868,7 +871,7 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {
868871

869872
if (IsBigEndian()) {
870873
StringBytes::InlineDecoder decoder;
871-
decoder.Decode(env, needle, args[3], UCS2);
874+
if (decoder.Decode(env, needle, args[3], UCS2).IsNothing()) return;
872875
const uint16_t* decoded_string =
873876
reinterpret_cast<const uint16_t*>(decoder.out());
874877

‎src/node_crypto.cc

+6-3
Original file line numberDiff line numberDiff line change
@@ -3062,7 +3062,8 @@ void CipherBase::Update(const FunctionCallbackInfo<Value>& args) {
30623062
// Only copy the data if we have to, because it's a string
30633063
if (args[0]->IsString()) {
30643064
StringBytes::InlineDecoder decoder;
3065-
if (!decoder.Decode(env, args[0].As<String>(), args[1], UTF8))
3065+
if (!decoder.Decode(env, args[0].As<String>(), args[1], UTF8)
3066+
.FromMaybe(false))
30663067
return;
30673068
r = cipher->Update(decoder.out(), decoder.size(), &out, &out_len);
30683069
} else {
@@ -3252,7 +3253,8 @@ void Hmac::HmacUpdate(const FunctionCallbackInfo<Value>& args) {
32523253
bool r = false;
32533254
if (args[0]->IsString()) {
32543255
StringBytes::InlineDecoder decoder;
3255-
if (decoder.Decode(env, args[0].As<String>(), args[1], UTF8)) {
3256+
if (decoder.Decode(env, args[0].As<String>(), args[1], UTF8)
3257+
.FromMaybe(false)) {
32563258
r = hmac->HmacUpdate(decoder.out(), decoder.size());
32573259
}
32583260
} else {
@@ -3359,7 +3361,8 @@ void Hash::HashUpdate(const FunctionCallbackInfo<Value>& args) {
33593361
bool r = true;
33603362
if (args[0]->IsString()) {
33613363
StringBytes::InlineDecoder decoder;
3362-
if (!decoder.Decode(env, args[0].As<String>(), args[1], UTF8)) {
3364+
if (!decoder.Decode(env, args[0].As<String>(), args[1], UTF8)
3365+
.FromMaybe(false)) {
33633366
args.GetReturnValue().Set(false);
33643367
return;
33653368
}

‎src/node_encoding.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ ssize_t DecodeBytes(Isolate* isolate,
121121
enum encoding encoding) {
122122
HandleScope scope(isolate);
123123

124-
return StringBytes::Size(isolate, val, encoding);
124+
return StringBytes::Size(isolate, val, encoding).FromMaybe(-1);
125125
}
126126

127127
// Returns number of bytes written.

‎src/node_file.cc

+3-2
Original file line numberDiff line numberDiff line change
@@ -1675,7 +1675,7 @@ static void WriteString(const FunctionCallbackInfo<Value>& args) {
16751675

16761676
if (is_async) { // write(fd, string, pos, enc, req)
16771677
CHECK_NOT_NULL(req_wrap_async);
1678-
len = StringBytes::StorageSize(env->isolate(), value, enc);
1678+
if (!StringBytes::StorageSize(env->isolate(), value, enc).To(&len)) return;
16791679
FSReqBase::FSReqBuffer& stack_buffer =
16801680
req_wrap_async->Init("write", len, enc);
16811681
// StorageSize may return too large a char, so correct the actual length
@@ -1703,7 +1703,8 @@ static void WriteString(const FunctionCallbackInfo<Value>& args) {
17031703
FSReqWrapSync req_wrap_sync;
17041704
FSReqBase::FSReqBuffer stack_buffer;
17051705
if (buf == nullptr) {
1706-
len = StringBytes::StorageSize(env->isolate(), value, enc);
1706+
if (!StringBytes::StorageSize(env->isolate(), value, enc).To(&len))
1707+
return;
17071708
stack_buffer.AllocateSufficientStorage(len + 1);
17081709
// StorageSize may return too large a char, so correct the actual length
17091710
// by the write size

‎src/spawn_sync.cc

+71-54
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,11 @@ using v8::FunctionCallbackInfo;
3737
using v8::HandleScope;
3838
using v8::Integer;
3939
using v8::Isolate;
40+
using v8::Just;
4041
using v8::Local;
42+
using v8::Maybe;
43+
using v8::MaybeLocal;
44+
using v8::Nothing;
4145
using v8::Null;
4246
using v8::Number;
4347
using v8::Object;
@@ -372,7 +376,8 @@ void SyncProcessRunner::Spawn(const FunctionCallbackInfo<Value>& args) {
372376
Environment* env = Environment::GetCurrent(args);
373377
env->PrintSyncTrace();
374378
SyncProcessRunner p(env);
375-
Local<Value> result = p.Run(args[0]);
379+
Local<Value> result;
380+
if (!p.Run(args[0]).ToLocal(&result)) return;
376381
args.GetReturnValue().Set(result);
377382
}
378383

@@ -430,22 +435,21 @@ Environment* SyncProcessRunner::env() const {
430435
return env_;
431436
}
432437

433-
434-
Local<Object> SyncProcessRunner::Run(Local<Value> options) {
438+
MaybeLocal<Object> SyncProcessRunner::Run(Local<Value> options) {
435439
EscapableHandleScope scope(env()->isolate());
436440

437441
CHECK_EQ(lifecycle_, kUninitialized);
438442

439-
TryInitializeAndRunLoop(options);
443+
Maybe<bool> r = TryInitializeAndRunLoop(options);
440444
CloseHandlesAndDeleteLoop();
445+
if (r.IsNothing()) return MaybeLocal<Object>();
441446

442447
Local<Object> result = BuildResultObject();
443448

444449
return scope.Escape(result);
445450
}
446451

447-
448-
void SyncProcessRunner::TryInitializeAndRunLoop(Local<Value> options) {
452+
Maybe<bool> SyncProcessRunner::TryInitializeAndRunLoop(Local<Value> options) {
449453
int r;
450454

451455
// There is no recovery from failure inside TryInitializeAndRunLoop - the
@@ -454,18 +458,24 @@ void SyncProcessRunner::TryInitializeAndRunLoop(Local<Value> options) {
454458
lifecycle_ = kInitialized;
455459

456460
uv_loop_ = new uv_loop_t;
457-
if (uv_loop_ == nullptr)
458-
return SetError(UV_ENOMEM);
461+
if (uv_loop_ == nullptr) {
462+
SetError(UV_ENOMEM);
463+
return Just(false);
464+
}
459465
CHECK_EQ(uv_loop_init(uv_loop_), 0);
460466

461-
r = ParseOptions(options);
462-
if (r < 0)
463-
return SetError(r);
467+
if (!ParseOptions(options).To(&r)) return Nothing<bool>();
468+
if (r < 0) {
469+
SetError(r);
470+
return Just(false);
471+
}
464472

465473
if (timeout_ > 0) {
466474
r = uv_timer_init(uv_loop_, &uv_timer_);
467-
if (r < 0)
468-
return SetError(r);
475+
if (r < 0) {
476+
SetError(r);
477+
return Just(false);
478+
}
469479

470480
uv_unref(reinterpret_cast<uv_handle_t*>(&uv_timer_));
471481

@@ -477,22 +487,28 @@ void SyncProcessRunner::TryInitializeAndRunLoop(Local<Value> options) {
477487
// which implicitly stops it, so there is no risk that the timeout callback
478488
// runs when the process didn't start.
479489
r = uv_timer_start(&uv_timer_, KillTimerCallback, timeout_, 0);
480-
if (r < 0)
481-
return SetError(r);
490+
if (r < 0) {
491+
SetError(r);
492+
return Just(false);
493+
}
482494
}
483495

484496
uv_process_options_.exit_cb = ExitCallback;
485497
r = uv_spawn(uv_loop_, &uv_process_, &uv_process_options_);
486-
if (r < 0)
487-
return SetError(r);
498+
if (r < 0) {
499+
SetError(r);
500+
return Just(false);
501+
}
488502
uv_process_.data = this;
489503

490504
for (uint32_t i = 0; i < stdio_count_; i++) {
491505
SyncProcessStdioPipe* h = stdio_pipes_[i].get();
492506
if (h != nullptr) {
493507
r = h->Start();
494-
if (r < 0)
495-
return SetPipeError(r);
508+
if (r < 0) {
509+
SetPipeError(r);
510+
return Just(false);
511+
}
496512
}
497513
}
498514

@@ -503,6 +519,7 @@ void SyncProcessRunner::TryInitializeAndRunLoop(Local<Value> options) {
503519

504520
// If we get here the process should have exited.
505521
CHECK_GE(exit_status_, 0);
522+
return Just(true);
506523
}
507524

508525

@@ -724,46 +741,41 @@ Local<Array> SyncProcessRunner::BuildOutputArray() {
724741
return scope.Escape(js_output);
725742
}
726743

727-
728-
int SyncProcessRunner::ParseOptions(Local<Value> js_value) {
744+
Maybe<int> SyncProcessRunner::ParseOptions(Local<Value> js_value) {
729745
HandleScope scope(env()->isolate());
730746
int r;
731747

732-
if (!js_value->IsObject())
733-
return UV_EINVAL;
748+
if (!js_value->IsObject()) return Just<int>(UV_EINVAL);
734749

735750
Local<Context> context = env()->context();
736751
Local<Object> js_options = js_value.As<Object>();
737752

738753
Local<Value> js_file =
739754
js_options->Get(context, env()->file_string()).ToLocalChecked();
740-
r = CopyJsString(js_file, &file_buffer_);
741-
if (r < 0)
742-
return r;
755+
if (!CopyJsString(js_file, &file_buffer_).To(&r)) return Nothing<int>();
756+
if (r < 0) return Just(r);
743757
uv_process_options_.file = file_buffer_;
744758

745759
Local<Value> js_args =
746760
js_options->Get(context, env()->args_string()).ToLocalChecked();
747-
r = CopyJsStringArray(js_args, &args_buffer_);
748-
if (r < 0)
749-
return r;
761+
if (!CopyJsStringArray(js_args, &args_buffer_).To(&r)) return Nothing<int>();
762+
if (r < 0) return Just(r);
750763
uv_process_options_.args = reinterpret_cast<char**>(args_buffer_);
751764

752765
Local<Value> js_cwd =
753766
js_options->Get(context, env()->cwd_string()).ToLocalChecked();
754767
if (IsSet(js_cwd)) {
755-
r = CopyJsString(js_cwd, &cwd_buffer_);
756-
if (r < 0)
757-
return r;
768+
if (!CopyJsString(js_cwd, &cwd_buffer_).To(&r)) return Nothing<int>();
769+
if (r < 0) return Just(r);
758770
uv_process_options_.cwd = cwd_buffer_;
759771
}
760772

761773
Local<Value> js_env_pairs =
762774
js_options->Get(context, env()->env_pairs_string()).ToLocalChecked();
763775
if (IsSet(js_env_pairs)) {
764-
r = CopyJsStringArray(js_env_pairs, &env_buffer_);
765-
if (r < 0)
766-
return r;
776+
if (!CopyJsStringArray(js_env_pairs, &env_buffer_).To(&r))
777+
return Nothing<int>();
778+
if (r < 0) return Just(r);
767779

768780
uv_process_options_.env = reinterpret_cast<char**>(env_buffer_);
769781
}
@@ -827,10 +839,9 @@ int SyncProcessRunner::ParseOptions(Local<Value> js_value) {
827839
Local<Value> js_stdio =
828840
js_options->Get(context, env()->stdio_string()).ToLocalChecked();
829841
r = ParseStdioOptions(js_stdio);
830-
if (r < 0)
831-
return r;
842+
if (r < 0) return Just(r);
832843

833-
return 0;
844+
return Just(0);
834845
}
835846

836847

@@ -970,44 +981,43 @@ bool SyncProcessRunner::IsSet(Local<Value> value) {
970981
return !value->IsUndefined() && !value->IsNull();
971982
}
972983

973-
974-
int SyncProcessRunner::CopyJsString(Local<Value> js_value,
975-
const char** target) {
984+
Maybe<int> SyncProcessRunner::CopyJsString(Local<Value> js_value,
985+
const char** target) {
976986
Isolate* isolate = env()->isolate();
977987
Local<String> js_string;
978988
size_t size, written;
979989
char* buffer;
980990

981991
if (js_value->IsString())
982992
js_string = js_value.As<String>();
983-
else
984-
js_string = js_value->ToString(env()->isolate()->GetCurrentContext())
985-
.ToLocalChecked();
993+
else if (!js_value->ToString(env()->isolate()->GetCurrentContext())
994+
.ToLocal(&js_string))
995+
return Nothing<int>();
986996

987997
// Include space for null terminator byte.
988-
size = StringBytes::StorageSize(isolate, js_string, UTF8) + 1;
998+
if (!StringBytes::StorageSize(isolate, js_string, UTF8).To(&size))
999+
return Nothing<int>();
1000+
size += 1;
9891001

9901002
buffer = new char[size];
9911003

9921004
written = StringBytes::Write(isolate, buffer, -1, js_string, UTF8);
9931005
buffer[written] = '\0';
9941006

9951007
*target = buffer;
996-
return 0;
1008+
return Just(0);
9971009
}
9981010

999-
1000-
int SyncProcessRunner::CopyJsStringArray(Local<Value> js_value,
1001-
char** target) {
1011+
Maybe<int> SyncProcessRunner::CopyJsStringArray(Local<Value> js_value,
1012+
char** target) {
10021013
Isolate* isolate = env()->isolate();
10031014
Local<Array> js_array;
10041015
uint32_t length;
10051016
size_t list_size, data_size, data_offset;
10061017
char** list;
10071018
char* buffer;
10081019

1009-
if (!js_value->IsArray())
1010-
return UV_EINVAL;
1020+
if (!js_value->IsArray()) return Just<int>(UV_EINVAL);
10111021

10121022
Local<Context> context = env()->context();
10131023
js_array = js_value.As<Array>()->Clone().As<Array>();
@@ -1025,15 +1035,22 @@ int SyncProcessRunner::CopyJsStringArray(Local<Value> js_value,
10251035
for (uint32_t i = 0; i < length; i++) {
10261036
auto value = js_array->Get(context, i).ToLocalChecked();
10271037

1028-
if (!value->IsString())
1038+
if (!value->IsString()) {
1039+
Local<String> string;
1040+
if (!value->ToString(env()->isolate()->GetCurrentContext())
1041+
.ToLocal(&string))
1042+
return Nothing<int>();
10291043
js_array
10301044
->Set(context,
10311045
i,
10321046
value->ToString(env()->isolate()->GetCurrentContext())
10331047
.ToLocalChecked())
10341048
.FromJust();
1049+
}
10351050

1036-
data_size += StringBytes::StorageSize(isolate, value, UTF8) + 1;
1051+
Maybe<size_t> maybe_size = StringBytes::StorageSize(isolate, value, UTF8);
1052+
if (maybe_size.IsNothing()) return Nothing<int>();
1053+
data_size += maybe_size.FromJust() + 1;
10371054
data_size = ROUND_UP(data_size, sizeof(void*));
10381055
}
10391056

@@ -1057,7 +1074,7 @@ int SyncProcessRunner::CopyJsStringArray(Local<Value> js_value,
10571074
list[length] = nullptr;
10581075

10591076
*target = buffer;
1060-
return 0;
1077+
return Just(0);
10611078
}
10621079

10631080

0 commit comments

Comments
 (0)