Skip to content

Commit 516cdcd

Browse files
RaisinTennodejs-github-bot
RaisinTen
authored and
nodejs-github-bot
committed
src,stream: remove *Check*() calls from non-Initialize() functions
There is no need to crash the process if any of these checks fail. Signed-off-by: Darshan Sen <[email protected]> PR-URL: #40425 Reviewed-By: Anna Henningsen <[email protected]>
1 parent 7ed303b commit 516cdcd

File tree

7 files changed

+104
-53
lines changed

7 files changed

+104
-53
lines changed

‎src/stream_base-inl.h‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,11 @@ int StreamBase::Shutdown(v8::Local<v8::Object> req_wrap_obj) {
149149

150150
const char* msg = Error();
151151
if (msg != nullptr) {
152-
req_wrap_obj->Set(
153-
env->context(),
154-
env->error_string(), OneByteString(env->isolate(), msg)).Check();
152+
if (req_wrap_obj->Set(env->context(),
153+
env->error_string(),
154+
OneByteString(env->isolate(), msg)).IsNothing()) {
155+
return UV_EBUSY;
156+
}
155157
ClearError();
156158
}
157159

@@ -203,9 +205,11 @@ StreamWriteResult StreamBase::Write(
203205

204206
const char* msg = Error();
205207
if (msg != nullptr) {
206-
req_wrap_obj->Set(env->context(),
207-
env->error_string(),
208-
OneByteString(env->isolate(), msg)).Check();
208+
if (req_wrap_obj->Set(env->context(),
209+
env->error_string(),
210+
OneByteString(env->isolate(), msg)).IsNothing()) {
211+
return StreamWriteResult { false, UV_EBUSY, nullptr, 0, {} };
212+
}
209213
ClearError();
210214
}
211215

@@ -279,10 +283,12 @@ void StreamReq::Done(int status, const char* error_str) {
279283
Environment* env = async_wrap->env();
280284
if (error_str != nullptr) {
281285
v8::HandleScope handle_scope(env->isolate());
282-
async_wrap->object()->Set(env->context(),
283-
env->error_string(),
284-
OneByteString(env->isolate(), error_str))
285-
.Check();
286+
if (async_wrap->object()->Set(
287+
env->context(),
288+
env->error_string(),
289+
OneByteString(env->isolate(), error_str)).IsNothing()) {
290+
return;
291+
}
286292
}
287293

288294
OnDone(status);

‎src/stream_base.cc‎

Lines changed: 46 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -106,31 +106,40 @@ int StreamBase::Writev(const FunctionCallbackInfo<Value>& args) {
106106
if (!all_buffers) {
107107
// Determine storage size first
108108
for (size_t i = 0; i < count; i++) {
109-
Local<Value> chunk = chunks->Get(context, i * 2).ToLocalChecked();
109+
Local<Value> chunk;
110+
if (!chunks->Get(context, i * 2).ToLocal(&chunk))
111+
return -1;
110112

111113
if (Buffer::HasInstance(chunk))
112114
continue;
113115
// Buffer chunk, no additional storage required
114116

115117
// String chunk
116-
Local<String> string = chunk->ToString(context).ToLocalChecked();
117-
enum encoding encoding = ParseEncoding(isolate,
118-
chunks->Get(context, i * 2 + 1).ToLocalChecked());
118+
Local<String> string;
119+
if (!chunk->ToString(context).ToLocal(&string))
120+
return -1;
121+
Local<Value> next_chunk;
122+
if (!chunks->Get(context, i * 2 + 1).ToLocal(&next_chunk))
123+
return -1;
124+
enum encoding encoding = ParseEncoding(isolate, next_chunk);
119125
size_t chunk_size;
120-
if (encoding == UTF8 && string->Length() > 65535 &&
121-
!StringBytes::Size(isolate, string, encoding).To(&chunk_size))
122-
return 0;
123-
else if (!StringBytes::StorageSize(isolate, string, encoding)
124-
.To(&chunk_size))
125-
return 0;
126+
if ((encoding == UTF8 &&
127+
string->Length() > 65535 &&
128+
!StringBytes::Size(isolate, string, encoding).To(&chunk_size)) ||
129+
!StringBytes::StorageSize(isolate, string, encoding)
130+
.To(&chunk_size)) {
131+
return -1;
132+
}
126133
storage_size += chunk_size;
127134
}
128135

129136
if (storage_size > INT_MAX)
130137
return UV_ENOBUFS;
131138
} else {
132139
for (size_t i = 0; i < count; i++) {
133-
Local<Value> chunk = chunks->Get(context, i).ToLocalChecked();
140+
Local<Value> chunk;
141+
if (!chunks->Get(context, i).ToLocal(&chunk))
142+
return -1;
134143
bufs[i].base = Buffer::Data(chunk);
135144
bufs[i].len = Buffer::Length(chunk);
136145
}
@@ -145,7 +154,9 @@ int StreamBase::Writev(const FunctionCallbackInfo<Value>& args) {
145154
offset = 0;
146155
if (!all_buffers) {
147156
for (size_t i = 0; i < count; i++) {
148-
Local<Value> chunk = chunks->Get(context, i * 2).ToLocalChecked();
157+
Local<Value> chunk;
158+
if (!chunks->Get(context, i * 2).ToLocal(&chunk))
159+
return -1;
149160

150161
// Write buffer
151162
if (Buffer::HasInstance(chunk)) {
@@ -160,9 +171,13 @@ int StreamBase::Writev(const FunctionCallbackInfo<Value>& args) {
160171
static_cast<char*>(bs ? bs->Data() : nullptr) + offset;
161172
size_t str_size = (bs ? bs->ByteLength() : 0) - offset;
162173

163-
Local<String> string = chunk->ToString(context).ToLocalChecked();
164-
enum encoding encoding = ParseEncoding(isolate,
165-
chunks->Get(context, i * 2 + 1).ToLocalChecked());
174+
Local<String> string;
175+
if (!chunk->ToString(context).ToLocal(&string))
176+
return -1;
177+
Local<Value> next_chunk;
178+
if (!chunks->Get(context, i * 2 + 1).ToLocal(&next_chunk))
179+
return -1;
180+
enum encoding encoding = ParseEncoding(isolate, next_chunk);
166181
str_size = StringBytes::Write(isolate,
167182
str_storage,
168183
str_size,
@@ -207,9 +222,11 @@ int StreamBase::WriteBuffer(const FunctionCallbackInfo<Value>& args) {
207222
send_handle = reinterpret_cast<uv_stream_t*>(wrap->GetHandle());
208223
// Reference LibuvStreamWrap instance to prevent it from being garbage
209224
// collected before `AfterWrite` is called.
210-
req_wrap_obj->Set(env->context(),
211-
env->handle_string(),
212-
send_handle_obj).Check();
225+
if (req_wrap_obj->Set(env->context(),
226+
env->handle_string(),
227+
send_handle_obj).IsNothing()) {
228+
return -1;
229+
}
213230
}
214231

215232
StreamWriteResult res = Write(&buf, 1, send_handle, req_wrap_obj);
@@ -236,12 +253,12 @@ int StreamBase::WriteString(const FunctionCallbackInfo<Value>& args) {
236253
// For UTF8 strings that are very long, go ahead and take the hit for
237254
// computing their actual size, rather than tripling the storage.
238255
size_t storage_size;
239-
if (enc == UTF8 && string->Length() > 65535 &&
240-
!StringBytes::Size(isolate, string, enc).To(&storage_size))
241-
return 0;
242-
else if (!StringBytes::StorageSize(isolate, string, enc)
243-
.To(&storage_size))
244-
return 0;
256+
if ((enc == UTF8 &&
257+
string->Length() > 65535 &&
258+
!StringBytes::Size(isolate, string, enc).To(&storage_size)) ||
259+
!StringBytes::StorageSize(isolate, string, enc).To(&storage_size)) {
260+
return -1;
261+
}
245262

246263
if (storage_size > INT_MAX)
247264
return UV_ENOBUFS;
@@ -312,9 +329,11 @@ int StreamBase::WriteString(const FunctionCallbackInfo<Value>& args) {
312329
send_handle = reinterpret_cast<uv_stream_t*>(wrap->GetHandle());
313330
// Reference LibuvStreamWrap instance to prevent it from being garbage
314331
// collected before `AfterWrite` is called.
315-
req_wrap_obj->Set(env->context(),
316-
env->handle_string(),
317-
send_handle_obj).Check();
332+
if (req_wrap_obj->Set(env->context(),
333+
env->handle_string(),
334+
send_handle_obj).IsNothing()) {
335+
return -1;
336+
}
318337
}
319338

320339
StreamWriteResult res = Write(&buf, 1, send_handle, req_wrap_obj);

‎src/stream_base.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ class StreamReq {
4949
virtual AsyncWrap* GetAsyncWrap() = 0;
5050
inline v8::Local<v8::Object> object();
5151

52+
// TODO(RaisinTen): Update the return type to a Maybe, so that we can indicate
53+
// if there is a pending exception/termination.
5254
inline void Done(int status, const char* error_str = nullptr);
5355
inline void Dispose();
5456

@@ -326,13 +328,17 @@ class StreamBase : public StreamResource {
326328
// subclasses are also `BaseObject`s.
327329
Environment* stream_env() const { return env_; }
328330

331+
// TODO(RaisinTen): Update the return type to a Maybe, so that we can indicate
332+
// if there is a pending exception/termination.
329333
// Shut down the current stream. This request can use an existing
330334
// ShutdownWrap object (that was created in JS), or a new one will be created.
331335
// Returns 1 in case of a synchronous completion, 0 in case of asynchronous
332336
// completion, and a libuv error case in case of synchronous failure.
333337
inline int Shutdown(
334338
v8::Local<v8::Object> req_wrap_obj = v8::Local<v8::Object>());
335339

340+
// TODO(RaisinTen): Update the return type to a Maybe, so that we can indicate
341+
// if there is a pending exception/termination.
336342
// Write data to the current stream. This request can use an existing
337343
// WriteWrap object (that was created in JS), or a new one will be created.
338344
// This will first try to write synchronously using `DoTryWrite()`, then

‎src/stream_pipe.cc‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,26 @@ StreamPipe::StreamPipe(StreamBase* source,
3333
// In particular, this makes sure that they are garbage collected as a group,
3434
// if that applies to the given streams (for example, Http2Streams use
3535
// weak references).
36-
obj->Set(env()->context(), env()->source_string(), source->GetObject())
37-
.Check();
38-
source->GetObject()->Set(env()->context(), env()->pipe_target_string(), obj)
39-
.Check();
40-
obj->Set(env()->context(), env()->sink_string(), sink->GetObject())
41-
.Check();
42-
sink->GetObject()->Set(env()->context(), env()->pipe_source_string(), obj)
43-
.Check();
36+
if (obj->Set(env()->context(),
37+
env()->source_string(),
38+
source->GetObject()).IsNothing()) {
39+
return;
40+
}
41+
if (source->GetObject()->Set(env()->context(),
42+
env()->pipe_target_string(),
43+
obj).IsNothing()) {
44+
return;
45+
}
46+
if (obj->Set(env()->context(),
47+
env()->sink_string(),
48+
sink->GetObject()).IsNothing()) {
49+
return;
50+
}
51+
if (sink->GetObject()->Set(env()->context(),
52+
env()->pipe_source_string(),
53+
obj).IsNothing()) {
54+
return;
55+
}
4456
}
4557

4658
StreamPipe::~StreamPipe() {
@@ -172,7 +184,8 @@ void StreamPipe::WritableListener::OnStreamAfterWrite(WriteWrap* w,
172184
Environment* env = pipe->env();
173185
HandleScope handle_scope(env->isolate());
174186
Context::Scope context_scope(env->context());
175-
pipe->MakeCallback(env->oncomplete_string(), 0, nullptr).ToLocalChecked();
187+
if (pipe->MakeCallback(env->oncomplete_string(), 0, nullptr).IsEmpty())
188+
return;
176189
stream()->RemoveStreamListener(this);
177190
}
178191
return;

‎src/stream_pipe.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,14 @@ namespace node {
99

1010
class StreamPipe : public AsyncWrap {
1111
public:
12-
StreamPipe(StreamBase* source, StreamBase* sink, v8::Local<v8::Object> obj);
1312
~StreamPipe() override;
1413

1514
void Unpipe(bool is_in_deletion = false);
1615

16+
// TODO(RaisinTen): Just like MessagePort, add the following overload:
17+
// static StreamPipe* New(StreamBase* source, StreamBase* sink,
18+
// v8::Local<v8::Object> obj);
19+
// so that we can indicate if there is a pending exception/termination.
1720
static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
1821
static void Start(const v8::FunctionCallbackInfo<v8::Value>& args);
1922
static void Unpipe(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -25,6 +28,8 @@ class StreamPipe : public AsyncWrap {
2528
SET_SELF_SIZE(StreamPipe)
2629

2730
private:
31+
StreamPipe(StreamBase* source, StreamBase* sink, v8::Local<v8::Object> obj);
32+
2833
inline StreamBase* source();
2934
inline StreamBase* sink();
3035

‎src/stream_wrap.cc‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -268,12 +268,12 @@ void LibuvStreamWrap::OnUvRead(ssize_t nread, const uv_buf_t* buf) {
268268
CHECK_EQ(type, UV_UNKNOWN_HANDLE);
269269
}
270270

271-
if (!pending_obj.IsEmpty()) {
272-
object()
273-
->Set(env()->context(),
274-
env()->pending_handle_string(),
275-
pending_obj.ToLocalChecked())
276-
.Check();
271+
Local<Object> local_pending_obj;
272+
if (pending_obj.ToLocal(&local_pending_obj) &&
273+
object()->Set(env()->context(),
274+
env()->pending_handle_string(),
275+
local_pending_obj).IsNothing()) {
276+
return;
277277
}
278278
}
279279

‎src/stream_wrap.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,8 @@ class LibuvStreamWrap : public HandleWrap, public StreamBase {
105105

106106
// Callbacks for libuv
107107
void OnUvAlloc(size_t suggested_size, uv_buf_t* buf);
108+
// TODO(RaisinTen): Update the return type to a Maybe, so that we can indicate
109+
// if there is a pending exception/termination.
108110
void OnUvRead(ssize_t nread, const uv_buf_t* buf);
109111

110112
static void AfterUvWrite(uv_write_t* req, int status);

0 commit comments

Comments
 (0)