1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

src: rework (mostly internal) functions to use Maybes

Browse Source 
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 <targos@protonmail.com>
PR-URL: https://github.com/nodejs/node/pull/21935
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This commit is contained in:
Ujjwal Sharma 2018-08-29 15:39:55 +02:00 committed by Anna Henningsen
parent 67403b3a84
commit a55c57b8c4
No known key found for this signature in database
GPG Key ID: 9C63F3A6CD2AD8F9
10 changed files with 149 additions and 117 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/node_buffer.cc
View File

@ -236,7 +236,9 @@ MaybeLocal<Object> New(Isolate* isolate,
enum encoding enc) {
EscapableHandleScope scope(isolate);
const size_t length = StringBytes::Size(isolate, string, enc);
size_t length;
if (!StringBytes::Size(isolate, string, enc).To(&length))
return Local<Object>();
size_t actual = 0;
char* data = nullptr;
@ -828,7 +830,8 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {
const size_t haystack_length = (enc == UCS2) ?
ts_obj_length &~ 1 : ts_obj_length; // NOLINT(whitespace/operators)
const size_t needle_length = StringBytes::Size(isolate, needle, enc);
size_t needle_length;
if (!StringBytes::Size(isolate, needle, enc).To(&needle_length)) return;
int64_t opt_offset = IndexOfOffset(haystack_length,
offset_i64,
@ -868,7 +871,7 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {
if (IsBigEndian()) {
StringBytes::InlineDecoder decoder;
decoder.Decode(env, needle, args[3], UCS2);
if (decoder.Decode(env, needle, args[3], UCS2).IsNothing()) return;
const uint16_t* decoded_string =
reinterpret_cast<const uint16_t*>(decoder.out());

9
src/node_crypto.cc
View File

@ -3059,7 +3059,8 @@ void CipherBase::Update(const FunctionCallbackInfo<Value>& args) {
// Only copy the data if we have to, because it's a string
if (args[0]->IsString()) {
StringBytes::InlineDecoder decoder;
if (!decoder.Decode(env, args[0].As<String>(), args[1], UTF8))
if (!decoder.Decode(env, args[0].As<String>(), args[1], UTF8)
.FromMaybe(false))
return;
r = cipher->Update(decoder.out(), decoder.size(), &out, &out_len);
} else {
@ -3249,7 +3250,8 @@ void Hmac::HmacUpdate(const FunctionCallbackInfo<Value>& args) {
bool r = false;
if (args[0]->IsString()) {
StringBytes::InlineDecoder decoder;
if (decoder.Decode(env, args[0].As<String>(), args[1], UTF8)) {
if (decoder.Decode(env, args[0].As<String>(), args[1], UTF8)
.FromMaybe(false)) {
r = hmac->HmacUpdate(decoder.out(), decoder.size());
}
} else {
@ -3356,7 +3358,8 @@ void Hash::HashUpdate(const FunctionCallbackInfo<Value>& args) {
bool r = true;
if (args[0]->IsString()) {
StringBytes::InlineDecoder decoder;
if (!decoder.Decode(env, args[0].As<String>(), args[1], UTF8)) {
if (!decoder.Decode(env, args[0].As<String>(), args[1], UTF8)
.FromMaybe(false)) {
args.GetReturnValue().Set(false);
return;
}

2
src/node_encoding.cc
View File

@ -121,7 +121,7 @@ ssize_t DecodeBytes(Isolate* isolate,
enum encoding encoding) {
HandleScope scope(isolate);
return StringBytes::Size(isolate, val, encoding);
return StringBytes::Size(isolate, val, encoding).FromMaybe(-1);
}
// Returns number of bytes written.

5
src/node_file.cc
View File

@ -1819,7 +1819,7 @@ static void WriteString(const FunctionCallbackInfo<Value>& args) {
if (is_async) { // write(fd, string, pos, enc, req)
CHECK_NOT_NULL(req_wrap_async);
len = StringBytes::StorageSize(env->isolate(), value, enc);
if (!StringBytes::StorageSize(env->isolate(), value, enc).To(&len)) return;
FSReqBase::FSReqBuffer& stack_buffer =
req_wrap_async->Init("write", len, enc);
// StorageSize may return too large a char, so correct the actual length
@ -1847,7 +1847,8 @@ static void WriteString(const FunctionCallbackInfo<Value>& args) {
FSReqWrapSync req_wrap_sync;
FSReqBase::FSReqBuffer stack_buffer;
if (buf == nullptr) {
len = StringBytes::StorageSize(env->isolate(), value, enc);
if (!StringBytes::StorageSize(env->isolate(), value, enc).To(&len))
return;
stack_buffer.AllocateSufficientStorage(len + 1);
// StorageSize may return too large a char, so correct the actual length
// by the write size

125
src/spawn_sync.cc
View File

@ -37,7 +37,11 @@ using v8::FunctionCallbackInfo;
using v8::HandleScope;
using v8::Integer;
using v8::Isolate;
using v8::Just;
using v8::Local;
using v8::Maybe;
using v8::MaybeLocal;
using v8::Nothing;
using v8::Null;
using v8::Number;
using v8::Object;
@ -372,7 +376,8 @@ void SyncProcessRunner::Spawn(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
env->PrintSyncTrace();
SyncProcessRunner p(env);
Local<Value> result = p.Run(args[0]);
Local<Value> result;
if (!p.Run(args[0]).ToLocal(&result)) return;
args.GetReturnValue().Set(result);
}
@ -430,22 +435,21 @@ Environment* SyncProcessRunner::env() const {
return env_;
}
Local<Object> SyncProcessRunner::Run(Local<Value> options) {
MaybeLocal<Object> SyncProcessRunner::Run(Local<Value> options) {
EscapableHandleScope scope(env()->isolate());
CHECK_EQ(lifecycle_, kUninitialized);
TryInitializeAndRunLoop(options);
Maybe<bool> r = TryInitializeAndRunLoop(options);
CloseHandlesAndDeleteLoop();
if (r.IsNothing()) return MaybeLocal<Object>();
Local<Object> result = BuildResultObject();
return scope.Escape(result);
}
void SyncProcessRunner::TryInitializeAndRunLoop(Local<Value> options) {
Maybe<bool> SyncProcessRunner::TryInitializeAndRunLoop(Local<Value> options) {
int r;
// There is no recovery from failure inside TryInitializeAndRunLoop - the
@ -454,18 +458,24 @@ void SyncProcessRunner::TryInitializeAndRunLoop(Local<Value> options) {
lifecycle_ = kInitialized;
uv_loop_ = new uv_loop_t;
if (uv_loop_ == nullptr)
return SetError(UV_ENOMEM);
if (uv_loop_ == nullptr) {
SetError(UV_ENOMEM);
return Just(false);
}
CHECK_EQ(uv_loop_init(uv_loop_), 0);
r = ParseOptions(options);
if (r < 0)
return SetError(r);
if (!ParseOptions(options).To(&r)) return Nothing<bool>();
if (r < 0) {
SetError(r);
return Just(false);
}
if (timeout_ > 0) {
r = uv_timer_init(uv_loop_, &uv_timer_);
if (r < 0)
return SetError(r);
if (r < 0) {
SetError(r);
return Just(false);
}
uv_unref(reinterpret_cast<uv_handle_t*>(&uv_timer_));
@ -477,22 +487,28 @@ void SyncProcessRunner::TryInitializeAndRunLoop(Local<Value> options) {
// which implicitly stops it, so there is no risk that the timeout callback
// runs when the process didn't start.
r = uv_timer_start(&uv_timer_, KillTimerCallback, timeout_, 0);
if (r < 0)
return SetError(r);
if (r < 0) {
SetError(r);
return Just(false);
}
}
uv_process_options_.exit_cb = ExitCallback;
r = uv_spawn(uv_loop_, &uv_process_, &uv_process_options_);
if (r < 0)
return SetError(r);
if (r < 0) {
SetError(r);
return Just(false);
}
uv_process_.data = this;
for (uint32_t i = 0; i < stdio_count_; i++) {
SyncProcessStdioPipe* h = stdio_pipes_[i].get();
if (h != nullptr) {
r = h->Start();
if (r < 0)
return SetPipeError(r);
if (r < 0) {
SetPipeError(r);
return Just(false);
}
}
}
@ -503,6 +519,7 @@ void SyncProcessRunner::TryInitializeAndRunLoop(Local<Value> options) {
// If we get here the process should have exited.
CHECK_GE(exit_status_, 0);
return Just(true);
}
@ -724,46 +741,41 @@ Local<Array> SyncProcessRunner::BuildOutputArray() {
return scope.Escape(js_output);
}
int SyncProcessRunner::ParseOptions(Local<Value> js_value) {
Maybe<int> SyncProcessRunner::ParseOptions(Local<Value> js_value) {
HandleScope scope(env()->isolate());
int r;
if (!js_value->IsObject())
return UV_EINVAL;
if (!js_value->IsObject()) return Just<int>(UV_EINVAL);
Local<Context> context = env()->context();
Local<Object> js_options = js_value.As<Object>();
Local<Value> js_file =
js_options->Get(context, env()->file_string()).ToLocalChecked();
r = CopyJsString(js_file, &file_buffer_);
if (r < 0)
return r;
if (!CopyJsString(js_file, &file_buffer_).To(&r)) return Nothing<int>();
if (r < 0) return Just(r);
uv_process_options_.file = file_buffer_;
Local<Value> js_args =
js_options->Get(context, env()->args_string()).ToLocalChecked();
r = CopyJsStringArray(js_args, &args_buffer_);
if (r < 0)
return r;
if (!CopyJsStringArray(js_args, &args_buffer_).To(&r)) return Nothing<int>();
if (r < 0) return Just(r);
uv_process_options_.args = reinterpret_cast<char**>(args_buffer_);
Local<Value> js_cwd =
js_options->Get(context, env()->cwd_string()).ToLocalChecked();
if (IsSet(js_cwd)) {
r = CopyJsString(js_cwd, &cwd_buffer_);
if (r < 0)
return r;
if (!CopyJsString(js_cwd, &cwd_buffer_).To(&r)) return Nothing<int>();
if (r < 0) return Just(r);
uv_process_options_.cwd = cwd_buffer_;
}
Local<Value> js_env_pairs =
js_options->Get(context, env()->env_pairs_string()).ToLocalChecked();
if (IsSet(js_env_pairs)) {
r = CopyJsStringArray(js_env_pairs, &env_buffer_);
if (r < 0)
return r;
if (!CopyJsStringArray(js_env_pairs, &env_buffer_).To(&r))
return Nothing<int>();
if (r < 0) return Just(r);
uv_process_options_.env = reinterpret_cast<char**>(env_buffer_);
}
@ -827,10 +839,9 @@ int SyncProcessRunner::ParseOptions(Local<Value> js_value) {
Local<Value> js_stdio =
js_options->Get(context, env()->stdio_string()).ToLocalChecked();
r = ParseStdioOptions(js_stdio);
if (r < 0)
return r;
if (r < 0) return Just(r);
return 0;
return Just(0);
}
@ -970,9 +981,8 @@ bool SyncProcessRunner::IsSet(Local<Value> value) {
return !value->IsUndefined() && !value->IsNull();
}
int SyncProcessRunner::CopyJsString(Local<Value> js_value,
const char** target) {
Maybe<int> SyncProcessRunner::CopyJsString(Local<Value> js_value,
const char** target) {
Isolate* isolate = env()->isolate();
Local<String> js_string;
size_t size, written;
@ -980,12 +990,14 @@ int SyncProcessRunner::CopyJsString(Local<Value> js_value,
if (js_value->IsString())
js_string = js_value.As<String>();
else
js_string = js_value->ToString(env()->isolate()->GetCurrentContext())
.ToLocalChecked();
else if (!js_value->ToString(env()->isolate()->GetCurrentContext())
.ToLocal(&js_string))
return Nothing<int>();
// Include space for null terminator byte.
size = StringBytes::StorageSize(isolate, js_string, UTF8) + 1;
if (!StringBytes::StorageSize(isolate, js_string, UTF8).To(&size))
return Nothing<int>();
size += 1;
buffer = new char[size];
@ -993,12 +1005,11 @@ int SyncProcessRunner::CopyJsString(Local<Value> js_value,
buffer[written] = '\0';
*target = buffer;
return 0;
return Just(0);
}
int SyncProcessRunner::CopyJsStringArray(Local<Value> js_value,
char** target) {
Maybe<int> SyncProcessRunner::CopyJsStringArray(Local<Value> js_value,
char** target) {
Isolate* isolate = env()->isolate();
Local<Array> js_array;
uint32_t length;
@ -1006,8 +1017,7 @@ int SyncProcessRunner::CopyJsStringArray(Local<Value> js_value,
char** list;
char* buffer;
if (!js_value->IsArray())
return UV_EINVAL;
if (!js_value->IsArray()) return Just<int>(UV_EINVAL);
Local<Context> context = env()->context();
js_array = js_value.As<Array>()->Clone().As<Array>();
@ -1025,15 +1035,22 @@ int SyncProcessRunner::CopyJsStringArray(Local<Value> js_value,
for (uint32_t i = 0; i < length; i++) {
auto value = js_array->Get(context, i).ToLocalChecked();
if (!value->IsString())
if (!value->IsString()) {
Local<String> string;
if (!value->ToString(env()->isolate()->GetCurrentContext())
.ToLocal(&string))
return Nothing<int>();
js_array
->Set(context,
i,
value->ToString(env()->isolate()->GetCurrentContext())
.ToLocalChecked())
.FromJust();
}
data_size += StringBytes::StorageSize(isolate, value, UTF8) + 1;
Maybe<size_t> maybe_size = StringBytes::StorageSize(isolate, value, UTF8);
if (maybe_size.IsNothing()) return Nothing<int>();
data_size += maybe_size.FromJust() + 1;
data_size = ROUND_UP(data_size, sizeof(void*));
}
@ -1057,7 +1074,7 @@ int SyncProcessRunner::CopyJsStringArray(Local<Value> js_value,
list[length] = nullptr;
*target = buffer;
return 0;
return Just(0);
}

12
src/spawn_sync.h
View File

@ -152,8 +152,8 @@ class SyncProcessRunner {
inline Environment* env() const;
v8::Local<v8::Object> Run(v8::Local<v8::Value> options);
void TryInitializeAndRunLoop(v8::Local<v8::Value> options);
v8::MaybeLocal<v8::Object> Run(v8::Local<v8::Value> options);
v8::Maybe<bool> TryInitializeAndRunLoop(v8::Local<v8::Value> options);
void CloseHandlesAndDeleteLoop();
void CloseStdioPipes();
@ -172,7 +172,7 @@ class SyncProcessRunner {
v8::Local<v8::Object> BuildResultObject();
v8::Local<v8::Array> BuildOutputArray();
int ParseOptions(v8::Local<v8::Value> js_value);
v8::Maybe<int> ParseOptions(v8::Local<v8::Value> js_value);
int ParseStdioOptions(v8::Local<v8::Value> js_value);
int ParseStdioOption(int child_fd, v8::Local<v8::Object> js_stdio_option);
@ -184,8 +184,10 @@ class SyncProcessRunner {
inline int AddStdioInheritFD(uint32_t child_fd, int inherit_fd);
static bool IsSet(v8::Local<v8::Value> value);
int CopyJsString(v8::Local<v8::Value> js_value, const char** target);
int CopyJsStringArray(v8::Local<v8::Value> js_value, char** target);
v8::Maybe<int> CopyJsString(v8::Local<v8::Value> js_value,
const char** target);
v8::Maybe<int> CopyJsStringArray(v8::Local<v8::Value> js_value,
char** target);
static void ExitCallback(uv_process_t* handle,
int64_t exit_status,

21
src/stream_base.cc
View File

@ -108,11 +108,12 @@ int StreamBase::Writev(const FunctionCallbackInfo<Value>& args) {
enum encoding encoding = ParseEncoding(env->isolate(),
chunks->Get(i * 2 + 1));
size_t chunk_size;
if (encoding == UTF8 && string->Length() > 65535)
chunk_size = StringBytes::Size(env->isolate(), string, encoding);
else
chunk_size = StringBytes::StorageSize(env->isolate(), string, encoding);
if (encoding == UTF8 && string->Length() > 65535 &&
!StringBytes::Size(env->isolate(), string, encoding).To(&chunk_size))
return 0;
else if (!StringBytes::StorageSize(env->isolate(), string, encoding)
.To(&chunk_size))
return 0;
storage_size += chunk_size;
}
@ -214,10 +215,12 @@ int StreamBase::WriteString(const FunctionCallbackInfo<Value>& args) {
// For UTF8 strings that are very long, go ahead and take the hit for
// computing their actual size, rather than tripling the storage.
size_t storage_size;
if (enc == UTF8 && string->Length() > 65535)
storage_size = StringBytes::Size(env->isolate(), string, enc);
else
storage_size = StringBytes::StorageSize(env->isolate(), string, enc);
if (enc == UTF8 && string->Length() > 65535 &&
!StringBytes::Size(env->isolate(), string, enc).To(&storage_size))
return 0;
else if (!StringBytes::StorageSize(env->isolate(), string, enc)
.To(&storage_size))
return 0;
if (storage_size > INT_MAX)
return UV_ENOBUFS;

42
src/string_bytes.cc
View File

@ -41,8 +41,11 @@ namespace node {
using v8::HandleScope;
using v8::Isolate;
using v8::Just;
using v8::Local;
using v8::Maybe;
using v8::MaybeLocal;
using v8::Nothing;
using v8::String;
using v8::Value;
@ -399,19 +402,20 @@ bool StringBytes::IsValidString(Local<String> string,
// Quick and dirty size calculation
// Will always be at least big enough, but may have some extra
// UTF8 can be as much as 3x the size, Base64 can have 1-2 extra bytes
size_t StringBytes::StorageSize(Isolate* isolate,
Local<Value> val,
enum encoding encoding) {
Maybe<size_t> StringBytes::StorageSize(Isolate* isolate,
Local<Value> val,
enum encoding encoding) {
HandleScope scope(isolate);
size_t data_size = 0;
bool is_buffer = Buffer::HasInstance(val);
if (is_buffer && (encoding == BUFFER || encoding == LATIN1)) {
return Buffer::Length(val);
return Just(Buffer::Length(val));
}
Local<String> str =
val->ToString(isolate->GetCurrentContext()).ToLocalChecked();
Local<String> str;
if (!val->ToString(isolate->GetCurrentContext()).ToLocal(&str))
return Nothing<size_t>();
switch (encoding) {
case ASCII:
@ -445,40 +449,40 @@ size_t StringBytes::StorageSize(Isolate* isolate,
break;
}
return data_size;
return Just(data_size);
}
size_t StringBytes::Size(Isolate* isolate,
Local<Value> val,
enum encoding encoding) {
Maybe<size_t> StringBytes::Size(Isolate* isolate,
Local<Value> val,
enum encoding encoding) {
HandleScope scope(isolate);
if (Buffer::HasInstance(val) && (encoding == BUFFER || encoding == LATIN1))
return Buffer::Length(val);
return Just(Buffer::Length(val));
Local<String> str =
val->ToString(isolate->GetCurrentContext()).ToLocalChecked();
Local<String> str;
if (!val->ToString(isolate->GetCurrentContext()).ToLocal(&str))
return Nothing<size_t>();
switch (encoding) {
case ASCII:
case LATIN1:
return str->Length();
return Just<size_t>(str->Length());
case BUFFER:
case UTF8:
return str->Utf8Length(isolate);
return Just<size_t>(str->Utf8Length(isolate));
case UCS2:
return str->Length() * sizeof(uint16_t);
return Just(str->Length() * sizeof(uint16_t));
case BASE64: {
String::Value value(isolate, str);
return base64_decoded_size(*value, value.length());
return Just(base64_decoded_size(*value, value.length()));
}
case HEX:
return str->Length() / 2;
return Just<size_t>(str->Length() / 2);
}
UNREACHABLE();

37
src/string_bytes.h
View File

@ -35,29 +35,26 @@ class StringBytes {
public:
class InlineDecoder : public MaybeStackBuffer<char> {
public:
inline bool Decode(Environment* env,
v8::Local<v8::String> string,
v8::Local<v8::Value> encoding,
enum encoding _default) {
inline v8::Maybe<bool> Decode(Environment* env,
v8::Local<v8::String> string,
v8::Local<v8::Value> encoding,
enum encoding _default) {
enum encoding enc = ParseEncoding(env->isolate(), encoding, _default);
if (!StringBytes::IsValidString(string, enc)) {
env->ThrowTypeError("Bad input string");
return false;
return v8::Just(false);
}
const size_t storage = StringBytes::StorageSize(env->isolate(),
string,
enc);
size_t storage;
if (!StringBytes::StorageSize(env->isolate(), string, enc).To(&storage))
return v8::Nothing<bool>();
AllocateSufficientStorage(storage);
const size_t length = StringBytes::Write(env->isolate(),
out(),
storage,
string,
enc);
const size_t length =
StringBytes::Write(env->isolate(), out(), storage, string, enc);
// No zero terminator is included when using this method.
SetLength(length);
return true;
return v8::Just(true);
}
inline size_t size() const { return length(); }
@ -71,15 +68,15 @@ class StringBytes {
// Fast, but can be 2 bytes oversized for Base64, and
// as much as triple UTF-8 strings <= 65536 chars in length
static size_t StorageSize(v8::Isolate* isolate,
v8::Local<v8::Value> val,
enum encoding enc);
static v8::Maybe<size_t> StorageSize(v8::Isolate* isolate,
v8::Local<v8::Value> val,
enum encoding enc);
// Precise byte count, but slightly slower for Base64 and
// very much slower for UTF-8
static size_t Size(v8::Isolate* isolate,
v8::Local<v8::Value> val,
enum encoding enc);
static v8::Maybe<size_t> Size(v8::Isolate* isolate,
v8::Local<v8::Value> val,
enum encoding enc);
// Write the bytes from the string or buffer into the char*
// returns the number of bytes written, which will always be

4
src/util.cc
View File

@ -41,7 +41,9 @@ static void MakeUtf8String(Isolate* isolate,
Local<String> string;
if (!value->ToString(isolate->GetCurrentContext()).ToLocal(&string)) return;
const size_t storage = StringBytes::StorageSize(isolate, string, UTF8) + 1;
size_t storage;
if (!StringBytes::StorageSize(isolate, string, UTF8).To(&storage)) return;
storage += 1;
target->AllocateSufficientStorage(storage);
const int flags =
String::NO_NULL_TERMINATION | String::REPLACE_INVALID_UTF8;